Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method field on Match object for page.on('metric') #1490

Merged
merged 12 commits into from
Oct 17, 2024
3 changes: 2 additions & 1 deletion common/frame_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"
"runtime"
"strings"
"sync"
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions common/network_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion common/network_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
26 changes: 24 additions & 2 deletions common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"net/http"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
ankur22 marked this conversation as resolved.
Show resolved Hide resolved

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)
Expand All @@ -447,15 +468,16 @@ 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
}

var newTagName string
var urlMatched bool
em := &MetricEvent{
url: url,
url: url,
method: method,
}

p.eventHandlersMu.RLock()
Expand Down
2 changes: 1 addition & 1 deletion examples/pageon-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'},
]
});
});
Expand Down
94 changes: 89 additions & 5 deletions tests/page_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
Expand All @@ -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)
Expand Down
Loading