Skip to content

Commit fb988d4

Browse files
torosentCopilot
andauthored
Fix misleading tracer cache invalidation comment and simplify logic (#97)
* Refactor tracing span names and types for consistency with Durable Task conventions * Refactor createPbTraceContextFromSpan to use createTraceparent helper (#98) * Remove package-lock.json from .gitignore (#99) * Fix setSpanError/setSpanOk to use getOtelApi() directly and widen createPbTraceContextFromSpan signature - Revert setSpanError and setSpanOk to call getOtelApi() instead of getTracingContext(), since they only need SpanStatusCode (no tracer) - Widen createPbTraceContextFromSpan parameter type to Span | undefined | null for consistency with extractTraceparentFromSpan --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
1 parent 777a90c commit fb988d4

File tree

7 files changed

+223
-75
lines changed

7 files changed

+223
-75
lines changed

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,4 @@ packages/**/version.ts
156156
!.vscode/tasks.json
157157
!.vscode/launch.json
158158
!.vscode/extensions.json
159-
*.code-workspace
159+
*.code-workspace

packages/durabletask-js/src/tracing/constants.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ export function createSpanName(type: string, name: string, version?: string): st
8585
/**
8686
* Creates a timer span name following the Durable Task naming convention.
8787
*
88-
* Format: "timer:{orchName}"
88+
* Format: "orchestration:{orchName}:timer"
8989
*
9090
* @param orchestrationName - The name of the parent orchestration.
9191
* @returns The formatted timer span name.
9292
*/
9393
export function createTimerSpanName(orchestrationName: string): string {
94-
return `${TaskType.TIMER}:${orchestrationName}`;
94+
return `${TaskType.ORCHESTRATION}:${orchestrationName}:${TaskType.TIMER}`;
9595
}

packages/durabletask-js/src/tracing/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export {
1010
createPbTraceContext,
1111
extractTraceparentFromSpan,
1212
createParentContextFromPb,
13+
createPbTraceContextFromSpan,
1314
} from "./trace-context-utils";
1415

1516
// Internal-only exports (not re-exported from package index.ts):

packages/durabletask-js/src/tracing/trace-context-utils.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,36 @@ export function extractTraceparentFromSpan(span: Span | undefined | null): { tra
130130
return { traceparent, tracestate };
131131
}
132132

133+
/**
134+
* Creates a protobuf TraceContext directly from a Span, avoiding the
135+
* format→parse roundtrip of extractTraceparentFromSpan + createPbTraceContext.
136+
* Returns undefined if the span context is not valid.
137+
*/
138+
export function createPbTraceContextFromSpan(span: Span | undefined | null): pb.TraceContext | undefined {
139+
const otel = getOtelApi();
140+
if (!otel || !span) return undefined;
141+
142+
const spanContext = span.spanContext();
143+
if (!otel.isSpanContextValid(spanContext)) {
144+
return undefined;
145+
}
146+
147+
const traceparent = createTraceparent(spanContext.traceId, spanContext.spanId, spanContext.traceFlags);
148+
149+
const ctx = new pb.TraceContext();
150+
ctx.setTraceparent(traceparent);
151+
ctx.setSpanid(spanContext.spanId);
152+
153+
const tracestate = spanContext.traceState?.serialize();
154+
if (tracestate) {
155+
const sv = new StringValue();
156+
sv.setValue(tracestate);
157+
ctx.setTracestate(sv);
158+
}
159+
160+
return ctx;
161+
}
162+
133163
/**
134164
* Creates an OTEL Context with a remote parent span from a protobuf TraceContext.
135165
* Returns undefined if OTEL is not available or the pbTraceContext is not provided.

packages/durabletask-js/src/tracing/trace-helper.ts

Lines changed: 71 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,39 @@ import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb";
77
import { TRACER_NAME, DurableTaskAttributes, TaskType, createSpanName, createTimerSpanName } from "./constants";
88
import {
99
getOtelApi,
10-
extractTraceparentFromSpan,
11-
createPbTraceContext,
10+
createPbTraceContextFromSpan,
1211
createParentContextFromPb,
1312
} from "./trace-context-utils";
1413
import type { Span, Tracer } from "@opentelemetry/api";
1514

15+
// Cached tracer instance to avoid repeated lookups. The tracer is created once
16+
// and reused for the lifetime of the process. This is safe because the OTEL JS
17+
// SDK returns a proxy tracer from `trace.getTracer()` that dynamically delegates
18+
// to the current global tracer provider, so provider swaps (e.g., in tests via
19+
// `setGlobalTracerProvider`) are handled transparently.
20+
let _cachedTracer: Tracer | undefined;
21+
22+
/**
23+
* Returns the OTEL API and tracer, or undefined if OTEL is not available.
24+
* Caches the tracer instance for efficiency.
25+
*/
26+
function getTracingContext(): { otel: typeof import("@opentelemetry/api"); tracer: Tracer } | undefined {
27+
const otel = getOtelApi();
28+
if (!otel) return undefined;
29+
30+
if (!_cachedTracer) {
31+
_cachedTracer = otel.trace.getTracer(TRACER_NAME);
32+
}
33+
34+
return { otel, tracer: _cachedTracer };
35+
}
36+
1637
/**
1738
* Gets the Durable Task tracer from the OpenTelemetry API.
1839
* Returns undefined if OpenTelemetry is not installed.
1940
*/
2041
export function getTracer(): Tracer | undefined {
21-
const otel = getOtelApi();
22-
if (!otel) return undefined;
23-
return otel.trace.getTracer(TRACER_NAME);
42+
return getTracingContext()?.tracer;
2443
}
2544

2645
/**
@@ -50,29 +69,27 @@ export interface OrchestrationSpanInfo {
5069
* @returns The span (or undefined if OTEL is not available). Caller must end it.
5170
*/
5271
export function startSpanForNewOrchestration(req: pb.CreateInstanceRequest): Span | undefined {
53-
const otel = getOtelApi();
54-
const tracer = getTracer();
55-
if (!otel || !tracer) return undefined;
72+
const ctx = getTracingContext();
73+
if (!ctx) return undefined;
5674

5775
const name = req.getName();
5876
const version = req.getVersion()?.getValue();
5977
const instanceId = req.getInstanceid();
6078
const spanName = createSpanName(TaskType.CREATE_ORCHESTRATION, name, version);
6179

62-
const span = tracer.startSpan(spanName, {
63-
kind: otel.SpanKind.PRODUCER,
80+
const span = ctx.tracer.startSpan(spanName, {
81+
kind: ctx.otel.SpanKind.PRODUCER,
6482
attributes: {
65-
[DurableTaskAttributes.TYPE]: TaskType.CREATE_ORCHESTRATION,
83+
[DurableTaskAttributes.TYPE]: TaskType.ORCHESTRATION,
6684
[DurableTaskAttributes.TASK_NAME]: name,
6785
[DurableTaskAttributes.TASK_INSTANCE_ID]: instanceId,
6886
...(version ? { [DurableTaskAttributes.TASK_VERSION]: version } : {}),
6987
},
7088
});
7189

7290
// Inject trace context into the proto request
73-
const traceInfo = extractTraceparentFromSpan(span);
74-
if (traceInfo) {
75-
const pbCtx = createPbTraceContext(traceInfo.traceparent, traceInfo.tracestate);
91+
const pbCtx = createPbTraceContextFromSpan(span);
92+
if (pbCtx) {
7693
req.setParenttracecontext(pbCtx);
7794
}
7895

@@ -93,9 +110,8 @@ export function startSpanForOrchestrationExecution(
93110
orchestrationTraceContext: pb.OrchestrationTraceContext | undefined,
94111
instanceId: string,
95112
): { span: Span; spanInfo: OrchestrationSpanInfo } | undefined {
96-
const otel = getOtelApi();
97-
const tracer = getTracer();
98-
if (!otel || !tracer) return undefined;
113+
const ctx = getTracingContext();
114+
if (!ctx) return undefined;
99115

100116
const name = executionStartedEvent.getName();
101117
const version = executionStartedEvent.getVersion()?.getValue();
@@ -118,10 +134,10 @@ export function startSpanForOrchestrationExecution(
118134
// first-execution time for storage in OrchestrationTraceContext.
119135
const persistedStartTime = isReplay ? existingStartTime! : spanStartTime;
120136

121-
const span = tracer.startSpan(
137+
const span = ctx.tracer.startSpan(
122138
spanName,
123139
{
124-
kind: otel.SpanKind.SERVER,
140+
kind: ctx.otel.SpanKind.SERVER,
125141
startTime: spanStartTime,
126142
attributes: {
127143
[DurableTaskAttributes.TYPE]: TaskType.ORCHESTRATION,
@@ -168,21 +184,20 @@ export function startSpanForSchedulingTask(
168184
action: pb.ScheduleTaskAction,
169185
taskId: number,
170186
): void {
171-
const otel = getOtelApi();
172-
const tracer = getTracer();
173-
if (!otel || !tracer) return;
187+
const ctx = getTracingContext();
188+
if (!ctx) return;
174189

175190
const name = action.getName();
176191
const version = action.getVersion()?.getValue();
177192
const spanName = createSpanName(TaskType.ACTIVITY, name, version);
178193

179194
// Create a context with the orchestration span as parent
180-
const parentContext = otel.trace.setSpan(otel.context.active(), orchestrationSpan);
195+
const parentContext = ctx.otel.trace.setSpan(ctx.otel.context.active(), orchestrationSpan);
181196

182-
const span = tracer.startSpan(
197+
const span = ctx.tracer.startSpan(
183198
spanName,
184199
{
185-
kind: otel.SpanKind.CLIENT,
200+
kind: ctx.otel.SpanKind.CLIENT,
186201
attributes: {
187202
[DurableTaskAttributes.TYPE]: TaskType.ACTIVITY,
188203
[DurableTaskAttributes.TASK_NAME]: name,
@@ -194,9 +209,8 @@ export function startSpanForSchedulingTask(
194209
);
195210

196211
// Inject trace context into the action
197-
const traceInfo = extractTraceparentFromSpan(span);
198-
if (traceInfo) {
199-
const pbCtx = createPbTraceContext(traceInfo.traceparent, traceInfo.tracestate);
212+
const pbCtx = createPbTraceContextFromSpan(span);
213+
if (pbCtx) {
200214
action.setParenttracecontext(pbCtx);
201215
}
202216

@@ -211,9 +225,8 @@ export function startSpanForSchedulingTask(
211225
* @returns The span (or undefined if OTEL is not available). Caller must end it.
212226
*/
213227
export function startSpanForTaskExecution(req: pb.ActivityRequest): Span | undefined {
214-
const otel = getOtelApi();
215-
const tracer = getTracer();
216-
if (!otel || !tracer) return undefined;
228+
const ctx = getTracingContext();
229+
if (!ctx) return undefined;
217230

218231
const name = req.getName();
219232
const spanName = createSpanName(TaskType.ACTIVITY, name);
@@ -223,10 +236,10 @@ export function startSpanForTaskExecution(req: pb.ActivityRequest): Span | undef
223236

224237
const instanceId = req.getOrchestrationinstance()?.getInstanceid() ?? "";
225238

226-
const span = tracer.startSpan(
239+
const span = ctx.tracer.startSpan(
227240
spanName,
228241
{
229-
kind: otel.SpanKind.SERVER,
242+
kind: ctx.otel.SpanKind.SERVER,
230243
attributes: {
231244
[DurableTaskAttributes.TYPE]: TaskType.ACTIVITY,
232245
[DurableTaskAttributes.TASK_NAME]: name,
@@ -253,23 +266,22 @@ export function startSpanForSchedulingSubOrchestration(
253266
action: pb.CreateSubOrchestrationAction,
254267
taskId: number,
255268
): void {
256-
const otel = getOtelApi();
257-
const tracer = getTracer();
258-
if (!otel || !tracer) return;
269+
const ctx = getTracingContext();
270+
if (!ctx) return;
259271

260272
const name = action.getName();
261273
const version = action.getVersion()?.getValue();
262274
const instanceId = action.getInstanceid();
263-
const spanName = createSpanName(TaskType.CREATE_ORCHESTRATION, name, version);
275+
const spanName = createSpanName(TaskType.ORCHESTRATION, name, version);
264276

265-
const parentContext = otel.trace.setSpan(otel.context.active(), orchestrationSpan);
277+
const parentContext = ctx.otel.trace.setSpan(ctx.otel.context.active(), orchestrationSpan);
266278

267-
const span = tracer.startSpan(
279+
const span = ctx.tracer.startSpan(
268280
spanName,
269281
{
270-
kind: otel.SpanKind.CLIENT,
282+
kind: ctx.otel.SpanKind.CLIENT,
271283
attributes: {
272-
[DurableTaskAttributes.TYPE]: TaskType.CREATE_ORCHESTRATION,
284+
[DurableTaskAttributes.TYPE]: TaskType.ORCHESTRATION,
273285
[DurableTaskAttributes.TASK_NAME]: name,
274286
[DurableTaskAttributes.TASK_INSTANCE_ID]: instanceId,
275287
[DurableTaskAttributes.TASK_TASK_ID]: taskId,
@@ -280,9 +292,8 @@ export function startSpanForSchedulingSubOrchestration(
280292
);
281293

282294
// Inject trace context into the action
283-
const traceInfo = extractTraceparentFromSpan(span);
284-
if (traceInfo) {
285-
const pbCtx = createPbTraceContext(traceInfo.traceparent, traceInfo.tracestate);
295+
const pbCtx = createPbTraceContextFromSpan(span);
296+
if (pbCtx) {
286297
action.setParenttracecontext(pbCtx);
287298
}
288299

@@ -304,17 +315,16 @@ export function emitSpanForTimer(
304315
fireAt: Date,
305316
timerId: number,
306317
): void {
307-
const otel = getOtelApi();
308-
const tracer = getTracer();
309-
if (!otel || !tracer) return;
318+
const ctx = getTracingContext();
319+
if (!ctx) return;
310320

311321
const spanName = createTimerSpanName(orchestrationName);
312-
const parentContext = otel.trace.setSpan(otel.context.active(), orchestrationSpan);
322+
const parentContext = ctx.otel.trace.setSpan(ctx.otel.context.active(), orchestrationSpan);
313323

314-
const span = tracer.startSpan(
324+
const span = ctx.tracer.startSpan(
315325
spanName,
316326
{
317-
kind: otel.SpanKind.INTERNAL,
327+
kind: ctx.otel.SpanKind.INTERNAL,
318328
attributes: {
319329
[DurableTaskAttributes.TYPE]: TaskType.TIMER,
320330
[DurableTaskAttributes.TASK_TASK_ID]: timerId,
@@ -339,19 +349,18 @@ export function emitSpanForEventSent(
339349
eventName: string,
340350
targetInstanceId?: string,
341351
): void {
342-
const otel = getOtelApi();
343-
const tracer = getTracer();
344-
if (!otel || !tracer) return;
352+
const ctx = getTracingContext();
353+
if (!ctx) return;
345354

346355
const spanName = createSpanName(TaskType.ORCHESTRATION_EVENT, eventName);
347-
const parentContext = otel.trace.setSpan(otel.context.active(), orchestrationSpan);
356+
const parentContext = ctx.otel.trace.setSpan(ctx.otel.context.active(), orchestrationSpan);
348357

349-
const span = tracer.startSpan(
358+
const span = ctx.tracer.startSpan(
350359
spanName,
351360
{
352-
kind: otel.SpanKind.PRODUCER,
361+
kind: ctx.otel.SpanKind.PRODUCER,
353362
attributes: {
354-
[DurableTaskAttributes.TYPE]: TaskType.ORCHESTRATION_EVENT,
363+
[DurableTaskAttributes.TYPE]: TaskType.EVENT,
355364
[DurableTaskAttributes.TASK_NAME]: eventName,
356365
...(targetInstanceId ? { [DurableTaskAttributes.EVENT_TARGET_INSTANCE_ID]: targetInstanceId } : {}),
357366
},
@@ -370,16 +379,15 @@ export function emitSpanForEventSent(
370379
* @returns The span (or undefined if OTEL is not available). Caller must end it.
371380
*/
372381
export function startSpanForEventRaisedFromClient(eventName: string, instanceId: string): Span | undefined {
373-
const otel = getOtelApi();
374-
const tracer = getTracer();
375-
if (!otel || !tracer) return undefined;
382+
const ctx = getTracingContext();
383+
if (!ctx) return undefined;
376384

377385
const spanName = createSpanName(TaskType.ORCHESTRATION_EVENT, eventName);
378386

379-
const span = tracer.startSpan(spanName, {
380-
kind: otel.SpanKind.PRODUCER,
387+
const span = ctx.tracer.startSpan(spanName, {
388+
kind: ctx.otel.SpanKind.PRODUCER,
381389
attributes: {
382-
[DurableTaskAttributes.TYPE]: TaskType.ORCHESTRATION_EVENT,
390+
[DurableTaskAttributes.TYPE]: TaskType.EVENT,
383391
[DurableTaskAttributes.TASK_NAME]: eventName,
384392
[DurableTaskAttributes.EVENT_TARGET_INSTANCE_ID]: instanceId,
385393
},

0 commit comments

Comments
 (0)