Repository: yarpc/yab
Pull request #326 (2021-03-04 16:10:33)

Add benchmark caller result to hold call latency 
This pull request is a duplicate of #324, which merged into the #319's branch, and it is currently in the review phase. This pull request aims to land those same changes into dev and then rebase #319 with `dev` to scrap out the #324 changes from its diff.

codecov[bot]: # [Codecov](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=h1) Report
> Merging [#326](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=desc) (a15e45f) into [dev](https://codecov.io/gh/yarpc/yab/commit/439b42e6858d230c8d6343dc75d50345caa38e3c?el=desc) (439b42e) will **decrease** coverage by `0.10%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/yarpc/yab/pull/326/graphs/tree.svg?width=650&height=150&src=pr&token=yEgrqUTCuh)](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##              dev     #326      +/-   ##
==========================================
- Coverage   90.44%   90.34%   -0.11%     
==========================================
  Files          53       53              
  Lines        2586     2588       +2     
==========================================
- Hits         2339     2338       -1     
- Misses        175      177       +2     
- Partials       72       73       +1     
```


| [Impacted Files](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bench\_method.go](https://codecov.io/gh/yarpc/yab/pull/326/diff?src=pr&el=tree#diff-YmVuY2hfbWV0aG9kLmdv) | `93.54% <100.00%> (+0.44%)` | :arrow_up: |
| [benchmark.go](https://codecov.io/gh/yarpc/yab/pull/326/diff?src=pr&el=tree#diff-YmVuY2htYXJrLmdv) | `93.49% <100.00%> (ø)` | |
| [benchmark\_unary.go](https://codecov.io/gh/yarpc/yab/pull/326/diff?src=pr&el=tree#diff-YmVuY2htYXJrX3VuYXJ5Lmdv) | `100.00% <100.00%> (ø)` | |
| [handler.go](https://codecov.io/gh/yarpc/yab/pull/326/diff?src=pr&el=tree#diff-aGFuZGxlci5nbw==) | `79.60% <0.00%> (-1.98%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=footer). Last update [439b42e...a15e45f](https://codecov.io/gh/yarpc/yab/pull/326?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/bench_method.go b/bench_method.go
index afda89e..07df6c6 100644
--- a/bench_method.go
+++ b/bench_method.go
@@ -38,7 +38,19 @@ type peerTransport struct {
 // benchmarkCaller exposes method to dispatch requests for benchmark.
 type benchmarkCaller interface {
 	// Call dispatches a request using the provided transport.
-	Call(transport.Transport) (time.Duration, error)
+	Call(transport.Transport) (benchmarkCallLatencyResult, error)
+}
+
+type benchmarkCallLatencyResult struct {
+	latency time.Duration
+}
+
+func newBenchmarkCallLatencyResult(latency time.Duration) benchmarkCallLatencyResult {
+	return benchmarkCallLatencyResult{latency}
+}
+
+func (r benchmarkCallLatencyResult) Latency() time.Duration {
+	return r.latency
 }
 
 // warmTransport warms up a transport and returns it. The transport is warmed
diff --git a/benchmark.go b/benchmark.go
index ee4b638..9748b65 100644
--- a/benchmark.go
+++ b/benchmark.go
@@ -111,7 +111,7 @@ func (o BenchmarkOptions) enabled() bool {
 
 func runWorker(t transport.Transport, b benchmarkCaller, s *benchmarkState, run *limiter.Run, logger *zap.Logger) {
 	for cur := run; cur.More(); {
-		latency, err := b.Call(t)
+		callResult, err := b.Call(t)
 		if err != nil {
 			s.recordError(err)
 			// TODO: Add information about which peer specifically failed.
@@ -119,7 +119,7 @@ func runWorker(t transport.Transport, b benchmarkCaller, s *benchmarkState, run
 			continue
 		}
 
-		s.recordLatency(latency)
+		s.recordLatency(callResult.Latency())
 	}
 }
 
diff --git a/benchmark_unary.go b/benchmark_unary.go
index d952b7c..232b510 100644
--- a/benchmark_unary.go
+++ b/benchmark_unary.go
@@ -34,7 +34,7 @@ type benchmarkUnaryMethod struct {
 }
 
 // Call dispatches unary request on the provided transport.
-func (m benchmarkUnaryMethod) Call(t transport.Transport) (time.Duration, error) {
+func (m benchmarkUnaryMethod) Call(t transport.Transport) (benchmarkCallLatencyResult, error) {
 	start := time.Now()
 	res, err := makeRequest(t, m.req)
 	duration := time.Since(start)
@@ -42,5 +42,5 @@ func (m benchmarkUnaryMethod) Call(t transport.Transport) (time.Duration, error)
 	if err == nil {
 		err = m.serializer.CheckSuccess(res)
 	}
-	return duration, err
+	return newBenchmarkCallLatencyResult(duration), err
 }


Test patch:
diff --git a/benchmark_unary_test.go b/benchmark_unary_test.go
index 672ab95..b902c7c 100644
--- a/benchmark_unary_test.go
+++ b/benchmark_unary_test.go
@@ -159,7 +159,7 @@ func TestBenchmarkMethodCall(t *testing.T) {
 			m.req.Method = tt.reqMethod
 		}
 
-		d, err := m.Call(tp)
+		callResult, err := m.Call(tp)
 		if tt.wantErr != "" {
 			if assert.Error(t, err, "call should fail") {
 				assert.Contains(t, err.Error(), tt.wantErr, "call should return 0 duration")
@@ -168,7 +168,7 @@ func TestBenchmarkMethodCall(t *testing.T) {
 		}
 
 		assert.NoError(t, err, "call should not fail")
-		assert.True(t, d > time.Microsecond, "duration was too short, got %v", d)
+		assert.True(t, callResult.Latency() > time.Microsecond, "duration was too short, got %v", callResult.Latency())
 	}
 }