From 0a88b0a6e2e94f042b981e58316a0b450a835af3 Mon Sep 17 00:00:00 2001 From: blakeli Date: Wed, 1 Apr 2026 23:14:16 -0400 Subject: [PATCH 1/2] refactor: implement lazy resource name evaluation in ApiTracerContext --- .../api/gax/tracing/ApiTracerContext.java | 49 ++++++- .../api/gax/tracing/TracedUnaryCallable.java | 19 +-- .../api/gax/tracing/ApiTracerContextTest.java | 123 +++++++++++++++++- .../gax/tracing/TracedUnaryCallableTest.java | 26 ---- 4 files changed, 168 insertions(+), 49 deletions(-) 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..5026cded54d6 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 @@ -164,9 +164,47 @@ String rpcSystemName() { @Nullable public abstract String urlDomain(); - /** The destination resource id of the request (e.g. projects/p/locations/l/topics/t). */ + @InternalApi @Nullable - public abstract String destinationResourceId(); + protected abstract java.util.function.Supplier destinationResourceIdSupplier(); + + /** + * The destination resource id of the request (e.g. + * //pubsub.googleapis.com/projects/p/locations/l/topics/t). + */ + @Nullable + public String destinationResourceId() { + java.util.function.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; + } + + public ApiTracerContext withResourceNameExtraction( + @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 java.util.function.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..39a6e678f4b4 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.withResourceNameExtraction(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..476b7338ce29 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 testWithResourceNameExtraction_nullExtractor() { + ApiTracerContext context = ApiTracerContext.empty(); + ApiTracerContext result = context.withResourceNameExtraction("request", null); + assertThat(result).isSameInstanceAs(context); + } + + @Test + void testWithResourceNameExtraction_extractorReturnsNull() { + ApiTracerContext context = ApiTracerContext.empty(); + ApiTracerContext result = context.withResourceNameExtraction("request", req -> null); + assertThat(result.destinationResourceId()).isNull(); + } + + @Test + void testWithResourceNameExtraction_lazyExtraction() { + ApiTracerContext context = ApiTracerContext.empty(); + boolean[] extracted = {false}; + ApiTracerContext result = + context.withResourceNameExtraction( + "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 testWithResourceNameExtraction_extractorThrowsException() { + ApiTracerContext context = ApiTracerContext.empty(); + ApiTracerContext result = + context.withResourceNameExtraction( + "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() + .withResourceNameExtraction( + "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.withResourceNameExtraction( + "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"); - } } From 8a41b769d1fe728149f0677dcccef7123a203393 Mon Sep 17 00:00:00 2001 From: blakeli Date: Wed, 1 Apr 2026 23:21:42 -0400 Subject: [PATCH 2/2] refactor --- .../api/gax/tracing/ApiTracerContext.java | 10 +++++----- .../api/gax/tracing/TracedUnaryCallable.java | 2 +- .../api/gax/tracing/ApiTracerContextTest.java | 20 +++++++++---------- 3 files changed, 16 insertions(+), 16 deletions(-) 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 5026cded54d6..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,8 @@ String rpcSystemName() { @Nullable public abstract String urlDomain(); - @InternalApi @Nullable - protected abstract java.util.function.Supplier destinationResourceIdSupplier(); + protected abstract Supplier destinationResourceIdSupplier(); /** * The destination resource id of the request (e.g. @@ -174,7 +174,7 @@ String rpcSystemName() { */ @Nullable public String destinationResourceId() { - java.util.function.Supplier supplier = destinationResourceIdSupplier(); + Supplier supplier = destinationResourceIdSupplier(); if (supplier == null) { return null; } @@ -188,7 +188,7 @@ public String destinationResourceId() { return "//" + urlDomain() + "/" + resourceId; } - public ApiTracerContext withResourceNameExtraction( + ApiTracerContext withResourceNameExtractor( @Nullable RequestT request, @Nullable com.google.api.gax.rpc.ResourceNameExtractor extractor) { if (extractor == null || request == null) { @@ -361,7 +361,7 @@ public abstract static class Builder { public abstract Builder setUrlDomain(@Nullable String urlDomain); public abstract Builder setDestinationResourceIdSupplier( - @Nullable java.util.function.Supplier destinationResourceIdSupplier); + @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 39a6e678f4b4..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 @@ -91,7 +91,7 @@ public ApiFuture futureCall(RequestT request, ApiCallContext context) tracer = tracerFactory.newTracer( context.getTracer(), - apiTracerContext.withResourceNameExtraction(request, resourceNameExtractor)); + apiTracerContext.withResourceNameExtractor(request, resourceNameExtractor)); } else { tracer = tracerFactory.newTracer(context.getTracer(), spanName, OperationType.Unary); } 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 476b7338ce29..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 @@ -461,25 +461,25 @@ void testDestinationResourceId_withoutUrlDomain() { } @Test - void testWithResourceNameExtraction_nullExtractor() { + void testWithResourceNameExtractor_nullExtractor() { ApiTracerContext context = ApiTracerContext.empty(); - ApiTracerContext result = context.withResourceNameExtraction("request", null); + ApiTracerContext result = context.withResourceNameExtractor("request", null); assertThat(result).isSameInstanceAs(context); } @Test - void testWithResourceNameExtraction_extractorReturnsNull() { + void testWithResourceNameExtractor_extractorReturnsNull() { ApiTracerContext context = ApiTracerContext.empty(); - ApiTracerContext result = context.withResourceNameExtraction("request", req -> null); + ApiTracerContext result = context.withResourceNameExtractor("request", req -> null); assertThat(result.destinationResourceId()).isNull(); } @Test - void testWithResourceNameExtraction_lazyExtraction() { + void testWithResourceNameExtraction_lazyExtractor() { ApiTracerContext context = ApiTracerContext.empty(); boolean[] extracted = {false}; ApiTracerContext result = - context.withResourceNameExtraction( + context.withResourceNameExtractor( "request", req -> { extracted[0] = true; @@ -492,10 +492,10 @@ void testWithResourceNameExtraction_lazyExtraction() { } @Test - void testWithResourceNameExtraction_extractorThrowsException() { + void testWithResourceNameExtractor_extractorThrowsException() { ApiTracerContext context = ApiTracerContext.empty(); ApiTracerContext result = - context.withResourceNameExtraction( + context.withResourceNameExtractor( "request", req -> { throw new RuntimeException("Intentional mock extraction failure"); @@ -522,7 +522,7 @@ void testMerge_preservesLaziness() { boolean[] extracted = {false}; ApiTracerContext context2 = ApiTracerContext.empty() - .withResourceNameExtraction( + .withResourceNameExtractor( "request", req -> { extracted[0] = true; @@ -540,7 +540,7 @@ void testDestinationResourceId_evaluatedEveryTime() { ApiTracerContext context = ApiTracerContext.empty(); int[] counter = {0}; ApiTracerContext result = - context.withResourceNameExtraction( + context.withResourceNameExtractor( "request", req -> { counter[0]++;