diff --git a/common/frame_session.go b/common/frame_session.go index f669e797f..7ee63ce72 100644 --- a/common/frame_session.go +++ b/common/frame_session.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "runtime" "strings" "sync" @@ -355,7 +356,7 @@ func (fs *FrameSession) parseAndEmitWebVitalMetric(object string) error { state := fs.vu.State() tags := state.Tags.GetCurrentValues().Tags if state.Options.SystemTags.Has(k6metrics.TagURL) { - tags = handleURLTag(fs.page, wv.URL, tags) + tags = handleURLTag(fs.page, wv.URL, http.MethodGet, tags) } tags = tags.With("rating", wv.Rating) diff --git a/common/network_manager.go b/common/network_manager.go index 1daba3054..e550f8fc3 100644 --- a/common/network_manager.go +++ b/common/network_manager.go @@ -58,7 +58,7 @@ func (c *Credentials) Parse(ctx context.Context, credentials sobek.Value) error } type metricInterceptor interface { - urlTagName(urlTag string) (string, bool) + urlTagName(urlTag string, method string) (string, bool) } // NetworkManager manages all frames in HTML document. @@ -192,7 +192,7 @@ func (m *NetworkManager) emitRequestMetrics(req *Request) { tags = tags.With("method", req.method) } if state.Options.SystemTags.Has(k6metrics.TagURL) { - tags = handleURLTag(m.mi, req.URL(), tags) + tags = handleURLTag(m.mi, req.URL(), req.method, tags) } k6metrics.PushIfNotDone(m.vu.Context(), state.Samples, k6metrics.ConnectedSamples{ @@ -245,7 +245,7 @@ func (m *NetworkManager) emitResponseMetrics(resp *Response, req *Request) { tags = tags.With("method", req.method) } if state.Options.SystemTags.Has(k6metrics.TagURL) { - tags = handleURLTag(m.mi, url, tags) + tags = handleURLTag(m.mi, url, req.method, tags) } if state.Options.SystemTags.Has(k6metrics.TagIP) { tags = tags.With("ip", ipAddress) @@ -292,8 +292,8 @@ func (m *NetworkManager) emitResponseMetrics(resp *Response, req *Request) { // handleURLTag will check if the url tag needs to be grouped by testing // against user supplied regex. If there's a match a user supplied name will // be used instead of the url for the url tag, otherwise the url will be used. -func handleURLTag(mi metricInterceptor, url string, tags *k6metrics.TagSet) *k6metrics.TagSet { - if newTagName, urlMatched := mi.urlTagName(url); urlMatched { +func handleURLTag(mi metricInterceptor, url string, method string, tags *k6metrics.TagSet) *k6metrics.TagSet { + if newTagName, urlMatched := mi.urlTagName(url, method); urlMatched { tags = tags.With("url", newTagName) tags = tags.With("name", newTagName) return tags diff --git a/common/network_manager_test.go b/common/network_manager_test.go index 7aa0e385d..60702d6a2 100644 --- a/common/network_manager_test.go +++ b/common/network_manager_test.go @@ -211,7 +211,7 @@ func TestOnRequestPausedBlockedIPs(t *testing.T) { type MetricInterceptorMock struct{} -func (m *MetricInterceptorMock) urlTagName(_ string) (string, bool) { +func (m *MetricInterceptorMock) urlTagName(_ string, _ string) (string, bool) { return "", false } diff --git a/common/page.go b/common/page.go index 45b6a6abf..0ad6a56bd 100644 --- a/common/page.go +++ b/common/page.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "net/http" "strings" "sync" "time" @@ -384,6 +385,8 @@ type MetricEvent struct { // The URL value from the metric's url tag. It will be used to match // against the URL grouping regexs. url string + // The method of the request made to the URL. + method string // When a match is found this userProvidedURLTagName field should be updated. userProvidedURLTagName string @@ -406,6 +409,8 @@ type TagMatches struct { type Match struct { // This is a regex that will be compared against the existing url tag. URLRegEx string `js:"url"` + // This is the request method to match on. + Method string `js:"method"` } // K6BrowserCheckRegEx is a function that will be used to check the URL tag @@ -421,6 +426,22 @@ func (e *MetricEvent) Tag(matchesRegex K6BrowserCheckRegEx, matches TagMatches) } for _, m := range matches.Matches { + // Validate the request method type if it has been assigned in a Match. + method := strings.TrimSpace(m.Method) + if method != "" { + method = strings.ToUpper(method) + switch method { + case http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete, http.MethodPatch, + http.MethodHead, http.MethodOptions, http.MethodConnect, http.MethodTrace: + default: + return fmt.Errorf("method %q is invalid", m.Method) + } + + if method != e.method { + continue + } + } + // matchesRegex is a function that will perform the regex test in the Sobek // runtime. matched, err := matchesRegex(m.URLRegEx, e.url) @@ -447,7 +468,7 @@ func (e *MetricEvent) Tag(matchesRegex K6BrowserCheckRegEx, matches TagMatches) // `page.on('metric')`. The user will need to use `Tag` to supply the // url regexes and the matching is done from within there. If a match is found, // the supplied name is returned back upstream to the caller of urlTagName. -func (p *Page) urlTagName(url string) (string, bool) { +func (p *Page) urlTagName(url string, method string) (string, bool) { if !hasPageOnHandler(p, EventPageMetricCalled) { return "", false } @@ -455,7 +476,8 @@ func (p *Page) urlTagName(url string) (string, bool) { var newTagName string var urlMatched bool em := &MetricEvent{ - url: url, + url: url, + method: method, } p.eventHandlersMu.RLock() diff --git a/examples/pageon-metric.js b/examples/pageon-metric.js index a804654b4..fdf099097 100644 --- a/examples/pageon-metric.js +++ b/examples/pageon-metric.js @@ -20,7 +20,7 @@ export default async function() { metric.tag({ name:'test', matches: [ - {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/}, + {url: /^https:\/\/test\.k6\.io\/\?q=[0-9a-z]+$/, method: 'GET'}, ] }); }); diff --git a/tests/page_test.go b/tests/page_test.go index ac84037c4..88472fc31 100644 --- a/tests/page_test.go +++ b/tests/page_test.go @@ -1910,9 +1910,11 @@ func TestPageOnMetric(t *testing.T) { } tests := []struct { - name string - fun string - want string + name string + fun string + want string + wantRegex string + wantErr string }{ { // Just a single page.on. @@ -1994,6 +1996,75 @@ func TestPageOnMetric(t *testing.T) { });`, want: "ping-4", }, + { + // With method field GET, which is the correct method for the request. + name: "with_method", + fun: `page.on('metric', (metric) => { + metric.tag({ + name:'ping-1', + matches: [ + {url: /^http:\/\/127\.0\.0\.1\:[0-9]+\/ping\?h=[0-9a-z]+$/, method: 'GET'}, + ] + }); + });`, + want: "ping-1", + }, + { + // With method field " get ", which is to ensure it is internally + // converted to "GET" before comparing. + name: "lowercase_needs_trimming", + fun: `page.on('metric', (metric) => { + metric.tag({ + name:'ping-1', + matches: [ + {url: /^http:\/\/127\.0\.0\.1\:[0-9]+\/ping\?h=[0-9a-z]+$/, method: ' get '}, + ] + }); + });`, + want: "ping-1", + }, + { + // When supplying the wrong request method (POST) when it should be GET. + // In this case the URLs aren't grouped. + name: "wrong_method_should_skip_method_comparison", + fun: `page.on('metric', (metric) => { + metric.tag({ + name:'ping-1', + matches: [ + {url: /^http:\/\/127\.0\.0\.1\:[0-9]+\/ping\?h=[0-9a-z]+$/, method: 'POST'}, + ] + }); + });`, + wantRegex: `http://127\.0\.0\.1:[0-9]+/ping\?h=[0-9a-z]+`, + }, + { + // We should get an error back when the name is invalid (empty string) + name: "with_invalid_name", + fun: `page.on('metric', (metric) => { + metric.tag({ + name:' ', + matches: [ + {url: /^http:\/\/127\.0\.0\.1\:[0-9]+\/ping\?h=[0-9a-z]+$/, method: 'GET'}, + ] + }); + });`, + wantRegex: `http://127\.0\.0\.1:[0-9]+/ping\?h=[0-9a-z]+`, + wantErr: `name " " is invalid`, + }, + { + // We should get an error back when the method is invalid. + name: "with_invalid_name", + fun: `page.on('metric', (metric) => { + metric.tag({ + name:'ping-1', + matches: [ + {url: /^http:\/\/127\.0\.0\.1\:[0-9]+\/ping\?h=[0-9a-z]+$/, method: 'foo'}, + ] + }); + });`, + wantRegex: `http://127\.0\.0\.1:[0-9]+/ping\?h=[0-9a-z]+`, + wantErr: `method "foo" is invalid`, + }, } for _, tt := range tests { @@ -2032,7 +2103,11 @@ func TestPageOnMetric(t *testing.T) { // Url shouldn't contain any of the hash values, and should // instead take the name that was supplied in the Tag // function on metric in page.on. - assert.Equal(t, tt.want, u) + if tt.wantRegex != "" { + assert.Regexp(t, tt.wantRegex, u) + } else { + assert.Equal(t, tt.want, u) + } foundAmended.Add(1) } @@ -2045,7 +2120,7 @@ func TestPageOnMetric(t *testing.T) { // Some of the business logic is in the mapping layer unfortunately. // To test everything is wried up correctly, we're required to work // with RunPromise. - got := vu.RunPromise(t, ` + gv, err := vu.RunAsync(t, ` const page = await browser.newPage() %s @@ -2054,6 +2129,15 @@ func TestPageOnMetric(t *testing.T) { await page.close() `, tt.fun, tb.url("/home")) + + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + } + + got := k6test.ToPromise(t, gv) + assert.True(t, got.Result().Equals(sobek.Null())) close(samples)