diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java index 0328efca16..43fdd848b6 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java @@ -274,6 +274,7 @@ public static ClientContext create(StubSettings settings) throws IOException { ApiTracerContext apiTracerContext = ApiTracerContext.newBuilder() .setServerAddress(endpointContext.resolvedServerAddress()) + .setServerPort(endpointContext.resolvedServerPort()) .setLibraryMetadata(settings.getLibraryMetadata()) .build(); ApiTracerFactory apiTracerFactory = settings.getTracerFactory(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/rpc/EndpointContext.java b/gax-java/gax/src/main/java/com/google/api/gax/rpc/EndpointContext.java index 84111dd620..09e105cceb 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/rpc/EndpointContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/rpc/EndpointContext.java @@ -136,6 +136,9 @@ public static EndpointContext getDefaultInstance() { public abstract String resolvedServerAddress(); + @Nullable + public abstract Integer resolvedServerPort(); + public abstract Builder toBuilder(); public static Builder newBuilder() { @@ -233,6 +236,8 @@ public abstract static class Builder { public abstract Builder setResolvedServerAddress(String serverAddress); + public abstract Builder setResolvedServerPort(Integer serverPort); + public abstract Builder setResolvedUniverseDomain(String resolvedUniverseDomain); abstract Builder setUseS2A(boolean useS2A); @@ -264,6 +269,8 @@ public abstract static class Builder { abstract String resolvedUniverseDomain(); + abstract String resolvedEndpoint(); + abstract EndpointContext autoBuild(); private String determineUniverseDomain() { @@ -388,19 +395,38 @@ boolean shouldUseS2A() { } private String parseServerAddress(String endpoint) { - if (Strings.isNullOrEmpty(endpoint)) { + if (endpoint.isEmpty()) { return endpoint; } + HostAndPort hostAndPort = parseServerHostAndPort(endpoint); + if (hostAndPort == null) { + return null; + } + return hostAndPort.getHost(); + } + + private Integer parseServerPort(String endpoint) { + if (endpoint.isEmpty()) { + return null; + } + HostAndPort hostAndPort = parseServerHostAndPort(endpoint); + if (!hostAndPort.hasPort()) { + return null; + } + return hostAndPort.getPort(); + } + + private HostAndPort parseServerHostAndPort(String endpoint) { String hostPort = endpoint; if (hostPort.contains("://")) { // Strip the scheme if present. HostAndPort doesn't support schemes. hostPort = hostPort.substring(hostPort.indexOf("://") + 3); } try { - return HostAndPort.fromString(hostPort).getHost(); + return HostAndPort.fromString(hostPort); } catch (IllegalArgumentException e) { // Fallback for cases HostAndPort can't handle. - return hostPort; + return null; } } @@ -440,7 +466,8 @@ public EndpointContext build() throws IOException { setResolvedUniverseDomain(determineUniverseDomain()); String endpoint = determineEndpoint(); setResolvedEndpoint(endpoint); - setResolvedServerAddress(parseServerAddress(endpoint)); + setResolvedServerAddress(parseServerAddress(resolvedEndpoint())); + setResolvedServerPort(parseServerPort(resolvedEndpoint())); setUseS2A(shouldUseS2A()); return autoBuild(); } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java index c0e2f86cc1..073b6ef90c 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java @@ -33,6 +33,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.rpc.LibraryMetadata; import com.google.auto.value.AutoValue; +import com.google.common.base.Strings; import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; @@ -49,20 +50,26 @@ public abstract class ApiTracerContext { @Nullable public abstract String serverAddress(); + @Nullable + public abstract Integer serverPort(); + public abstract LibraryMetadata libraryMetadata(); /** * @return a map of attributes to be included in attempt-level spans */ - public Map getAttemptAttributes() { - Map attributes = new HashMap<>(); - if (serverAddress() != null) { + public Map getAttemptAttributes() { + Map attributes = new HashMap<>(); + if (!Strings.isNullOrEmpty(serverAddress())) { attributes.put(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, serverAddress()); } - if (libraryMetadata().repository() != null) { + if (serverPort() != null) { + attributes.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, serverPort()); + } + if (!Strings.isNullOrEmpty(libraryMetadata().repository())) { attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository()); } - if (libraryMetadata().artifactName() != null) { + if (!Strings.isNullOrEmpty(libraryMetadata().artifactName())) { attributes.put(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName()); } return attributes; @@ -82,6 +89,8 @@ public abstract static class Builder { public abstract Builder setLibraryMetadata(LibraryMetadata gapicProperties); + public abstract Builder setServerPort(Integer serverPort); + public abstract ApiTracerContext build(); } } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java index 072dcdc098..6815cb5fda 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java @@ -44,6 +44,9 @@ public class ObservabilityAttributes { /** The address of the server being called (e.g., "pubsub.googleapis.com"). */ public static final String SERVER_ADDRESS_ATTRIBUTE = "server.address"; + /** The port of the server being called (e.g., 443). */ + public static final String SERVER_PORT_ATTRIBUTE = "server.port"; + /** The repository of the client library (e.g., "googleapis/google-cloud-java"). */ public static final String REPO_ATTRIBUTE = "gcp.client.repo"; diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTraceManager.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTraceManager.java index fbbdb292b0..923b524648 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTraceManager.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/OpenTelemetryTraceManager.java @@ -51,14 +51,21 @@ public OpenTelemetryTraceManager(OpenTelemetry openTelemetry) { } @Override - public Span createSpan(String name, Map attributes) { + public Span createSpan(String name, Map attributes) { SpanBuilder spanBuilder = tracer.spanBuilder(name); // Attempt spans are of the CLIENT kind spanBuilder.setSpanKind(SpanKind.CLIENT); if (attributes != null) { - attributes.forEach((k, v) -> spanBuilder.setAttribute(k, v)); + attributes.forEach( + (k, v) -> { + if (v instanceof String) { + spanBuilder.setAttribute(k, (String) v); + } else if (v instanceof Integer) { + spanBuilder.setAttribute(k, (long) (Integer) v); + } + }); } io.opentelemetry.api.trace.Span span = spanBuilder.startSpan(); diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java index 7bbe435d8b..c5c28aebe0 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java @@ -48,7 +48,7 @@ public class SpanTracer implements ApiTracer { public static final String DEFAULT_LANGUAGE = "Java"; private final TraceManager traceManager; - private final Map attemptAttributes; + private final Map attemptAttributes; private final String attemptSpanName; private final ApiTracerContext apiTracerContext; private TraceManager.Span attemptHandle; @@ -75,7 +75,7 @@ private void buildAttributes() { @Override public void attemptStarted(Object request, int attemptNumber) { - Map attemptAttributes = new HashMap<>(this.attemptAttributes); + Map attemptAttributes = new HashMap<>(this.attemptAttributes); // Start the specific attempt span with the operation span as parent this.attemptHandle = traceManager.createSpan(attemptSpanName, attemptAttributes); } diff --git a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceManager.java b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceManager.java index edafd1dfb2..8572d1ce11 100644 --- a/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceManager.java +++ b/gax-java/gax/src/main/java/com/google/api/gax/tracing/TraceManager.java @@ -42,7 +42,7 @@ @InternalApi public interface TraceManager { /** Starts a span and returns a handle to manage its lifecycle. */ - Span createSpan(String name, Map attributes); + Span createSpan(String name, Map attributes); interface Span { void end(); diff --git a/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java index c1bcc50512..99a8f63fcf 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java @@ -595,7 +595,7 @@ void shouldUseS2A_success() throws IOException { } @Test - void endpointContextBuild_resolvesPortAndServerAddress() throws IOException { + void endpointContextBuild_resolvesServerAddress() throws IOException { String endpoint = "http://localhost:7469"; EndpointContext endpointContext = defaultEndpointContextBuilder @@ -638,4 +638,49 @@ void endpointContextBuild_resolvesPortAndServerAddress() throws IOException { .build(); Truth.assertThat(endpointContext.resolvedServerAddress()).isEqualTo("2001:db8::1"); } + + @Test + void endpointContextBuild_resolvesPort() throws IOException { + String endpoint = "http://localhost:7469"; + EndpointContext endpointContext = + defaultEndpointContextBuilder + .setClientSettingsEndpoint(endpoint) + .setTransportChannelProviderEndpoint(null) + .build(); + Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(7469); + + endpoint = "localhost:7469"; + endpointContext = + defaultEndpointContextBuilder + .setClientSettingsEndpoint(endpoint) + .setTransportChannelProviderEndpoint(null) + .build(); + Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(7469); + + endpoint = "test.googleapis.com:443"; + endpointContext = + defaultEndpointContextBuilder + .setClientSettingsEndpoint(endpoint) + .setTransportChannelProviderEndpoint(null) + .build(); + Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(443); + + // IPv6 literal with port + endpoint = "[2001:db8::1]:443"; + endpointContext = + defaultEndpointContextBuilder + .setClientSettingsEndpoint(endpoint) + .setTransportChannelProviderEndpoint(null) + .build(); + Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(443); + + // Bare IPv6 literal (no port) + endpoint = "2001:db8::1"; + endpointContext = + defaultEndpointContextBuilder + .setClientSettingsEndpoint(endpoint) + .setTransportChannelProviderEndpoint(null) + .build(); + Truth.assertThat(endpointContext.resolvedServerPort()).isNull(); + } } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java index 94d0ff3ed4..cc40fffecd 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java @@ -45,7 +45,7 @@ void testGetAttemptAttributes_serverAddress() { .setLibraryMetadata(LibraryMetadata.empty()) .setServerAddress("test-address") .build(); - Map attributes = context.getAttemptAttributes(); + Map attributes = context.getAttemptAttributes(); assertThat(attributes) .containsEntry(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, "test-address"); @@ -57,7 +57,7 @@ void testGetAttemptAttributes_repo() { LibraryMetadata.newBuilder().setRepository("test-repo").build(); ApiTracerContext context = ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build(); - Map attributes = context.getAttemptAttributes(); + Map attributes = context.getAttemptAttributes(); assertThat(attributes).containsEntry(ObservabilityAttributes.REPO_ATTRIBUTE, "test-repo"); } @@ -68,7 +68,7 @@ void testGetAttemptAttributes_artifact() { LibraryMetadata.newBuilder().setArtifactName("test-artifact").build(); ApiTracerContext context = ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build(); - Map attributes = context.getAttemptAttributes(); + Map attributes = context.getAttemptAttributes(); assertThat(attributes) .containsEntry(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, "test-artifact"); @@ -77,7 +77,21 @@ void testGetAttemptAttributes_artifact() { @Test void testGetAttemptAttributes_empty() { ApiTracerContext context = ApiTracerContext.empty(); - Map attributes = context.getAttemptAttributes(); + Map attributes = context.getAttemptAttributes(); + + assertThat(attributes).isEmpty(); + } + + @Test + void testGetAttemptAttributes_emptyStrings() { + LibraryMetadata libraryMetadata = + LibraryMetadata.newBuilder().setRepository("").setArtifactName("").build(); + ApiTracerContext context = + ApiTracerContext.newBuilder() + .setLibraryMetadata(libraryMetadata) + .setServerAddress("") + .build(); + Map attributes = context.getAttemptAttributes(); assertThat(attributes).isEmpty(); } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTraceManagerTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTraceManagerTest.java index 9dc692904c..1e466cfb20 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTraceManagerTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTraceManagerTest.java @@ -30,6 +30,7 @@ package com.google.api.gax.tracing; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -90,7 +91,7 @@ void testCreateSpan_attempt_isClient() { @Test void testCreateSpan_recordsSpan() { String spanName = "test-span"; - Map attributes = ImmutableMap.of("key1", "value1"); + Map attributes = ImmutableMap.of("key1", "value1"); when(tracer.spanBuilder(spanName)).thenReturn(spanBuilder); when(spanBuilder.setSpanKind(SpanKind.CLIENT)).thenReturn(spanBuilder); @@ -102,4 +103,19 @@ void testCreateSpan_recordsSpan() { verify(span).end(); } + + @Test + void testCreateSpan_recordsIntegerAttribute() { + String spanName = "test-span"; + Map attributes = ImmutableMap.of("port", 443); + + when(tracer.spanBuilder(spanName)).thenReturn(spanBuilder); + when(spanBuilder.setSpanKind(SpanKind.CLIENT)).thenReturn(spanBuilder); + when(spanBuilder.setAttribute(anyString(), anyLong())).thenReturn(spanBuilder); + when(spanBuilder.startSpan()).thenReturn(span); + + recorder.createSpan(spanName, attributes); + + verify(spanBuilder).setAttribute("port", 443L); + } } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerFactoryTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerFactoryTest.java index 66b75f5eee..ad614ddc53 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerFactoryTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerFactoryTest.java @@ -76,10 +76,10 @@ void testNewTracer_addsAttributes() { tracer.attemptStarted(null, 1); - ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); verify(recorder, atLeastOnce()).createSpan(anyString(), attributesCaptor.capture()); - Map attemptAttributes = attributesCaptor.getValue(); + Map attemptAttributes = attributesCaptor.getValue(); assertThat(attemptAttributes).containsEntry("server.address", "test-address"); } @@ -104,10 +104,10 @@ void testWithContext_addsInferredAttributes() { tracer.attemptStarted(null, 1); - ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); verify(recorder, atLeastOnce()).createSpan(anyString(), attributesCaptor.capture()); - Map attemptAttributes = attributesCaptor.getValue(); + Map attemptAttributes = attributesCaptor.getValue(); assertThat(attemptAttributes) .containsEntry(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, "example.com"); } @@ -129,10 +129,10 @@ void testWithContext_noEndpointContext_doesNotAddAttributes() { tracer.attemptStarted(null, 1); - ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); verify(recorder, atLeastOnce()).createSpan(anyString(), attributesCaptor.capture()); - Map attemptAttributes = attributesCaptor.getValue(); + Map attemptAttributes = attributesCaptor.getValue(); assertThat(attemptAttributes) .doesNotContainKey(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE); } diff --git a/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerTest.java b/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerTest.java index 3f47a87de1..b5e6100fe6 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/tracing/SpanTracerTest.java @@ -70,7 +70,7 @@ void testAttemptStarted_includesLanguageAttribute() { tracer.attemptStarted(new Object(), 1); - ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); + ArgumentCaptor> attributesCaptor = ArgumentCaptor.forClass(Map.class); verify(recorder).createSpan(eq(ATTEMPT_SPAN_NAME), attributesCaptor.capture()); assertThat(attributesCaptor.getValue()) diff --git a/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java b/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java index 3dffc13baf..218e6dd2a3 100644 --- a/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java +++ b/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelTracing.java @@ -54,6 +54,7 @@ class ITOtelTracing { private static final String SHOWCASE_SERVER_ADDRESS = "localhost"; + private static final long SHOWCASE_SERVER_PORT = 7469; private static final String SHOWCASE_REPO = "googleapis/sdk-platform-java"; private static final String SHOWCASE_ARTIFACT = "com.google.cloud:gapic-showcase"; @@ -110,6 +111,11 @@ void testTracing_successfulEcho_grpc() throws Exception { .getAttributes() .get(AttributeKey.stringKey(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE))) .isEqualTo(SHOWCASE_SERVER_ADDRESS); + assertThat( + attemptSpan + .getAttributes() + .get(AttributeKey.longKey(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE))) + .isEqualTo(SHOWCASE_SERVER_PORT); assertThat( attemptSpan .getAttributes() @@ -152,6 +158,11 @@ void testTracing_successfulEcho_httpjson() throws Exception { .getAttributes() .get(AttributeKey.stringKey(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE))) .isEqualTo(SHOWCASE_SERVER_ADDRESS); + assertThat( + attemptSpan + .getAttributes() + .get(AttributeKey.longKey(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE))) + .isEqualTo(SHOWCASE_SERVER_PORT); assertThat( attemptSpan .getAttributes()