Skip to content

Commit 4d975b1

Browse files
authored
fix: external traces now respect parent sampling, and prevent broken traces when there is no external trace context (#2395)
* fix: external traces now respect parent sampling, and prevent broken traces when there is no external trace context * Add changeset * improve trace flag handling and better internal host checking * the traceFlags are now being properly passed through as a number
1 parent 4c4b774 commit 4d975b1

File tree

10 files changed

+148
-21
lines changed

10 files changed

+148
-21
lines changed

.changeset/violet-llamas-roll.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"trigger.dev": patch
3+
---
4+
5+
fix: external traces now respect parent sampling, and prevent broken traces when there is no external trace context

apps/webapp/app/runEngine/services/triggerTask.server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,8 @@ export class RunEngineTriggerTaskService {
408408

409409
const newExternalTraceparent = serializeTraceparent(
410410
parsedTraceparent.traceId,
411-
parentSpanId ?? parsedTraceparent.spanId
411+
parentSpanId ?? parsedTraceparent.spanId,
412+
parsedTraceparent.traceFlags
412413
);
413414

414415
return {
Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export function parseTraceparent(
22
traceparent?: string
3-
): { traceId: string; spanId: string } | undefined {
3+
): { traceId: string; spanId: string; traceFlags?: string } | undefined {
44
if (!traceparent) {
55
return undefined;
66
}
@@ -11,7 +11,7 @@ export function parseTraceparent(
1111
return undefined;
1212
}
1313

14-
const [version, traceId, spanId] = parts;
14+
const [version, traceId, spanId, traceFlags] = parts;
1515

1616
if (version !== "00") {
1717
return undefined;
@@ -21,9 +21,9 @@ export function parseTraceparent(
2121
return undefined;
2222
}
2323

24-
return { traceId, spanId };
24+
return { traceId, spanId, traceFlags };
2525
}
2626

27-
export function serializeTraceparent(traceId: string, spanId: string) {
28-
return `00-${traceId}-${spanId}-01`;
27+
export function serializeTraceparent(traceId: string, spanId: string, traceFlags?: string) {
28+
return `00-${traceId}-${spanId}-${traceFlags ?? "01"}`;
2929
}

packages/core/src/v3/otel/tracingSDK.ts

Lines changed: 88 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { DiagConsoleLogger, DiagLogLevel, TracerProvider, diag } from "@opentelemetry/api";
1+
import {
2+
DiagConsoleLogger,
3+
DiagLogLevel,
4+
TraceFlags,
5+
TracerProvider,
6+
diag,
7+
} from "@opentelemetry/api";
28
import { logs } from "@opentelemetry/api-logs";
39
import { TraceState } from "@opentelemetry/core";
410
import { OTLPLogExporter } from "@opentelemetry/exporter-logs-otlp-http";
@@ -21,7 +27,7 @@ import {
2127
SimpleSpanProcessor,
2228
SpanExporter,
2329
} from "@opentelemetry/sdk-trace-node";
24-
import { SemanticResourceAttributes } from "@opentelemetry/semantic-conventions";
30+
import { SemanticResourceAttributes, SEMATTRS_HTTP_URL } from "@opentelemetry/semantic-conventions";
2531
import { VERSION } from "../../version.js";
2632
import {
2733
OTEL_ATTRIBUTE_PER_EVENT_COUNT_LIMIT,
@@ -287,17 +293,24 @@ function setLogLevel(level: TracingDiagnosticLogLevel) {
287293
}
288294

289295
class ExternalSpanExporterWrapper {
296+
private readonly _isExternallySampled: boolean;
297+
290298
constructor(
291299
private underlyingExporter: SpanExporter,
292300
private externalTraceId: string,
293301
private externalTraceContext:
294-
| { traceId: string; spanId: string; tracestate?: string }
302+
| { traceId: string; spanId: string; traceFlags: number; tracestate?: string }
295303
| undefined
296-
) {}
304+
) {
305+
this._isExternallySampled = isTraceFlagSampled(externalTraceContext?.traceFlags);
306+
}
297307

298308
private transformSpan(span: ReadableSpan): ReadableSpan | undefined {
299-
if (span.attributes[SemanticInternalAttributes.SPAN_PARTIAL]) {
300-
// Skip partial spans
309+
if (!this._isExternallySampled) {
310+
return;
311+
}
312+
313+
if (isSpanInternalOnly(span)) {
301314
return;
302315
}
303316

@@ -325,8 +338,10 @@ class ExternalSpanExporterWrapper {
325338
traceState: this.externalTraceContext.tracestate
326339
? new TraceState(this.externalTraceContext.tracestate)
327340
: undefined,
328-
traceFlags: parentSpanContext?.traceFlags ?? 0,
341+
traceFlags: this.externalTraceContext.traceFlags,
329342
};
343+
} else if (isAttemptSpan) {
344+
parentSpanContext = undefined;
330345
}
331346

332347
return {
@@ -360,15 +375,25 @@ class ExternalSpanExporterWrapper {
360375
}
361376

362377
class ExternalLogRecordExporterWrapper {
378+
private readonly _isExternallySampled: boolean;
379+
363380
constructor(
364381
private underlyingExporter: LogRecordExporter,
365382
private externalTraceId: string,
366383
private externalTraceContext:
367-
| { traceId: string; spanId: string; tracestate?: string }
384+
| { traceId: string; spanId: string; tracestate?: string; traceFlags: number }
368385
| undefined
369-
) {}
386+
) {
387+
this._isExternallySampled = isTraceFlagSampled(externalTraceContext?.traceFlags);
388+
}
370389

371390
export(logs: any[], resultCallback: (result: any) => void): void {
391+
if (!this._isExternallySampled) {
392+
this.underlyingExporter.export([], resultCallback);
393+
394+
return;
395+
}
396+
372397
const modifiedLogs = logs.map(this.transformLogRecord.bind(this));
373398

374399
this.underlyingExporter.export(modifiedLogs, resultCallback);
@@ -410,3 +435,57 @@ class ExternalLogRecordExporterWrapper {
410435
});
411436
}
412437
}
438+
439+
function isSpanInternalOnly(span: ReadableSpan): boolean {
440+
if (span.attributes[SemanticInternalAttributes.SPAN_PARTIAL]) {
441+
// Skip partial spans
442+
return true;
443+
}
444+
445+
const urlPath = span.attributes["url.path"];
446+
447+
if (typeof urlPath === "string" && urlPath === "/api/v1/usage/ingest") {
448+
return true;
449+
}
450+
451+
const httpUrl = span.attributes[SEMATTRS_HTTP_URL] ?? span.attributes["url.full"];
452+
453+
const url = safeParseUrl(httpUrl);
454+
455+
if (!url) {
456+
return false;
457+
}
458+
459+
const internalHosts = [
460+
"api.trigger.dev",
461+
"billing.trigger.dev",
462+
"cloud.trigger.dev",
463+
"engine.trigger.dev",
464+
"platform.trigger.dev",
465+
];
466+
467+
return (
468+
internalHosts.some((host) => url.hostname.includes(host)) ||
469+
url.pathname.includes("/api/v1/usage/ingest")
470+
);
471+
}
472+
473+
function safeParseUrl(url: unknown): URL | undefined {
474+
if (typeof url !== "string") {
475+
return undefined;
476+
}
477+
478+
try {
479+
return new URL(url);
480+
} catch (e) {
481+
return undefined;
482+
}
483+
}
484+
485+
function isTraceFlagSampled(traceFlags?: number): boolean {
486+
if (typeof traceFlags !== "number") {
487+
return true;
488+
}
489+
490+
return (traceFlags & TraceFlags.SAMPLED) === TraceFlags.SAMPLED;
491+
}

packages/core/src/v3/traceContext/manager.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Context, context, propagation, trace, TraceFlags } from "@opentelemetry/api";
22
import { TraceContextManager } from "./types.js";
3+
import { parseTraceParent } from "@opentelemetry/core";
34

45
export class StandardTraceContextManager implements TraceContextManager {
56
public traceContext: Record<string, unknown> = {};
@@ -39,7 +40,7 @@ export class StandardTraceContextManager implements TraceContextManager {
3940
const spanContext = {
4041
traceId: externalTraceContext.traceId,
4142
spanId: currentSpanContext.spanId,
42-
traceFlags: TraceFlags.SAMPLED,
43+
traceFlags: externalTraceContext.traceFlags,
4344
isRemote: true,
4445
};
4546

@@ -60,15 +61,16 @@ function extractExternalTraceContext(traceContext: unknown) {
6061
: undefined;
6162

6263
if ("traceparent" in traceContext && typeof traceContext.traceparent === "string") {
63-
const [version, traceId, spanId] = traceContext.traceparent.split("-");
64+
const externalSpanContext = parseTraceParent(traceContext.traceparent);
6465

65-
if (!traceId || !spanId) {
66+
if (!externalSpanContext) {
6667
return undefined;
6768
}
6869

6970
return {
70-
traceId,
71-
spanId,
71+
traceId: externalSpanContext.traceId,
72+
spanId: externalSpanContext.spanId,
73+
traceFlags: externalSpanContext.traceFlags,
7274
tracestate: tracestate,
7375
};
7476
}

packages/core/src/v3/traceContext/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface TraceContextManager {
88
| {
99
traceId: string;
1010
spanId: string;
11+
traceFlags: number;
1112
tracestate?: string;
1213
}
1314
| undefined;

pnpm-lock.yaml

Lines changed: 34 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

references/d3-chat/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
"@opentelemetry/api-logs": "^0.203.0",
2727
"@opentelemetry/exporter-logs-otlp-http": "0.203.0",
2828
"@opentelemetry/exporter-trace-otlp-http": "0.203.0",
29+
"@opentelemetry/instrumentation-http": "0.203.0",
30+
"@opentelemetry/instrumentation-undici": "0.14.0",
2931
"@opentelemetry/instrumentation": "^0.203.0",
3032
"@opentelemetry/sdk-logs": "^0.203.0",
3133
"@radix-ui/react-avatar": "^1.1.3",
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { registerOTel } from "@vercel/otel";
22

33
export function register() {
4-
registerOTel({ serviceName: "d3-chat" });
4+
registerOTel({ serviceName: "d3-chat", traceSampler: "traceidratio" });
55
}

references/d3-chat/trigger.config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@ import { pythonExtension } from "@trigger.dev/python/extension";
33
import { installPlaywrightChromium } from "./src/extensions/playwright";
44
import { OTLPLogExporter } from "@opentelemetry/exporter-logs-otlp-http";
55
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http";
6+
import { HttpInstrumentation } from "@opentelemetry/instrumentation-http";
7+
import { UndiciInstrumentation } from "@opentelemetry/instrumentation-undici";
68

79
export default defineConfig({
810
project: process.env.TRIGGER_PROJECT_REF!,
911
dirs: ["./src/trigger"],
1012
telemetry: {
13+
instrumentations: [new HttpInstrumentation(), new UndiciInstrumentation()],
1114
logExporters: [
1215
new OTLPLogExporter({
1316
url: "https://api.axiom.co/v1/logs",

0 commit comments

Comments
 (0)