Skip to content

Commit

Permalink
lib: use vu ctx instead of request ctx for pushing metrics
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cuonglm committed Dec 6, 2019
1 parent b788e2e commit 4722cb3
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
4 changes: 2 additions & 2 deletions js/modules/k6/http/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,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)
}
})
Expand Down
2 changes: 1 addition & 1 deletion lib/netext/httpext/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
7 changes: 5 additions & 2 deletions lib/netext/httpext/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 4722cb3

Please sign in to comment.