Skip to content

Commit 35830d8

Browse files
torosentCopilot
andcommitted
refactor: simplify tracing code — remove dead code, extract helpers
- Remove unused createClientSpan() method (52 lines) and its 2 tests. Replaced by emitRetroactiveClientSpan() which creates spans with proper scheduling-to-completion duration. - Extract emitClientSpanIfTracked() helper to eliminate 4× duplicated retroactive client span emission blocks in task/sub-orchestration completed/failed handlers. - Extract storeSchedulingMetadata() helper to consolidate 2× duplicated scheduling metadata storage in handleTaskScheduled and handleSubOrchestrationCreated. Net: -151 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a7ecd60 commit 35830d8

File tree

3 files changed

+36
-187
lines changed

3 files changed

+36
-187
lines changed

client/src/main/java/com/microsoft/durabletask/TaskOrchestrationExecutor.java

Lines changed: 36 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -545,18 +545,7 @@ private void handleTaskScheduled(HistoryEvent e) {
545545

546546
TaskScheduledEvent taskScheduled = e.getTaskScheduled();
547547

548-
// Store scheduling metadata for retroactive client span at completion time.
549-
// Use orchestrationSpanContext as parent so the client span is a child of the orchestration span.
550-
Instant scheduledTime = e.hasTimestamp()
551-
? DataConverter.getInstantFromTimestamp(e.getTimestamp())
552-
: null;
553-
TraceContext spanParent = this.orchestrationSpanContext != null
554-
? this.orchestrationSpanContext : this.parentTraceContext;
555-
this.scheduledTaskInfoMap.put(taskId, new ScheduledTaskInfo(
556-
taskScheduled.getName(),
557-
scheduledTime,
558-
spanParent,
559-
TracingHelper.TYPE_ACTIVITY));
548+
storeSchedulingMetadata(taskId, taskScheduled.getName(), TracingHelper.TYPE_ACTIVITY, e);
560549

561550
// The history shows that this orchestrator created a durable task in a previous execution.
562551
// We can therefore remove it from the map of pending actions. If we can't find the pending
@@ -595,17 +584,7 @@ private void handleTaskCompleted(HistoryEvent e) {
595584

596585
// Emit a retroactive Client span covering scheduling-to-completion duration.
597586
// Matches .NET SDK's EmitTraceActivityForTaskCompleted pattern.
598-
ScheduledTaskInfo info = this.scheduledTaskInfoMap.remove(taskId);
599-
if (info != null) {
600-
TracingHelper.emitRetroactiveClientSpan(
601-
info.spanType + ":" + info.taskName,
602-
info.parentTraceContext,
603-
info.spanType,
604-
info.taskName,
605-
this.instanceId,
606-
taskId,
607-
info.scheduledTime);
608-
}
587+
emitClientSpanIfTracked(taskId);
609588
}
610589
CompletableTask task = record.getTask();
611590
try {
@@ -631,17 +610,7 @@ private void handleTaskFailed(HistoryEvent e) {
631610
// TODO: Log task failure, including the number of bytes in the result
632611

633612
// Emit a retroactive Client span covering scheduling-to-failure duration.
634-
ScheduledTaskInfo info = this.scheduledTaskInfoMap.remove(taskId);
635-
if (info != null) {
636-
TracingHelper.emitRetroactiveClientSpan(
637-
info.spanType + ":" + info.taskName,
638-
info.parentTraceContext,
639-
info.spanType,
640-
info.taskName,
641-
this.instanceId,
642-
taskId,
643-
info.scheduledTime);
644-
}
613+
emitClientSpanIfTracked(taskId);
645614
}
646615

647616
CompletableTask<?> task = record.getTask();
@@ -798,17 +767,8 @@ private void handleSubOrchestrationCreated(HistoryEvent e) {
798767
int taskId = e.getEventId();
799768
SubOrchestrationInstanceCreatedEvent subOrchestrationInstanceCreated = e.getSubOrchestrationInstanceCreated();
800769

801-
// Store scheduling metadata for retroactive client span at completion time
802-
Instant scheduledTime = e.hasTimestamp()
803-
? DataConverter.getInstantFromTimestamp(e.getTimestamp())
804-
: null;
805-
TraceContext spanParent = this.orchestrationSpanContext != null
806-
? this.orchestrationSpanContext : this.parentTraceContext;
807-
this.scheduledTaskInfoMap.put(taskId, new ScheduledTaskInfo(
808-
subOrchestrationInstanceCreated.getName(),
809-
scheduledTime,
810-
spanParent,
811-
TracingHelper.TYPE_ORCHESTRATION));
770+
storeSchedulingMetadata(taskId, subOrchestrationInstanceCreated.getName(),
771+
TracingHelper.TYPE_ORCHESTRATION, e);
812772

813773
OrchestratorAction taskAction = this.pendingActions.remove(taskId);
814774
if (taskAction == null) {
@@ -841,17 +801,7 @@ private void handleSubOrchestrationCompleted(HistoryEvent e) {
841801
rawResult != null ? rawResult : "(null)"));
842802

843803
// Emit a retroactive Client span covering scheduling-to-completion duration.
844-
ScheduledTaskInfo info = this.scheduledTaskInfoMap.remove(taskId);
845-
if (info != null) {
846-
TracingHelper.emitRetroactiveClientSpan(
847-
info.spanType + ":" + info.taskName,
848-
info.parentTraceContext,
849-
info.spanType,
850-
info.taskName,
851-
this.instanceId,
852-
taskId,
853-
info.scheduledTime);
854-
}
804+
emitClientSpanIfTracked(taskId);
855805
}
856806
CompletableTask task = record.getTask();
857807
try {
@@ -877,17 +827,7 @@ private void handleSubOrchestrationFailed(HistoryEvent e){
877827
// TODO: Log task failure, including the number of bytes in the result
878828

879829
// Emit a retroactive Client span covering scheduling-to-failure duration.
880-
ScheduledTaskInfo info = this.scheduledTaskInfoMap.remove(taskId);
881-
if (info != null) {
882-
TracingHelper.emitRetroactiveClientSpan(
883-
info.spanType + ":" + info.taskName,
884-
info.parentTraceContext,
885-
info.spanType,
886-
info.taskName,
887-
this.instanceId,
888-
taskId,
889-
info.scheduledTime);
890-
}
830+
emitClientSpanIfTracked(taskId);
891831
}
892832

893833
CompletableTask<?> task = record.getTask();
@@ -1104,6 +1044,35 @@ public Class<V> getDataType() {
11041044
}
11051045
}
11061046

1047+
/**
1048+
* Emits a retroactive Client span for a completed/failed task, if scheduling metadata was tracked.
1049+
*/
1050+
private void emitClientSpanIfTracked(int taskId) {
1051+
ScheduledTaskInfo info = this.scheduledTaskInfoMap.remove(taskId);
1052+
if (info != null) {
1053+
TracingHelper.emitRetroactiveClientSpan(
1054+
info.spanType + ":" + info.taskName,
1055+
info.parentTraceContext,
1056+
info.spanType,
1057+
info.taskName,
1058+
this.instanceId,
1059+
taskId,
1060+
info.scheduledTime);
1061+
}
1062+
}
1063+
1064+
/**
1065+
* Stores scheduling metadata for a task so a retroactive client span can be emitted at completion time.
1066+
*/
1067+
private void storeSchedulingMetadata(int taskId, String taskName, String spanType, HistoryEvent e) {
1068+
Instant scheduledTime = e.hasTimestamp()
1069+
? DataConverter.getInstantFromTimestamp(e.getTimestamp()) : null;
1070+
TraceContext spanParent = this.orchestrationSpanContext != null
1071+
? this.orchestrationSpanContext : this.parentTraceContext;
1072+
this.scheduledTaskInfoMap.put(taskId, new ScheduledTaskInfo(
1073+
taskName, scheduledTime, spanParent, spanType));
1074+
}
1075+
11071076
/**
11081077
* Stores scheduling metadata for retroactive client span creation at completion time.
11091078
* Matches .NET SDK pattern where client spans are emitted with the full scheduling-to-completion duration.

client/src/main/java/com/microsoft/durabletask/TracingHelper.java

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -219,74 +219,6 @@ static Span startSpanWithStartTime(
219219
return span;
220220
}
221221

222-
/**
223-
* Creates a short-lived Client-kind span for scheduling an activity or sub-orchestration,
224-
* captures its trace context as a protobuf {@code TraceContext}, and ends the span immediately.
225-
* This mirrors the .NET SDK pattern of paired Client+Server spans.
226-
*
227-
* @param spanName The span name (e.g. "activity:GetWeather").
228-
* @param parentContext The parent trace context from the orchestration, may be {@code null}.
229-
* @param type The durabletask.type value (e.g. "activity").
230-
* @param taskName The task name attribute value.
231-
* @param instanceId The orchestration instance ID.
232-
* @param taskId The task sequence ID.
233-
* @return A {@code TraceContext} captured from the Client span, or the original parentContext if tracing is unavailable.
234-
*/
235-
@Nullable
236-
static TraceContext createClientSpan(
237-
String spanName,
238-
@Nullable TraceContext parentContext,
239-
String type,
240-
String taskName,
241-
@Nullable String instanceId,
242-
int taskId) {
243-
244-
Tracer tracer = GlobalOpenTelemetry.getTracer(TRACER_NAME);
245-
SpanBuilder spanBuilder = tracer.spanBuilder(spanName)
246-
.setSpanKind(SpanKind.CLIENT)
247-
.setAttribute(ATTR_TYPE, type)
248-
.setAttribute(ATTR_TASK_NAME, taskName)
249-
.setAttribute(ATTR_TASK_ID, String.valueOf(taskId));
250-
251-
if (instanceId != null) {
252-
spanBuilder.setAttribute(ATTR_INSTANCE_ID, instanceId);
253-
}
254-
255-
Context parentCtx = extractTraceContext(parentContext);
256-
if (parentCtx != null) {
257-
spanBuilder.setParent(parentCtx);
258-
}
259-
260-
Span clientSpan = spanBuilder.startSpan();
261-
262-
// Capture the client span's context before ending it
263-
TraceContext captured;
264-
try {
265-
SpanContext sc = clientSpan.getSpanContext();
266-
if (sc.isValid()) {
267-
String traceParent = String.format("00-%s-%s-%02x",
268-
sc.getTraceId(), sc.getSpanId(), sc.getTraceFlags().asByte());
269-
270-
String traceState = sc.getTraceState().asMap()
271-
.entrySet()
272-
.stream()
273-
.map(entry -> entry.getKey() + "=" + entry.getValue())
274-
.collect(Collectors.joining(","));
275-
276-
TraceContext.Builder builder = TraceContext.newBuilder().setTraceParent(traceParent);
277-
if (traceState != null && !traceState.isEmpty()) {
278-
builder.setTraceState(StringValue.of(traceState));
279-
}
280-
captured = builder.build();
281-
} else {
282-
captured = parentContext;
283-
}
284-
} finally {
285-
clientSpan.end();
286-
}
287-
return captured;
288-
}
289-
290222
/**
291223
* Ends the given span, optionally recording an error.
292224
*

client/src/test/java/com/microsoft/durabletask/TracingHelperTest.java

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -239,58 +239,6 @@ void getCurrentTraceContext_roundTrip() {
239239
extractedSpan.getSpanContext().getSpanId());
240240
}
241241

242-
@Test
243-
void createClientSpan_createsClientKindSpan_withNewSpanId() {
244-
// Create a parent context
245-
Tracer tracer = openTelemetry.getTracer("test");
246-
Span parentSpan = tracer.spanBuilder("parent-orch").startSpan();
247-
TraceContext parentCtx;
248-
try (Scope ignored = parentSpan.makeCurrent()) {
249-
parentCtx = TracingHelper.getCurrentTraceContext();
250-
} finally {
251-
parentSpan.end();
252-
}
253-
assertNotNull(parentCtx);
254-
spanExporter.reset();
255-
256-
// Create a client span
257-
TraceContext clientCtx = TracingHelper.createClientSpan(
258-
"activity:GetWeather", parentCtx,
259-
TracingHelper.TYPE_ACTIVITY, "GetWeather", "instance-123", 3);
260-
261-
assertNotNull(clientCtx);
262-
assertNotNull(clientCtx.getTraceParent());
263-
264-
// Client span should have the same trace ID but different span ID
265-
String parentTraceId = parentCtx.getTraceParent().split("-")[1];
266-
String clientTraceId = clientCtx.getTraceParent().split("-")[1];
267-
String parentSpanId = parentCtx.getTraceParent().split("-")[2];
268-
String clientSpanId = clientCtx.getTraceParent().split("-")[2];
269-
assertEquals(parentTraceId, clientTraceId, "Should share the same trace ID");
270-
assertNotEquals(parentSpanId, clientSpanId, "Should have a new span ID");
271-
272-
// Verify the exported client span has correct attributes and kind
273-
List<SpanData> spans = spanExporter.getFinishedSpanItems();
274-
assertEquals(1, spans.size());
275-
SpanData sd = spans.get(0);
276-
assertEquals("activity:GetWeather", sd.getName());
277-
assertEquals(SpanKind.CLIENT, sd.getKind());
278-
assertEquals("activity", sd.getAttributes().get(io.opentelemetry.api.common.AttributeKey.stringKey("durabletask.type")));
279-
assertEquals("GetWeather", sd.getAttributes().get(io.opentelemetry.api.common.AttributeKey.stringKey("durabletask.task.name")));
280-
assertEquals("instance-123", sd.getAttributes().get(io.opentelemetry.api.common.AttributeKey.stringKey("durabletask.task.instance_id")));
281-
assertEquals("3", sd.getAttributes().get(io.opentelemetry.api.common.AttributeKey.stringKey("durabletask.task.task_id")));
282-
}
283-
284-
@Test
285-
void createClientSpan_withNullParent_returnsNull() {
286-
TraceContext result = TracingHelper.createClientSpan(
287-
"activity:Test", null, TracingHelper.TYPE_ACTIVITY, "Test", null, 0);
288-
// When parent is null, the span has no parent but still creates a valid context
289-
// (or returns null if the tracer returns an invalid span)
290-
// With a registered SDK, it should create a root span
291-
assertNotNull(result);
292-
}
293-
294242
@Test
295243
void emitTimerSpan_createsInternalSpanWithFireAt() {
296244
TracingHelper.emitTimerSpan("MyOrchestration", "instance-1", 5, "2026-01-01T00:00:00Z", null, null);

0 commit comments

Comments
 (0)