Skip to content

Commit

Permalink
Pass span to params helper in Koa hook
Browse files Browse the repository at this point in the history
When the Koa instrumentation request hook is executed, the current
active span is the span from the _previous_ layer. This means that,
for the router layer, the active span is the last middleware in the
middleware chain.

While this is fine functionally, it makes the behaviour of the hook
hard to reason about. This change makes it so that the params are
set in the span passed to the hook, instead of in the active span.

The integration tests are also amended to check for the params in
the router child span.
  • Loading branch information
unflxw committed Nov 3, 2022
1 parent 0c89aeb commit 295858e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,10 @@ export class Client {
dbStatementSerializer: RedisDbStatementSerializer
},
"@opentelemetry/instrumentation-koa": {
requestHook: function (_span, info) {
requestHook: function (span, info) {
if (sendParams && info.layerType === KoaLayerType.ROUTER) {
const queryParams = info.context.request.query
setParams(queryParams)
setParams(queryParams, span)
}
}
},
Expand Down
7 changes: 4 additions & 3 deletions test/koa-mysql/tests/spec/app_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,18 @@
end

describe "GET /get with params" do
it "adds params to the HTTP root span" do
it "adds params to the router child span" do
response = HTTP.get("#{@test_app_url}/get?param1=user&param2=password")
expect(response.status).to eq(200)
expect(Span.root!).to be_http_span_with_route("GET /get")

router_span = Span.find_by_name!("router - /get")

expected_request_parameters = {
"param1" => "user",
"param2" => "password"
}

expect(Span.root!).to match_request_parameters(expected_request_parameters)
expect(router_span).to match_request_parameters(expected_request_parameters)
end
end

Expand Down

0 comments on commit 295858e

Please sign in to comment.