From a0d9f54f2edf6b239882128c4e5e35b934cf6288 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Fri, 6 Dec 2019 09:30:58 +0100 Subject: [PATCH] lib: use vu ctx instead of request ctx for pushing metrics PushIfNotDone is intended to use with VU context instead of request context. This is a bug in current code base, but it is not triggered due to the bug in Go net/http. A request context can be in cancelled state due to be cancelled or timeouted. PushIfNotDone won't see the difference and will skip pushing metrics for timeouted requests. Fixing it by passing VU context to PushIfNotDone instead of request ctx. Fixes #1260 --- js/modules/k6/http/request_test.go | 4 ++-- lib/netext/httpext/request.go | 2 +- lib/netext/httpext/transport.go | 7 +++++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/js/modules/k6/http/request_test.go b/js/modules/k6/http/request_test.go index aa8648918a1..a77fc022b26 100644 --- a/js/modules/k6/http/request_test.go +++ b/js/modules/k6/http/request_test.go @@ -276,13 +276,13 @@ func TestRequestAndBatch(t *testing.T) { `)) endTime := time.Now() require.Error(t, err) - assert.Contains(t, err.Error(), "Client.Timeout exceeded while awaiting headers") + assert.Contains(t, err.Error(), "context deadline exceeded") assert.WithinDuration(t, startTime.Add(1*time.Second), endTime, 2*time.Second) logEntry := hook.LastEntry() if assert.NotNil(t, logEntry) { assert.Equal(t, logrus.WarnLevel, logEntry.Level) - assert.Contains(t, logEntry.Data["error"].(error).Error(), "Client.Timeout exceeded while awaiting headers") + assert.Contains(t, logEntry.Data["error"].(error).Error(), "context deadline exceeded") assert.Equal(t, "Request Failed", logEntry.Message) } }) diff --git a/lib/netext/httpext/request.go b/lib/netext/httpext/request.go index bf4462ee9e0..72acd545022 100644 --- a/lib/netext/httpext/request.go +++ b/lib/netext/httpext/request.go @@ -264,7 +264,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error } } - tracerTransport := newTransport(state, tags) + tracerTransport := newTransport(ctx, state, tags) var transport http.RoundTripper = tracerTransport if state.Options.HTTPDebug.String != "" { diff --git a/lib/netext/httpext/transport.go b/lib/netext/httpext/transport.go index b73c3785133..3950454ebe3 100644 --- a/lib/netext/httpext/transport.go +++ b/lib/netext/httpext/transport.go @@ -33,9 +33,10 @@ import ( "github.com/loadimpact/k6/stats" ) -// transport is an implemenation of http.RoundTripper that will measure and emit +// transport is an implementation of http.RoundTripper that will measure and emit // different metrics for each roundtrip type transport struct { + ctx context.Context state *lib.State tags map[string]string @@ -72,10 +73,12 @@ var _ http.RoundTripper = &transport{} // requests made through it and annotates and emits the recorded metric samples // through the state.Samples channel. func newTransport( + ctx context.Context, state *lib.State, tags map[string]string, ) *transport { return &transport{ + ctx: ctx, state: state, tags: tags, lastRequestLock: new(sync.Mutex), @@ -147,7 +150,7 @@ func (t *transport) measureAndEmitMetrics(unfReq *unfinishedRequest) *finishedRe } trail.SaveSamples(stats.IntoSampleTags(&tags)) - stats.PushIfNotDone(unfReq.ctx, t.state.Samples, trail) + stats.PushIfNotDone(t.ctx, t.state.Samples, trail) return result }