Skip to content

Commit

Permalink
Add error_code tag to http metrics resolves #895
Browse files Browse the repository at this point in the history 
This also sets the error tag to the now more non dynamic messages.

This also changes a little bit of the behaviour specifically in that if
there is an error while making the http request that is not from the
RoundTripper than we consider this to be exception worthy now and
independantly of `throw` it will result in a exception. This should not
be ... common at all: the only error that is suppose to be able to
happen is if a redirect has an unparsable/missing location.
  • Loading branch information
@mstoykov
mstoykov committed Mar 7, 2019
1 parent 97cd435 commit c77b461
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 30 deletions.
84 changes: 84 additions & 0 deletions js/modules/k6/http/request_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,85 @@ func TestRequestAndBatch(t *testing.T) {
assert.Error(t, err)
})

t.Run("ErrorCodes", func(t *testing.T) {
defer func(value null.Bool) {
state.Options.Throw = value
}(state.Options.Throw)
state.Options.Throw = null.BoolFrom(false)

var checkErrorCode = func(t testing.TB, tags *stats.SampleTags, code int, msg string) {
var errorMsg, ok = tags.Get("error")
if msg == "" {
assert.False(t, ok)
} else {
assert.Equal(t, errorMsg, msg)
}
errorCodeStr, ok := tags.Get("error_code")
if code == 0 {
assert.False(t, ok)
} else {
var errorCode, err = strconv.Atoi(errorCodeStr)
assert.NoError(t, err)
assert.Equal(t, errorCode, code)
}
}
var testCases = []struct {
name string
status int
expectedErrorCode int
expectedErrorMsg string
script string
}{
{
name: "Unroutable",
expectedErrorCode: 1101,
expectedErrorMsg: "lookup: no such host",
script: `let res = http.request("GET", "http://sdafsgdhfjg/");`,
},
{
name: "Unroutable redirect",
expectedErrorCode: 1101,
expectedErrorMsg: "lookup: no such host",
script: `let res = http.request("GET", "http://HTTPBIN/redirectto?url=http://sdafsgdhfjg/");`,
},
{
name: "Missing protocol",
expectedErrorCode: 1000,
expectedErrorMsg: `unsupported protocol scheme ""`,
script: `let res = http.request("GET", "dafsgdhfjg/");`,
},
{
name: "Too many redirects",
status: 302,
script: `
let res = http.get("HTTPBIN_URL/redirect/1", {redirects: 0});
if (res.url != "HTTPBIN_URL/redirect/1") { throw new Error("incorrect URL: " + res.url) }`,
},
}

for _, testCase := range testCases {
// clear the Samples
stats.GetBufferedSamples(samples)
t.Run(testCase.name, func(t *testing.T) {
_, err := common.RunString(rt,
sr(testCase.script+"\n"+fmt.Sprintf(`
if (res.status != %d) { /*throw new Error("wrong status: "+ res.status);*/}
if (res.error != '%s') { throw new Error("wrong error: "+ res.error);}
if (res.error_code != %d) { throw new Error("wrong error_code: "+ res.error_code);}
`, testCase.status, testCase.expectedErrorMsg, testCase.expectedErrorCode)))
require.NoError(t, err)
var cs = stats.GetBufferedSamples(samples)
require.Len(t, cs, 1)
for _, c := range cs {
require.NotZero(t, len(c.GetSamples()))
for _, sample := range c.GetSamples() {
checkErrorCode(t, sample.GetTags(), testCase.expectedErrorCode, testCase.expectedErrorMsg)
}
}
})
}
})

t.Run("Params", func(t *testing.T) {
for _, literal := range []string{`undefined`, `null`} {
t.Run(literal, func(t *testing.T) {
Expand Down Expand Up @@ -1176,6 +1255,11 @@ func TestSystemTags(t *testing.T) {
tb.Replacer.Replace(`http.get("http://127.0.0.1:56789");`),
tb.Replacer.Replace(`dial tcp 127.0.0.1:56789: ` + connectionRefusedErrorText),
},
{
"error_code",
tb.Replacer.Replace(`http.get("http://127.0.0.1:56789");`),
"1210",
},
}

state.Options.Throw = null.BoolFrom(false)
Expand Down
15 changes: 8 additions & 7 deletions lib/netext/httpext/request.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
debugRequest(state, preq.Req, "DigestRequest")
res, err := client.Do(preq.Req.WithContext(ctx))
debugRequest(state, preq.Req, "DigestResponse")
resp.Error = tracerTransport.errorMsg
resp.ErrorCode = int(tracerTransport.errorCode)
if err != nil {
// Do *not* log errors about the contex being cancelled.
select {
Expand All @@ -208,11 +210,10 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
state.Logger.WithField("error", res).Warn("Digest request failed")
}

if preq.Throw {
if preq.Throw || resp.Error == "" {
return nil, err
}

resp.setError(err)
return resp, nil
}

Expand All @@ -232,6 +233,8 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
debugRequest(state, preq.Req, "Request")
res, resErr := client.Do(preq.Req.WithContext(ctx))
debugResponse(state, res, "Response")
resp.Error = tracerTransport.errorMsg
resp.ErrorCode = int(tracerTransport.errorCode)
if resErr == nil && res != nil {
switch res.Header.Get("Content-Encoding") {
case "deflate":
Expand Down Expand Up @@ -287,17 +290,15 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
Receiving: stats.D(trail.Receiving),
}

if resErr != nil {
resp.setError(resErr)
} else {
if resErr == nil {
if preq.ActiveJar != nil {
if rc := res.Cookies(); len(rc) > 0 {
preq.ActiveJar.SetCookies(res.Request.URL, rc)
}
}

resp.URL = res.Request.URL.String()
resp.setStatusCode(res.StatusCode)
resp.Status = res.StatusCode
resp.Proto = res.Proto

if res.TLS != nil {
Expand Down Expand Up @@ -333,7 +334,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
state.Logger.WithField("error", resErr).Warn("Request Failed")
}

if preq.Throw {
if preq.Throw || resp.Error == "" {
return nil, resErr
}
}
Expand Down
19 changes: 0 additions & 19 deletions lib/netext/httpext/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,25 +102,6 @@ type Response struct {
validatedJSON bool
}

// This should be used instead of setting Error as it will correctly set ErrorCode as well
func (res *Response) setError(err error) {
var errorCode, errorMsg = errorCodeForError(err)
res.ErrorCode = int(errorCode)
if errorMsg == "" {
errorMsg = err.Error()
}
res.Error = errorMsg
}

// This should be used instead of setting Error as it will correctly set ErrorCode as well
func (res *Response) setStatusCode(statusCode int) {
res.Status = statusCode
if statusCode >= 400 && statusCode < 600 {
res.ErrorCode = 1000 + statusCode
// TODO: maybe set the res.Error to some custom message
}
}

func (res *Response) setTLSInfo(tlsState *tls.ConnectionState) {
tlsInfo, oscp := netext.ParseTLSConnState(tlsState)
res.TLSVersion = tlsInfo.Version
Expand Down
16 changes: 13 additions & 3 deletions lib/netext/httpext/transport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type transport struct {
options *lib.Options
tags map[string]string
trail *Trail
errorMsg string
errorCode errCode
tlsInfo netext.TLSInfo
samplesCh chan<- stats.SampleContainer
}
Expand Down Expand Up @@ -90,12 +92,15 @@ func (t *transport) RoundTrip(req *http.Request) (res *http.Response, err error)
resp, err := t.roundTripper.RoundTrip(reqWithTracer)
trail := tracer.Done()
if err != nil {
t.errorCode, t.errorMsg = errorCodeForError(err)
if t.options.SystemTags["error"] {
tags["error"] = err.Error()
tags["error"] = t.errorMsg
}

if t.options.SystemTags["error_code"] {
tags["error_code"] = strconv.Itoa(int(t.errorCode))
}

//TODO: expand/replace this so we can recognize the different non-HTTP
// errors, probably by using a type switch for resErr
if t.options.SystemTags["status"] {
tags["status"] = "0"
}
Expand All @@ -106,6 +111,11 @@ func (t *transport) RoundTrip(req *http.Request) (res *http.Response, err error)
if t.options.SystemTags["status"] {
tags["status"] = strconv.Itoa(resp.StatusCode)
}
if resp.StatusCode >= 400 {
if t.options.SystemTags["error_code"] {
tags["error_code"] = strconv.Itoa(1000 + resp.StatusCode)
}
}
if t.options.SystemTags["proto"] {
tags["proto"] = resp.Proto
}
Expand Down
3 changes: 2 additions & 1 deletion lib/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import (
// DefaultSystemTagList includes all of the system tags emitted with metrics by default.
// Other tags that are not enabled by default include: iter, vu, ocsp_status, ip
var DefaultSystemTagList = []string{
"proto", "subproto", "status", "method", "url", "name", "group", "check", "error", "tls_version",

"proto", "subproto", "status", "method", "url", "name", "group", "check", "error", "error_code", "tls_version",
}

// TagSet is a string to bool map (for lookup efficiency) that is used to keep track
Expand Down

0 comments on commit c77b461

Please sign in to comment.