Skip to content

Add checkIfShouldTrace option to OpenTelemetry plugin #51

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add checkIfShouldTrace option to OpenTelemetry plugin #51

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ type OpenTeleMetryOptions = NonNullable<
*/
export interface ElysiaOpenTelemetryOptions extends OpenTeleMetryOptions {
contextManager?: ContextManager
/**
* 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.
*/
checkIfShouldTrace?: (req: Request) => boolean
}

export type ActiveSpanArgs<
Expand Down Expand Up @@ -213,6 +220,7 @@ export const opentelemetry = ({
serviceName = 'Elysia',
instrumentations,
contextManager,
checkIfShouldTrace,
...options
}: ElysiaOpenTelemetryOptions = {}) => {
let tracer = trace.getTracer(serviceName)
Expand Down Expand Up @@ -265,9 +273,18 @@ export const opentelemetry = ({
// }
}

return new Elysia({
name: '@elysia/opentelemetry'
}).wrap((fn, request) => {
return new Elysia({
name: '@elysia/opentelemetry'
})
.wrap((fn, request) => {
const shouldTrace = checkIfShouldTrace
? checkIfShouldTrace(request)
: true

if (!shouldTrace) {
return fn
}

Comment on lines +276 to +287
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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…
})

let headers
if (headerHasToJSON) {
// @ts-ignore bun only
Expand Down