feat: k6 gRPC middleware #133
Conversation
Adding the baggage to the request context makes it easier for users to pass the baggage along in subsequent requests.
| ctx := r.Context() | ||
| labels := getBaggageLabelsFromContext(ctx) | ||
| if labels == nil { | ||
| lh.innerHandler.ServeHTTP(w, r) | ||
| return | ||
| } |
There was a problem hiding this comment.
I might be mistaken (couldn't find a test case), but it seems that baggageToLabels called from getBaggageLabelsFromContext may return a non-nil empty labels (LabelSet), if the baggage only contains non-k6 labels. I would move zero length check to baggageToLabels, so that we do this after filtering, just in case
There was a problem hiding this comment.
Hmm I'm not sure I completely follow.
You are right that if a baggage header came in but all members were filtered out, we'd end up with a non-nil, empty LabelSet at this point. In which case, we'd fall through and try set labels in the profile context. I fixed that issue in 8477ab7. But I'm not sure I addressed your original concern.
Previously we would still try set profiler labels even if incoming baggage had no k6 metadata. This skips setting profiler labels with no k6 metadata.
| @@ -15,19 +15,32 @@ jobs: | |||
| go: ['1.18', '1.19', '1.20', '1.21', '1.22', '1.23', 'tip'] | |||
| steps: | |||
| - name: Checkout | |||
| uses: actions/checkout@v3 | |||
| uses: actions/checkout@v4 | |||
There was a problem hiding this comment.
All these actions upgrades here are in response to https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/.
There was a problem hiding this comment.
Nit: could we update the other 2 workflows with checkout@v4?
There was a problem hiding this comment.
Fixed in 3349e0c
| - name: Run go/mod | ||
| run: make go/mod && git diff --exit-code | ||
|
|
||
| - name: Run k6 go/mod |
There was a problem hiding this comment.
Also adds extra steps to build and test k6 as an entirely separate module.
| @@ -1,5 +1,5 @@ | |||
| GO_VERSION_PRE20 := $(shell go version | awk '{print $$3}' | awk -F '.' '{print ($$1 == "go1" && int($$2) < 20)}') | |||
| TEST_PACKAGES := ./... ./godeltaprof/compat/... ./godeltaprof/... ./x/k6/... | |||
| TEST_PACKAGES := ./... ./godeltaprof/compat/... ./godeltaprof/... | |||
There was a problem hiding this comment.
We don't test the k6 module as part of the main SDK anymore.
| @@ -1,8 +1,7 @@ | |||
| go 1.19 | |||
| go 1.18 | |||
There was a problem hiding this comment.
I'm not sure why we ended up on go1.19 here, but all the submodules are targeting go1.18, so I'm moving the go.work version to match.
x/k6/Makefile
Outdated
There was a problem hiding this comment.
Make targets to build and test the k6 extensions.
| @@ -15,19 +15,32 @@ jobs: | |||
| go: ['1.18', '1.19', '1.20', '1.21', '1.22', '1.23', 'tip'] | |||
| steps: | |||
| - name: Checkout | |||
| uses: actions/checkout@v3 | |||
| uses: actions/checkout@v4 | |||
There was a problem hiding this comment.
Nit: could we update the other 2 workflows with checkout@v4?
Adds a gRPC variant of the HTTP middleware introduced in this PR: #131
It's important to note that this PR also makes the k6 module independent of the rest of the SDK. I removed it from the root
go.workfile and create a separatego.workfile in thex/k6directory. I also gave the k6 module its own makefile. This way we can build and test it on the last two versions of Go (andgotip) without imposing that same strict version requirement on the rest of the SDK.