-
Notifications
You must be signed in to change notification settings - Fork 513
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
feat(koa): add requestHook support #1099
feat(koa): add requestHook support #1099
Conversation
export type KoaRequestInfo = { | ||
context: KoaContext; | ||
middlewareLayer: KoaMiddleware; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for noticing. This should be part of the requestHook
. Fixing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! The idea is to replicate the behaviour suggested here: #1091 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yea, i wasn't sure if that's what u meant to do.
Nice :)
11a8b6a
to
7ac4ed2
Compare
Codecov Report
@@ Coverage Diff @@
## main #1099 +/- ##
==========================================
+ Coverage 96.07% 96.36% +0.29%
==========================================
Files 14 19 +5
Lines 892 1046 +154
Branches 191 220 +29
==========================================
+ Hits 857 1008 +151
- Misses 35 38 +3
|
The `requestHook` support allows custom span handling per middleware layer on requests.
7ac4ed2
to
013b46c
Compare
); | ||
|
||
const requestHook = sinon.spy((span: Span, info: KoaRequestInfo) => { | ||
span.setAttribute('http.method', info.context.request.method); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to use the enum SemanticAttributes.HTTP_METHOD
@luismiramirez friendly ping because the lint is failling |
@vmarchaud Linter issues addressed! |
The
requestHook
support allows custom span handling per middlewarelayer on requests.
Which problem is this PR solving?
requestHook
support so users can add custom handling on spans per middleware layer on requests.Short description of the changes
requestHook
is implemented in other instrumentationsrequestHook
user-defined function is called any time a patched middleware runs and if the function exists.Checklist
npm run test-all-versions
for the edited package(s) on the latest commit if applicable.