Add a way to mark http requests as failed

This is done through running a callback on every request before emitting
the metrics. Currently only a built-in metric looking at good statuses
is possible, but a possibility for future JS based callbacks is left
open.

The implementation specifically makes it hard to figure out anything
about the returned callback from JS and tries not to change any other
code so it makes it easier for future implementation, but instead tries
to do the bare minimum without imposing any limitations on the future
work.

Additionally because it turned out to be easy, setting the callback to
null will make the http library to neither tag requests with `expected_response`
nor emit the new `http_req_failed` metric, essentially giving users a way to
go back to the previous behaviour.

part of #1828

core/engine.go

@@ -108,6 +108,20 @@ func NewEngine(
 		e.submetrics[parent] = append(e.submetrics[parent], sm)
 	}
 
+	// TODO: refactor this out of here when https://github.com/loadimpact/k6/issues/1832 lands and
+	// there is a better way to enable a metric with tag
+	if opts.SystemTags.Has(stats.TagExpectedResponse) {
+		for _, name := range []string{
+			"http_req_duration{expected_response:true}",
+		} {
+			if _, ok := e.thresholds[name]; ok {
+				continue
+			}
+			parent, sm := stats.NewSubmetric(name)
+			e.submetrics[parent] = append(e.submetrics[parent], sm)
+		}
+	}
+
 	return e, nil
 }
 

core/local/local_test.go

@@ -329,12 +329,13 @@ func TestExecutionSchedulerSystemTags(t *testing.T) {
 	}()
 
 	expCommonTrailTags := stats.IntoSampleTags(&map[string]string{
-		"group":  "",
-		"method": "GET",
-		"name":   sr("HTTPBIN_IP_URL/"),
-		"url":    sr("HTTPBIN_IP_URL/"),
-		"proto":  "HTTP/1.1",
-		"status": "200",
+		"group":             "",
+		"method":            "GET",
+		"name":              sr("HTTPBIN_IP_URL/"),
+		"url":               sr("HTTPBIN_IP_URL/"),
+		"proto":             "HTTP/1.1",
+		"status":            "200",
+		"expected_response": "true",
 	})
 	expTrailPVUTagsRaw := expCommonTrailTags.CloneTags()
 	expTrailPVUTagsRaw["scenario"] = "per_vu_test"

js/modules/k6/http/http.go

@@ -73,6 +73,8 @@ func (g *GlobalHTTP) NewVUModule() interface{} { // this here needs to return in
 		OCSP_REASON_REMOVE_FROM_CRL:        netext.OCSP_REASON_REMOVE_FROM_CRL,
 		OCSP_REASON_PRIVILEGE_WITHDRAWN:    netext.OCSP_REASON_PRIVILEGE_WITHDRAWN,
 		OCSP_REASON_AA_COMPROMISE:          netext.OCSP_REASON_AA_COMPROMISE,
+
+		responseCallback: defaultExpectedStatuses.match,
 	}
 }
 
@@ -97,6 +99,8 @@ type HTTP struct {
 	OCSP_REASON_REMOVE_FROM_CRL        string `js:"OCSP_REASON_REMOVE_FROM_CRL"`
 	OCSP_REASON_PRIVILEGE_WITHDRAWN    string `js:"OCSP_REASON_PRIVILEGE_WITHDRAWN"`
 	OCSP_REASON_AA_COMPROMISE          string `js:"OCSP_REASON_AA_COMPROMISE"`
+
+	responseCallback func(int) bool
 }
 
 func (*HTTP) XCookieJar(ctx *context.Context) *HTTPCookieJar {

js/modules/k6/http/request.go

@@ -134,12 +134,14 @@ func (h *HTTP) parseRequest(
 			URL:    reqURL.GetURL(),
 			Header: make(http.Header),
 		},
-		Timeout:   60 * time.Second,
-		Throw:     state.Options.Throw.Bool,
-		Redirects: state.Options.MaxRedirects,
-		Cookies:   make(map[string]*httpext.HTTPRequestCookie),
-		Tags:      make(map[string]string),
+		Timeout:          60 * time.Second,
+		Throw:            state.Options.Throw.Bool,
+		Redirects:        state.Options.MaxRedirects,
+		Cookies:          make(map[string]*httpext.HTTPRequestCookie),
+		Tags:             make(map[string]string),
+		ResponseCallback: h.responseCallback,
 	}
+
 	if state.Options.DiscardResponseBodies.Bool {
 		result.ResponseType = httpext.ResponseTypeNone
 	} else {
@@ -349,6 +351,15 @@ func (h *HTTP) parseRequest(
 					return nil, err
 				}
 				result.ResponseType = responseType
+			case "responseCallback":
+				v := params.Get(k).Export()
+				if v == nil {
+					result.ResponseCallback = nil
+				} else if c, ok := v.(*expectedStatuses); ok {
+					result.ResponseCallback = c.match
+				} else {
+					return nil, fmt.Errorf("unsupported responseCallback")
+				}
 			}
 		}
 	}

js/modules/k6/http/request_test.go

@@ -81,6 +81,7 @@ func TestRunES6String(t *testing.T) {
 	})
 }
 
+// TODO replace this with the Single version
 func assertRequestMetricsEmitted(t *testing.T, sampleContainers []stats.SampleContainer, method, url, name string, status int, group string) {
 	if name == "" {
 		name = url
@@ -130,6 +131,29 @@ func assertRequestMetricsEmitted(t *testing.T, sampleContainers []stats.SampleCo
 	assert.True(t, seenReceiving, "url %s didn't emit Receiving", url)
 }
 
+func assertRequestMetricsEmittedSingle(t *testing.T, sampleContainer stats.SampleContainer, expectedTags map[string]string, metrics []*stats.Metric, callback func(sample stats.Sample)) {
+	t.Helper()
+
+	metricMap := make(map[string]bool, len(metrics))
+	for _, m := range metrics {
+		metricMap[m.Name] = false
+	}
+	for _, sample := range sampleContainer.GetSamples() {
+		tags := sample.Tags.CloneTags()
+		v, ok := metricMap[sample.Metric.Name]
+		assert.True(t, ok, "unexpected metric %s", sample.Metric.Name)
+		assert.False(t, v, "second metric %s", sample.Metric.Name)
+		metricMap[sample.Metric.Name] = true
+		assert.EqualValues(t, expectedTags, tags, "%s", tags)
+		if callback != nil {
+			callback(sample)
+		}
+	}
+	for k, v := range metricMap {
+		assert.True(t, v, "didn't emit %s", k)
+	}
+}
+
 func newRuntime(
 	t testing.TB,
 ) (*httpmultibin.HTTPMultiBin, *lib.State, chan stats.SampleContainer, *goja.Runtime, *context.Context) {
@@ -1945,26 +1969,28 @@ func TestRedirectMetricTags(t *testing.T) {
 
 	checkTags := func(sc stats.SampleContainer, expTags map[string]string) {
 		allSamples := sc.GetSamples()
-		assert.Len(t, allSamples, 8)
+		assert.Len(t, allSamples, 9)
 		for _, s := range allSamples {
 			assert.Equal(t, expTags, s.Tags.CloneTags())
 		}
 	}
 	expPOSTtags := map[string]string{
-		"group":  "",
-		"method": "POST",
-		"url":    sr("HTTPBIN_URL/redirect/post"),
-		"name":   sr("HTTPBIN_URL/redirect/post"),
-		"status": "301",
-		"proto":  "HTTP/1.1",
+		"group":             "",
+		"method":            "POST",
+		"url":               sr("HTTPBIN_URL/redirect/post"),
+		"name":              sr("HTTPBIN_URL/redirect/post"),
+		"status":            "301",
+		"proto":             "HTTP/1.1",
+		"expected_response": "true",
 	}
 	expGETtags := map[string]string{
-		"group":  "",
-		"method": "GET",
-		"url":    sr("HTTPBIN_URL/get"),
-		"name":   sr("HTTPBIN_URL/get"),
-		"status": "200",
-		"proto":  "HTTP/1.1",
+		"group":             "",
+		"method":            "GET",
+		"url":               sr("HTTPBIN_URL/get"),
+		"name":              sr("HTTPBIN_URL/get"),
+		"status":            "200",
+		"proto":             "HTTP/1.1",
+		"expected_response": "true",
 	}
 	checkTags(<-samples, expPOSTtags)
 	checkTags(<-samples, expGETtags)

js/modules/k6/http/response_test.go

@@ -55,6 +55,7 @@ const testGetFormHTML = `
 	</form>
 </body>
 `
+
 const jsonData = `{"glossary": {
     "friends": [
       {"first": "Dale", "last": "Murphy", "age": 44},

js/runner.go

@@ -179,7 +179,7 @@ func (r *Runner) newVU(id int64, samplesOut chan<- stats.SampleContainer) (*VU,
 	}
 
 	tlsConfig := &tls.Config{
-		InsecureSkipVerify: r.Bundle.Options.InsecureSkipTLSVerify.Bool,
+		InsecureSkipVerify: r.Bundle.Options.InsecureSkipTLSVerify.Bool, //nolint:gosec
 		CipherSuites:       cipherSuites,
 		MinVersion:         uint16(tlsVersions.Min),
 		MaxVersion:         uint16(tlsVersions.Max),
@@ -244,6 +244,7 @@ func (r *Runner) newVU(id int64, samplesOut chan<- stats.SampleContainer) (*VU,
 	return vu, nil
 }
 
+// Setup runs the setup function if there is one and sets the setupData to the returned value
 func (r *Runner) Setup(ctx context.Context, out chan<- stats.SampleContainer) error {
 	setupCtx, setupCancel := context.WithTimeout(ctx, r.getTimeoutFor(consts.SetupFn))
 	defer setupCancel()

lib/metrics/metrics.go

@@ -24,7 +24,7 @@ import (
 	"github.com/loadimpact/k6/stats"
 )
 
-//TODO: refactor this, using non thread-safe global variables seems like a bad idea for various reasons...
+// TODO: refactor this, using non thread-safe global variables seems like a bad idea for various reasons...
 
 //nolint:gochecknoglobals
 var (
@@ -42,6 +42,7 @@ var (
 
 	// HTTP-related.
 	HTTPReqs              = stats.New("http_reqs", stats.Counter)
+	HTTPReqFailed         = stats.New("http_req_failed", stats.Rate)
 	HTTPReqDuration       = stats.New("http_req_duration", stats.Trend, stats.Time)
 	HTTPReqBlocked        = stats.New("http_req_blocked", stats.Trend, stats.Time)
 	HTTPReqConnecting     = stats.New("http_req_connecting", stats.Trend, stats.Time)

lib/netext/httpext/request.go

@@ -103,18 +103,19 @@ type Request struct {
 
 // ParsedHTTPRequest a represantion of a request after it has been parsed from a user script
 type ParsedHTTPRequest struct {
-	URL          *URL
-	Body         *bytes.Buffer
-	Req          *http.Request
-	Timeout      time.Duration
-	Auth         string
-	Throw        bool
-	ResponseType ResponseType
-	Compressions []CompressionType
-	Redirects    null.Int
-	ActiveJar    *cookiejar.Jar
-	Cookies      map[string]*HTTPRequestCookie
-	Tags         map[string]string
+	URL              *URL
+	Body             *bytes.Buffer
+	Req              *http.Request
+	Timeout          time.Duration
+	Auth             string
+	Throw            bool
+	ResponseType     ResponseType
+	ResponseCallback func(int) bool
+	Compressions     []CompressionType
+	Redirects        null.Int
+	ActiveJar        *cookiejar.Jar
+	Cookies          map[string]*HTTPRequestCookie
+	Tags             map[string]string
 }
 
 // Matches non-compliant io.Closer implementations (e.g. zstd.Decoder)
@@ -139,7 +140,7 @@ func (r readCloser) Close() error {
 }
 
 func stdCookiesToHTTPRequestCookies(cookies []*http.Cookie) map[string][]*HTTPRequestCookie {
-	var result = make(map[string][]*HTTPRequestCookie, len(cookies))
+	result := make(map[string][]*HTTPRequestCookie, len(cookies))
 	for _, cookie := range cookies {
 		result[cookie.Name] = append(result[cookie.Name],
 			&HTTPRequestCookie{Name: cookie.Name, Value: cookie.Value})
@@ -249,7 +250,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
 		}
 	}
 
-	tracerTransport := newTransport(ctx, state, tags)
+	tracerTransport := newTransport(ctx, state, tags, preq.ResponseCallback)
 	var transport http.RoundTripper = tracerTransport
 
 	// Combine tags with common log fields
@@ -269,8 +270,25 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
 	}
 
 	if preq.Auth == "digest" {
+		// In both digest and NTLM it is expected that the first response will be 401
+		// this does mean that a non 401 response will be marked as failed/unexpected
+		if tracerTransport.responseCallback != nil {
+			originalResponseCallback := tracerTransport.responseCallback
+			tracerTransport.responseCallback = func(status int) bool {
+				tracerTransport.responseCallback = originalResponseCallback
+				return status == 401
+			}
+		}
 		transport = digestTransport{originalTransport: transport}
 	} else if preq.Auth == "ntlm" {
+		if tracerTransport.responseCallback != nil {
+			originalResponseCallback := tracerTransport.responseCallback
+			tracerTransport.responseCallback = func(status int) bool {
+				tracerTransport.responseCallback = originalResponseCallback
+				// ntlm is connection-level based so we could've already authorized the connection and to now reuse it
+				return status == 401 || originalResponseCallback(status)
+			}
+		}
 		transport = ntlmssp.Negotiator{RoundTripper: transport}
 	}
 
@@ -381,7 +399,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
 // SetRequestCookies sets the cookies of the requests getting those cookies both from the jar and
 // from the reqCookies map. The Replace field of the HTTPRequestCookie will be taken into account
 func SetRequestCookies(req *http.Request, jar *cookiejar.Jar, reqCookies map[string]*HTTPRequestCookie) {
-	var replacedCookies = make(map[string]struct{})
+	replacedCookies := make(map[string]struct{})
 	for key, reqCookie := range reqCookies {
 		req.AddCookie(&http.Cookie{Name: key, Value: reqCookie.Value})
 		if reqCookie.Replace {

lib/netext/httpext/tracer.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/loadimpact/k6/lib/metrics"
 	"github.com/loadimpact/k6/stats"
+	"gopkg.in/guregu/null.v3"
 )
 
 // A Trail represents detailed information about an HTTP request.
@@ -53,6 +54,7 @@ type Trail struct {
 	ConnReused     bool
 	ConnRemoteAddr net.Addr
 
+	Failed null.Bool
 	// Populated by SaveSamples()
 	Tags    *stats.SampleTags
 	Samples []stats.Sample