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

refactor: Avoid param reassignment in group_snippet #303

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 16 additions & 43 deletions resources/group_snippet.js
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite happy with this snippet "polluting" the global namespace, essentially making it impossible to create user-defined variables with names like Client, trimAndRemovePrefix, etc - it's only possible in custom code snippets (and I'm still not sure whether those should have their own scope - let's have a separate discussion on that)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point!
We should definitely not reserve those names 🤔

Original file line number Diff line number Diff line change
@@ -1,24 +1,18 @@
import execution from 'k6/execution'

// saving the original values
const request = http.request
const asyncRequest = http.asyncRequest
const httpRequest = http.request
const httpAsyncRequest = http.asyncRequest

class Client {
// request instruments the http module's request function with the group header
request(method, url, ...args) {
const group = { 'X-k6-group': trimAndRemovePrefix(execution.vu.tags.group) }
args = instrumentArguments(group, ...args)

return request(method, url, ...args)
return httpRequest(method, url, ...instrumentArguments(args))
}

// asyncRequest instruments the http module's asyncRequest function with the group header
async asyncRequest(method, url, ...args) {
const group = { 'X-k6-group': trimAndRemovePrefix(execution.vu.tags.group) }
args = instrumentArguments(group, ...args)

return asyncRequest(method, url, ...args)
return httpAsyncRequest(method, url, ...instrumentArguments(args))
}

del(url, ...args) {
Expand All @@ -45,44 +39,23 @@ class Client {
}

function trimAndRemovePrefix(input) {
// Use the trim method to remove leading and trailing whitespace
let trimmedInput = input.trim()

// Check if the trimmed input starts with "::" and remove it
if (trimmedInput.startsWith('::')) {
trimmedInput = trimmedInput.slice(2)
}

return trimmedInput
return input.trim().replace(/^::/, '')
}

function instrumentArguments(groupName, ...args) {
switch (args.length) {
case 0:
args.push(null)
// fallthrough to add the header
case 1:
// We only received a body argument
args.push({ headers: groupName })
break
default: // this handles 2 and more just in case someone provided more arguments
// We received both a body and a params argument. In the
// event params would be nullish, we'll instantiate
// a new object.
if (args[1] == null) args[1] = {}

let params = args[1]
if (params.headers == null) {
params.headers = {}
}
Object.assign(params.headers, groupName)
break
function instrumentArguments(args) {
const [body = null, params = {}] = args
const groupHeaders = {
'X-k6-group': trimAndRemovePrefix(execution.vu.tags.group),
}

return args
const updatedParams = Object.assign({}, params, {
headers: Object.assign({}, params.headers || {}, groupHeaders),
})
Comment on lines +51 to +53
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that our version of k6 doesn't support the spread operator. It was added to a newer version (grafana/k6#3456)


return [body, updatedParams]
}

function instrumentHTTP(opts) {
function instrumentHTTP(opts = {}) {
const client = new Client(opts)

http.del = client.del.bind(client)
Expand All @@ -96,4 +69,4 @@ function instrumentHTTP(opts) {
http.asyncRequest = client.asyncRequest.bind(client)
}

instrumentHTTP({})
instrumentHTTP()
Loading