Skip to content

Commit 349c14a

Browse files
committed
fix(gax): address review feedback on span names and span completion helper
1 parent 7ba64dc commit 349c14a

1 file changed

Lines changed: 17 additions & 28 deletions

File tree

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTracingTracer.java‎

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,9 @@ public Scope inScope() {
138138
String operationSpanName) {
139139
this.tracer = tracer;
140140
this.apiTracerContext = apiTracerContext;
141-
this.attemptSpanName = attemptSpanName;
142141
this.operationSpanName = operationSpanName;
142+
this.attemptSpanName =
143+
attemptSpanName.equals(operationSpanName) ? attemptSpanName + "/attempt" : attemptSpanName;
143144
this.attemptAttributes = new HashMap<>();
144145
this.parentContext = io.opentelemetry.context.Context.current();
145146
buildAttributes();
@@ -242,7 +243,7 @@ public void attemptStarted(Object request, int attemptNumber) {
242243
}
243244
// End lingering previous attempt outside the lock to avoid holding the lock during callbacks.
244245
if (oldSpan != null) {
245-
endAttemptSpan(oldSpan, null);
246+
endSpan(oldSpan, null);
246247
}
247248
}
248249

@@ -304,24 +305,10 @@ private void recordErrorAndEndOperation(@Nullable Throwable error) {
304305
}
305306

306307
if (localAttemptSpan != null) {
307-
endAttemptSpan(localAttemptSpan, error);
308+
endSpan(localAttemptSpan, error);
308309
}
309310

310-
Map<String, Object> responseAttributes =
311-
ObservabilityUtils.getResponseAttributes(error, this.apiTracerContext.transport());
312-
if (!responseAttributes.isEmpty()) {
313-
localOperationSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes));
314-
}
315-
316-
if (error != null) {
317-
localOperationSpan.setStatus(StatusCode.ERROR);
318-
if (!Strings.isNullOrEmpty(error.getMessage())) {
319-
localOperationSpan.setAttribute(
320-
ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage());
321-
}
322-
}
323-
324-
localOperationSpan.end();
311+
endSpan(localOperationSpan, error);
325312
}
326313

327314
@Override
@@ -412,31 +399,33 @@ private void recordErrorAndEndAttempt(@Nullable Throwable error) {
412399
lock.unlock();
413400
}
414401

415-
endAttemptSpan(localAttemptSpan, error);
402+
endSpan(localAttemptSpan, error);
416403
}
417404

418405
/**
419-
* Attaches response status attributes and error messages to the attempt span and ends it.
406+
* Attaches response status attributes and error messages to the span and ends it.
420407
*
421408
* <p>This method runs outside of synchronization locks to avoid blocking threads during
422409
* OpenTelemetry span completion callbacks.
423410
*
424-
* @param localAttemptSpan the attempt span to finish
425-
* @param error the exception that caused the attempt to end, or {@code null} if successful
411+
* @param span the span to finish
412+
* @param error the exception that caused the span to end, or {@code null} if successful
426413
*/
427-
private void endAttemptSpan(Span localAttemptSpan, @Nullable Throwable error) {
414+
private void endSpan(Span span, @Nullable Throwable error) {
428415
Map<String, Object> responseAttributes =
429416
ObservabilityUtils.getResponseAttributes(error, this.apiTracerContext.transport());
430417
if (!responseAttributes.isEmpty()) {
431-
localAttemptSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes));
418+
span.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes));
432419
}
433420

434-
if (error != null && !Strings.isNullOrEmpty(error.getMessage())) {
435-
localAttemptSpan.setAttribute(
436-
ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage());
421+
if (error != null) {
422+
span.setStatus(StatusCode.ERROR);
423+
if (!Strings.isNullOrEmpty(error.getMessage())) {
424+
span.setAttribute(ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage());
425+
}
437426
}
438427

439-
localAttemptSpan.end();
428+
span.end();
440429
}
441430

442431
@Override

0 commit comments

Comments
 (0)