-
Notifications
You must be signed in to change notification settings - Fork 35
feat: add tracing channels for fetch and middleware handlers #141
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
base: main
Are you sure you want to change the base?
Conversation
0ca8e01 to
4c49d41
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
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.
We should probably split addons as an opt-in plugin that modifies registred middleware/routes
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 sorry, I just saw this, so something like this?
import { serve } from "srvx";
import { tracingPlugin } from 'srvx/tracing'
const server = serve({
fetch: () => {
// ...
},
plugins: [tracingPlugin],
});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.
Yes!
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.
All done!
a89a68d to
c8e96a0
Compare
This PR adds tracing instrumentation for middleware and fetch handlers using Node.js diagnostics/tracing channels, enabling integration with observability tools like OpenTelemetry and Sentry.
Implementation
srvx.middlewareandsrvx.fetchtracing events with full lifecycle hooks (start, end, asyncStart, asyncEnd, error)I think this has to be available out of the box, so it "just works" with SDK providers rather than OPT in.
Example usage
Span Relationships
Something I noticed is since we have an onion effect here with each middleware being able to wait for the response/next, then it means if the SDK provider isn't careful, they might end up creating middleware spans as children of one another rather than siblings.
Now this can be fine since it is technically correct from an execution standpoint, but each provider can handle this manually when subscribing to those diagnostic events, and they can manually unscope each span from the previous one to get the desired effect.
TODO: