diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java index dffc1df0695b..ab2c92e60547 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java @@ -37,6 +37,7 @@ import com.google.common.base.Strings; import java.util.HashMap; import java.util.Map; +import java.util.function.Supplier; import javax.annotation.Nullable; /** @@ -164,9 +165,46 @@ String rpcSystemName() { @Nullable public abstract String urlDomain(); - /** The destination resource id of the request (e.g. projects/p/locations/l/topics/t). */ @Nullable - public abstract String destinationResourceId(); + protected abstract Supplier destinationResourceIdSupplier(); + + /** + * The destination resource id of the request (e.g. + * //pubsub.googleapis.com/projects/p/locations/l/topics/t). + */ + @Nullable + public String destinationResourceId() { + Supplier supplier = destinationResourceIdSupplier(); + if (supplier == null) { + return null; + } + String resourceId = supplier.get(); + if (Strings.isNullOrEmpty(resourceId)) { + return null; + } + if (Strings.isNullOrEmpty(urlDomain())) { + return resourceId; + } + return "//" + urlDomain() + "/" + resourceId; + } + + ApiTracerContext withResourceNameExtractor( + @Nullable RequestT request, + @Nullable com.google.api.gax.rpc.ResourceNameExtractor extractor) { + if (extractor == null || request == null) { + return this; + } + return toBuilder() + .setDestinationResourceIdSupplier( + () -> { + try { + return extractor.extract(request); + } catch (Exception e) { + return null; + } + }) + .build(); + } /** * @return a map of attributes to be included in attempt-level spans @@ -284,8 +322,8 @@ ApiTracerContext merge(ApiTracerContext other) { if (!Strings.isNullOrEmpty(other.urlDomain())) { builder.setUrlDomain(other.urlDomain()); } - if (other.destinationResourceId() != null) { - builder.setDestinationResourceId(other.destinationResourceId()); + if (other.destinationResourceIdSupplier() != null) { + builder.setDestinationResourceIdSupplier(other.destinationResourceIdSupplier()); } return builder.build(); } @@ -322,7 +360,8 @@ public abstract static class Builder { public abstract Builder setUrlDomain(@Nullable String urlDomain); - public abstract Builder setDestinationResourceId(@Nullable String destinationResourceId); + public abstract Builder setDestinationResourceIdSupplier( + @Nullable Supplier destinationResourceIdSupplier); public abstract ApiTracerContext build(); } diff --git a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedUnaryCallable.java b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedUnaryCallable.java index 99fe33c99fed..7c8a4f71c350 100644 --- a/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedUnaryCallable.java +++ b/sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedUnaryCallable.java @@ -37,8 +37,6 @@ import com.google.api.gax.rpc.ResourceNameExtractor; import com.google.api.gax.rpc.UnaryCallable; import com.google.api.gax.tracing.ApiTracerFactory.OperationType; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Strings; import com.google.common.util.concurrent.MoreExecutors; import javax.annotation.Nullable; @@ -90,8 +88,10 @@ public TracedUnaryCallable( public ApiFuture futureCall(RequestT request, ApiCallContext context) { ApiTracer tracer; if (apiTracerContext != null) { - ApiTracerContext finalContext = extractResourceNameToApiTracerContext(request); - tracer = tracerFactory.newTracer(context.getTracer(), finalContext); + tracer = + tracerFactory.newTracer( + context.getTracer(), + apiTracerContext.withResourceNameExtractor(request, resourceNameExtractor)); } else { tracer = tracerFactory.newTracer(context.getTracer(), spanName, OperationType.Unary); } @@ -108,15 +108,4 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) throw e; } } - - @VisibleForTesting - ApiTracerContext extractResourceNameToApiTracerContext(RequestT request) { - ApiTracerContext finalContext = apiTracerContext; - String resourceName = - resourceNameExtractor != null ? resourceNameExtractor.extract(request) : null; - if (!Strings.isNullOrEmpty(resourceName)) { - finalContext = finalContext.toBuilder().setDestinationResourceId(resourceName).build(); - } - return finalContext; - } } diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java index 5a2c015e72eb..6b1109ae8f42 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java @@ -310,7 +310,7 @@ void testGetAttemptAttributes_destinationResourceId() { ApiTracerContext context = ApiTracerContext.newBuilder() .setLibraryMetadata(LibraryMetadata.empty()) - .setDestinationResourceId("projects/123/instances/abc") + .setDestinationResourceIdSupplier(() -> "projects/123/instances/abc") .build(); Map attributes = context.getAttemptAttributes(); @@ -383,13 +383,13 @@ void testMerge_destinationResourceId() { ApiTracerContext context1 = ApiTracerContext.newBuilder() .setLibraryMetadata(LibraryMetadata.empty()) - .setDestinationResourceId("name1") + .setDestinationResourceIdSupplier(() -> "name1") .build(); ApiTracerContext context2 = ApiTracerContext.newBuilder() .setLibraryMetadata(LibraryMetadata.empty()) - .setDestinationResourceId("name2") + .setDestinationResourceIdSupplier(() -> "name2") .build(); ApiTracerContext merged = context1.merge(context2); @@ -434,4 +434,121 @@ void testMerge_httpFields() { assertThat(merged.httpMethod()).isEqualTo("GET"); assertThat(merged.httpPathTemplate()).isEqualTo("v1/projects/{project}"); } + + @Test + void testDestinationResourceId_withUrlDomain() { + ApiTracerContext context = + ApiTracerContext.newBuilder() + .setLibraryMetadata(LibraryMetadata.empty()) + .setUrlDomain("compute.googleapis.com") + .setDestinationResourceIdSupplier(() -> "projects/my-project/locations/us-central1") + .build(); + + assertThat(context.destinationResourceId()) + .isEqualTo("//compute.googleapis.com/projects/my-project/locations/us-central1"); + } + + @Test + void testDestinationResourceId_withoutUrlDomain() { + ApiTracerContext context = + ApiTracerContext.newBuilder() + .setLibraryMetadata(LibraryMetadata.empty()) + .setDestinationResourceIdSupplier(() -> "projects/my-project/locations/us-central1") + .build(); + + assertThat(context.destinationResourceId()) + .isEqualTo("projects/my-project/locations/us-central1"); + } + + @Test + void testWithResourceNameExtractor_nullExtractor() { + ApiTracerContext context = ApiTracerContext.empty(); + ApiTracerContext result = context.withResourceNameExtractor("request", null); + assertThat(result).isSameInstanceAs(context); + } + + @Test + void testWithResourceNameExtractor_extractorReturnsNull() { + ApiTracerContext context = ApiTracerContext.empty(); + ApiTracerContext result = context.withResourceNameExtractor("request", req -> null); + assertThat(result.destinationResourceId()).isNull(); + } + + @Test + void testWithResourceNameExtraction_lazyExtractor() { + ApiTracerContext context = ApiTracerContext.empty(); + boolean[] extracted = {false}; + ApiTracerContext result = + context.withResourceNameExtractor( + "request", + req -> { + extracted[0] = true; + return "extracted-id"; + }); + + assertThat(extracted[0]).isFalse(); // Should be lazily evaluated + assertThat(result.destinationResourceId()).isEqualTo("extracted-id"); + assertThat(extracted[0]).isTrue(); + } + + @Test + void testWithResourceNameExtractor_extractorThrowsException() { + ApiTracerContext context = ApiTracerContext.empty(); + ApiTracerContext result = + context.withResourceNameExtractor( + "request", + req -> { + throw new RuntimeException("Intentional mock extraction failure"); + }); + + assertThat(result.destinationResourceId()).isNull(); + } + + @Test + void testDestinationResourceId_emptyIdWithUrlDomain() { + ApiTracerContext context = + ApiTracerContext.newBuilder() + .setLibraryMetadata(LibraryMetadata.empty()) + .setUrlDomain("compute.googleapis.com") + .setDestinationResourceIdSupplier(() -> "") + .build(); + + assertThat(context.destinationResourceId()).isNull(); + } + + @Test + void testMerge_preservesLaziness() { + ApiTracerContext context1 = ApiTracerContext.empty(); + boolean[] extracted = {false}; + ApiTracerContext context2 = + ApiTracerContext.empty() + .withResourceNameExtractor( + "request", + req -> { + extracted[0] = true; + return "lazy-id"; + }); + + ApiTracerContext merged = context1.merge(context2); + assertThat(extracted[0]).isFalse(); // Should not be evaluated during merge + assertThat(merged.destinationResourceId()).isEqualTo("lazy-id"); + assertThat(extracted[0]).isTrue(); // Evaluated upon calling getter + } + + @Test + void testDestinationResourceId_evaluatedEveryTime() { + ApiTracerContext context = ApiTracerContext.empty(); + int[] counter = {0}; + ApiTracerContext result = + context.withResourceNameExtractor( + "request", + req -> { + counter[0]++; + return "extracted-id-" + counter[0]; + }); + + assertThat(result.destinationResourceId()).isEqualTo("extracted-id-1"); + assertThat(result.destinationResourceId()).isEqualTo("extracted-id-2"); + assertThat(counter[0]).isEqualTo(2); + } } diff --git a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedUnaryCallableTest.java b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedUnaryCallableTest.java index 15e637ebffcc..cedaf9a0d39f 100644 --- a/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedUnaryCallableTest.java +++ b/sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedUnaryCallableTest.java @@ -208,30 +208,4 @@ void testResourceNameExtractorUsed() { assertThat(contextCaptor.getValue().destinationResourceId()) .isEqualTo("extracted-resource-name"); } - - @Test - void testExtractResourceNameToApiTracerContext_nullExtractor() { - tracedUnaryCallable = - new TracedUnaryCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT, null); - ApiTracerContext context = tracedUnaryCallable.extractResourceNameToApiTracerContext("request"); - assertThat(context).isEqualTo(TRACER_CONTEXT); - } - - @Test - void testExtractResourceNameToApiTracerContext_extractorReturnsNull() { - tracedUnaryCallable = - new TracedUnaryCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT, request -> null); - ApiTracerContext context = tracedUnaryCallable.extractResourceNameToApiTracerContext("request"); - assertThat(context).isEqualTo(TRACER_CONTEXT); - } - - @Test - void testExtractResourceNameToApiTracerContext_extractorReturnsResourceId() { - tracedUnaryCallable = - new TracedUnaryCallable<>( - innerCallable, tracerFactory, TRACER_CONTEXT, request -> "extracted-id"); - ApiTracerContext context = tracedUnaryCallable.extractResourceNameToApiTracerContext("request"); - assertThat(context).isNotSameInstanceAs(TRACER_CONTEXT); - assertThat(context.destinationResourceId()).isEqualTo("extracted-id"); - } }