-
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(fastify): add requestHook support #1255
feat(fastify): add requestHook support #1255
Conversation
a6c063f
to
679c56b
Compare
The `requestHook` config option allows custom span handling per request layer.
679c56b
to
2549d4c
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1255 +/- ##
==========================================
+ Coverage 96.08% 96.09% +0.01%
==========================================
Files 14 19 +5
Lines 893 1127 +234
Branches 191 233 +42
==========================================
+ Hits 858 1083 +225
- Misses 35 44 +9
|
plugins/node/opentelemetry-instrumentation-fastify/src/types.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/src/types.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
- Add a test for when an exception is thrown from inside the hook - Export the types - Don't use Fastify core types so the package doesn't need to be a dependency - Rephrase docs to be clearer about when the request hook is executed - Remove sinon - Use proper component to log hook errors
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 addressing everything.
added few nit comments
plugins/node/opentelemetry-instrumentation-fastify/src/types.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/test/instrumentation.test.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-fastify/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
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.
LGTM 🥇
- Fix types in docs - Make FastifyRequestInfo an interface - Use SemanticAttributes enums instead of plain strings - Remove extra diag error call - Some lint fixes
7be51f9
to
44b7c5f
Compare
The
requestHook
config option allows custom span handling per request layer.Which problem is this PR solving?
requestHook
support so users can add custom handling on spans.Short description of the changes
requestHook
is implemented in web instrumentationsrequestHook
user-defined function is called any time_hookPreHandler()
is executed