Add checkIfShouldTrace option to OpenTelemetry plugin
#51
Conversation
WalkthroughIntroduces a per-request tracing gate to the OpenTelemetry integration by adding an optional checkIfShouldTrace(req) option. The opentelemetry wrapper now conditionally applies tracing based on this predicate, returning the original handler unwrapped when the check returns false. JSDoc added for the new option. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as Server Route Handler
participant OT as OpenTelemetry Wrapper
participant P as checkIfShouldTrace(req)
C->>S: HTTP Request
activate S
S->>OT: Invoke wrapped handler
activate OT
OT->>P: Evaluate predicate
alt Should trace
OT->>OT: Start span
OT->>S: Call original handler (traced)
S-->>OT: Response/Result
OT->>OT: End span
OT-->>C: Response
else Skip tracing
OT-->>S: Return original handler (no tracing)
S-->>C: Response
end
deactivate OT
deactivate S
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/index.ts (1)
61-67: Clarify default behavior in JSDoc and constraintsState explicitly that when omitted, all requests are traced (default true). Also note that the predicate should be fast and must not consume the request body, as it runs before parsing.
Apply this doc tweak:
/** * Optional function to determine whether a given request should be traced. * * @param req - The incoming request object to evaluate. - * @returns A boolean indicating whether tracing should be enabled for this request. + * @returns A boolean indicating whether tracing should be enabled for this request. + * If not provided, tracing is enabled for all requests by default. + * Keep this check lightweight and avoid consuming the request body. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (1)
test/test-setup.ts (1)
req(37-38)
🔇 Additional comments (1)
src/index.ts (1)
223-225: Option plumbed correctlyDestructuring
checkIfShouldTraceinto the plugin factory signature looks good.
| return new Elysia({ | ||
| name: '@elysia/opentelemetry' | ||
| }) | ||
| .wrap((fn, request) => { | ||
| const shouldTrace = checkIfShouldTrace | ||
| ? checkIfShouldTrace(request) | ||
| : true | ||
|
|
||
| if (!shouldTrace) { | ||
| return fn | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Suppress downstream auto-instrumentation when tracing is disabled + harden predicate call
Returning fn skips the root “request” span, but downstream auto-instrumentations (fetch/http, db, etc.) may still create spans, producing noise for routes you intended to exclude. Suppress instrumentation in the context for the request when shouldTrace is false. Also guard the predicate to avoid crashing the request if it throws; fail-open to tracing for debuggability.
Apply within this hunk:
- .wrap((fn, request) => {
- const shouldTrace = checkIfShouldTrace
- ? checkIfShouldTrace(request)
- : true
+ .wrap((fn, request) => {
+ let shouldTrace = true
+ if (checkIfShouldTrace) {
+ try {
+ shouldTrace = !!checkIfShouldTrace(request)
+ } catch {
+ // Fail-open: keep tracing enabled if predicate throws
+ shouldTrace = true
+ }
+ }
- if (!shouldTrace) {
- return fn
- }
+ if (!shouldTrace) {
+ // Prevent any spans from being created for this request
+ const suppressed = suppressInstrumentation(otelContext.active())
+ return otelContext.bind(suppressed, fn)
+ }And add the required import at the top of the file:
+import { suppressInstrumentation } from '@opentelemetry/core'If @opentelemetry/core isn’t already a runtime dependency, add it to dependencies (not just via transitive SDK).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new Elysia({ | |
| name: '@elysia/opentelemetry' | |
| }) | |
| .wrap((fn, request) => { | |
| const shouldTrace = checkIfShouldTrace | |
| ? checkIfShouldTrace(request) | |
| : true | |
| if (!shouldTrace) { | |
| return fn | |
| } | |
| // Add at the top of src/index.ts | |
| import { suppressInstrumentation } from '@opentelemetry/core' | |
| … | |
| return new Elysia({ | |
| name: '@elysia/opentelemetry' | |
| }) | |
| .wrap((fn, request) => { | |
| // Harden the predicate call | |
| let shouldTrace = true | |
| if (checkIfShouldTrace) { | |
| try { | |
| shouldTrace = !!checkIfShouldTrace(request) | |
| } catch { | |
| // Fail-open: keep tracing enabled if predicate throws | |
| shouldTrace = true | |
| } | |
| } | |
| // If tracing is disabled, suppress all downstream instrumentation | |
| if (!shouldTrace) { | |
| const suppressed = suppressInstrumentation(otelContext.active()) | |
| return otelContext.bind(suppressed, fn) | |
| } | |
| // …rest of the original handler… | |
| }) |
This PR introduces a new optional function
checkIfShouldTracetoElysiaOpenTelemetryOptions.It allows users to filter out specific requests (e.g.,
/health,/metrics) that are typically not useful to trace, reducing noise and overhead in telemetry data.Changes
Extended
ElysiaOpenTelemetryOptionswith a new property:checkIfShouldTrace?: (req: Request) => booleanWrapped the tracing logic in a conditional that evaluates this function before starting a span.
Updated Interface
Example Usage
Benefits
/health,/metrics, or static assets.Summary by CodeRabbit