Skip to content

Commit 1b10e02

Browse files
diegomarquezpdiegomarquezp
authored
feat(o11y): introduce server.port attribute (#4128)
<img width="1338" height="899" alt="image" src="https://github.com/user-attachments/assets/84af9d15-a09d-433b-8193-8c0741ad31bc" />
1 parent 0d98c37 commit 1b10e02

File tree

13 files changed

+160
-27
lines changed

13 files changed

+160
-27
lines changed

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ClientContext.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ public static ClientContext create(StubSettings settings) throws IOException {
274274
ApiTracerContext apiTracerContext =
275275
ApiTracerContext.newBuilder()
276276
.setServerAddress(endpointContext.resolvedServerAddress())
277+
.setServerPort(endpointContext.resolvedServerPort())
277278
.setLibraryMetadata(settings.getLibraryMetadata())
278279
.build();
279280
ApiTracerFactory apiTracerFactory = settings.getTracerFactory();

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/EndpointContext.java‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ public static EndpointContext getDefaultInstance() {
136136

137137
public abstract String resolvedServerAddress();
138138

139+
@Nullable
140+
public abstract Integer resolvedServerPort();
141+
139142
public abstract Builder toBuilder();
140143

141144
public static Builder newBuilder() {
@@ -233,6 +236,8 @@ public abstract static class Builder {
233236

234237
public abstract Builder setResolvedServerAddress(String serverAddress);
235238

239+
public abstract Builder setResolvedServerPort(Integer serverPort);
240+
236241
public abstract Builder setResolvedUniverseDomain(String resolvedUniverseDomain);
237242

238243
abstract Builder setUseS2A(boolean useS2A);
@@ -264,6 +269,8 @@ public abstract static class Builder {
264269

265270
abstract String resolvedUniverseDomain();
266271

272+
abstract String resolvedEndpoint();
273+
267274
abstract EndpointContext autoBuild();
268275

269276
private String determineUniverseDomain() {
@@ -388,19 +395,38 @@ boolean shouldUseS2A() {
388395
}
389396

390397
private String parseServerAddress(String endpoint) {
391-
if (Strings.isNullOrEmpty(endpoint)) {
398+
if (endpoint.isEmpty()) {
392399
return endpoint;
393400
}
401+
HostAndPort hostAndPort = parseServerHostAndPort(endpoint);
402+
if (hostAndPort == null) {
403+
return null;
404+
}
405+
return hostAndPort.getHost();
406+
}
407+
408+
private Integer parseServerPort(String endpoint) {
409+
if (endpoint.isEmpty()) {
410+
return null;
411+
}
412+
HostAndPort hostAndPort = parseServerHostAndPort(endpoint);
413+
if (!hostAndPort.hasPort()) {
414+
return null;
415+
}
416+
return hostAndPort.getPort();
417+
}
418+
419+
private HostAndPort parseServerHostAndPort(String endpoint) {
394420
String hostPort = endpoint;
395421
if (hostPort.contains("://")) {
396422
// Strip the scheme if present. HostAndPort doesn't support schemes.
397423
hostPort = hostPort.substring(hostPort.indexOf("://") + 3);
398424
}
399425
try {
400-
return HostAndPort.fromString(hostPort).getHost();
426+
return HostAndPort.fromString(hostPort);
401427
} catch (IllegalArgumentException e) {
402428
// Fallback for cases HostAndPort can't handle.
403-
return hostPort;
429+
return null;
404430
}
405431
}
406432

@@ -440,7 +466,8 @@ public EndpointContext build() throws IOException {
440466
setResolvedUniverseDomain(determineUniverseDomain());
441467
String endpoint = determineEndpoint();
442468
setResolvedEndpoint(endpoint);
443-
setResolvedServerAddress(parseServerAddress(endpoint));
469+
setResolvedServerAddress(parseServerAddress(resolvedEndpoint()));
470+
setResolvedServerPort(parseServerPort(resolvedEndpoint()));
444471
setUseS2A(shouldUseS2A());
445472
return autoBuild();
446473
}

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.google.api.core.InternalApi;
3434
import com.google.api.gax.rpc.LibraryMetadata;
3535
import com.google.auto.value.AutoValue;
36+
import com.google.common.base.Strings;
3637
import java.util.HashMap;
3738
import java.util.Map;
3839
import javax.annotation.Nullable;
@@ -49,20 +50,26 @@ public abstract class ApiTracerContext {
4950
@Nullable
5051
public abstract String serverAddress();
5152

53+
@Nullable
54+
public abstract Integer serverPort();
55+
5256
public abstract LibraryMetadata libraryMetadata();
5357

5458
/**
5559
* @return a map of attributes to be included in attempt-level spans
5660
*/
57-
public Map<String, String> getAttemptAttributes() {
58-
Map<String, String> attributes = new HashMap<>();
59-
if (serverAddress() != null) {
61+
public Map<String, Object> getAttemptAttributes() {
62+
Map<String, Object> attributes = new HashMap<>();
63+
if (!Strings.isNullOrEmpty(serverAddress())) {
6064
attributes.put(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, serverAddress());
6165
}
62-
if (libraryMetadata().repository() != null) {
66+
if (serverPort() != null) {
67+
attributes.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, serverPort());
68+
}
69+
if (!Strings.isNullOrEmpty(libraryMetadata().repository())) {
6370
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
6471
}
65-
if (libraryMetadata().artifactName() != null) {
72+
if (!Strings.isNullOrEmpty(libraryMetadata().artifactName())) {
6673
attributes.put(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName());
6774
}
6875
return attributes;
@@ -82,6 +89,8 @@ public abstract static class Builder {
8289

8390
public abstract Builder setLibraryMetadata(LibraryMetadata gapicProperties);
8491

92+
public abstract Builder setServerPort(Integer serverPort);
93+
8594
public abstract ApiTracerContext build();
8695
}
8796
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ public class ObservabilityAttributes {
4444
/** The address of the server being called (e.g., "pubsub.googleapis.com"). */
4545
public static final String SERVER_ADDRESS_ATTRIBUTE = "server.address";
4646

47+
/** The port of the server being called (e.g., 443). */
48+
public static final String SERVER_PORT_ATTRIBUTE = "server.port";
49+
4750
/** The repository of the client library (e.g., "googleapis/google-cloud-java"). */
4851
public static final String REPO_ATTRIBUTE = "gcp.client.repo";
4952

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,21 @@ public OpenTelemetryTraceManager(OpenTelemetry openTelemetry) {
5151
}
5252

5353
@Override
54-
public Span createSpan(String name, Map<String, String> attributes) {
54+
public Span createSpan(String name, Map<String, Object> attributes) {
5555
SpanBuilder spanBuilder = tracer.spanBuilder(name);
5656

5757
// Attempt spans are of the CLIENT kind
5858
spanBuilder.setSpanKind(SpanKind.CLIENT);
5959

6060
if (attributes != null) {
61-
attributes.forEach((k, v) -> spanBuilder.setAttribute(k, v));
61+
attributes.forEach(
62+
(k, v) -> {
63+
if (v instanceof String) {
64+
spanBuilder.setAttribute(k, (String) v);
65+
} else if (v instanceof Integer) {
66+
spanBuilder.setAttribute(k, (long) (Integer) v);
67+
}
68+
});
6269
}
6370

6471
io.opentelemetry.api.trace.Span span = spanBuilder.startSpan();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class SpanTracer implements ApiTracer {
4848
public static final String DEFAULT_LANGUAGE = "Java";
4949

5050
private final TraceManager traceManager;
51-
private final Map<String, String> attemptAttributes;
51+
private final Map<String, Object> attemptAttributes;
5252
private final String attemptSpanName;
5353
private final ApiTracerContext apiTracerContext;
5454
private TraceManager.Span attemptHandle;
@@ -75,7 +75,7 @@ private void buildAttributes() {
7575

7676
@Override
7777
public void attemptStarted(Object request, int attemptNumber) {
78-
Map<String, String> attemptAttributes = new HashMap<>(this.attemptAttributes);
78+
Map<String, Object> attemptAttributes = new HashMap<>(this.attemptAttributes);
7979
// Start the specific attempt span with the operation span as parent
8080
this.attemptHandle = traceManager.createSpan(attemptSpanName, attemptAttributes);
8181
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
@InternalApi
4343
public interface TraceManager {
4444
/** Starts a span and returns a handle to manage its lifecycle. */
45-
Span createSpan(String name, Map<String, String> attributes);
45+
Span createSpan(String name, Map<String, Object> attributes);
4646

4747
interface Span {
4848
void end();

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/rpc/EndpointContextTest.java‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ void shouldUseS2A_success() throws IOException {
595595
}
596596

597597
@Test
598-
void endpointContextBuild_resolvesPortAndServerAddress() throws IOException {
598+
void endpointContextBuild_resolvesServerAddress() throws IOException {
599599
String endpoint = "http://localhost:7469";
600600
EndpointContext endpointContext =
601601
defaultEndpointContextBuilder
@@ -638,4 +638,49 @@ void endpointContextBuild_resolvesPortAndServerAddress() throws IOException {
638638
.build();
639639
Truth.assertThat(endpointContext.resolvedServerAddress()).isEqualTo("2001:db8::1");
640640
}
641+
642+
@Test
643+
void endpointContextBuild_resolvesPort() throws IOException {
644+
String endpoint = "http://localhost:7469";
645+
EndpointContext endpointContext =
646+
defaultEndpointContextBuilder
647+
.setClientSettingsEndpoint(endpoint)
648+
.setTransportChannelProviderEndpoint(null)
649+
.build();
650+
Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(7469);
651+
652+
endpoint = "localhost:7469";
653+
endpointContext =
654+
defaultEndpointContextBuilder
655+
.setClientSettingsEndpoint(endpoint)
656+
.setTransportChannelProviderEndpoint(null)
657+
.build();
658+
Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(7469);
659+
660+
endpoint = "test.googleapis.com:443";
661+
endpointContext =
662+
defaultEndpointContextBuilder
663+
.setClientSettingsEndpoint(endpoint)
664+
.setTransportChannelProviderEndpoint(null)
665+
.build();
666+
Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(443);
667+
668+
// IPv6 literal with port
669+
endpoint = "[2001:db8::1]:443";
670+
endpointContext =
671+
defaultEndpointContextBuilder
672+
.setClientSettingsEndpoint(endpoint)
673+
.setTransportChannelProviderEndpoint(null)
674+
.build();
675+
Truth.assertThat(endpointContext.resolvedServerPort()).isEqualTo(443);
676+
677+
// Bare IPv6 literal (no port)
678+
endpoint = "2001:db8::1";
679+
endpointContext =
680+
defaultEndpointContextBuilder
681+
.setClientSettingsEndpoint(endpoint)
682+
.setTransportChannelProviderEndpoint(null)
683+
.build();
684+
Truth.assertThat(endpointContext.resolvedServerPort()).isNull();
685+
}
641686
}

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void testGetAttemptAttributes_serverAddress() {
4545
.setLibraryMetadata(LibraryMetadata.empty())
4646
.setServerAddress("test-address")
4747
.build();
48-
Map<String, String> attributes = context.getAttemptAttributes();
48+
Map<String, Object> attributes = context.getAttemptAttributes();
4949

5050
assertThat(attributes)
5151
.containsEntry(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, "test-address");
@@ -57,7 +57,7 @@ void testGetAttemptAttributes_repo() {
5757
LibraryMetadata.newBuilder().setRepository("test-repo").build();
5858
ApiTracerContext context =
5959
ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build();
60-
Map<String, String> attributes = context.getAttemptAttributes();
60+
Map<String, Object> attributes = context.getAttemptAttributes();
6161

6262
assertThat(attributes).containsEntry(ObservabilityAttributes.REPO_ATTRIBUTE, "test-repo");
6363
}
@@ -68,7 +68,7 @@ void testGetAttemptAttributes_artifact() {
6868
LibraryMetadata.newBuilder().setArtifactName("test-artifact").build();
6969
ApiTracerContext context =
7070
ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build();
71-
Map<String, String> attributes = context.getAttemptAttributes();
71+
Map<String, Object> attributes = context.getAttemptAttributes();
7272

7373
assertThat(attributes)
7474
.containsEntry(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, "test-artifact");
@@ -77,7 +77,21 @@ void testGetAttemptAttributes_artifact() {
7777
@Test
7878
void testGetAttemptAttributes_empty() {
7979
ApiTracerContext context = ApiTracerContext.empty();
80-
Map<String, String> attributes = context.getAttemptAttributes();
80+
Map<String, Object> attributes = context.getAttemptAttributes();
81+
82+
assertThat(attributes).isEmpty();
83+
}
84+
85+
@Test
86+
void testGetAttemptAttributes_emptyStrings() {
87+
LibraryMetadata libraryMetadata =
88+
LibraryMetadata.newBuilder().setRepository("").setArtifactName("").build();
89+
ApiTracerContext context =
90+
ApiTracerContext.newBuilder()
91+
.setLibraryMetadata(libraryMetadata)
92+
.setServerAddress("")
93+
.build();
94+
Map<String, Object> attributes = context.getAttemptAttributes();
8195

8296
assertThat(attributes).isEmpty();
8397
}

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/OpenTelemetryTraceManagerTest.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
package com.google.api.gax.tracing;
3232

33+
import static org.mockito.ArgumentMatchers.anyLong;
3334
import static org.mockito.ArgumentMatchers.anyString;
3435
import static org.mockito.Mockito.verify;
3536
import static org.mockito.Mockito.when;
@@ -90,7 +91,7 @@ void testCreateSpan_attempt_isClient() {
9091
@Test
9192
void testCreateSpan_recordsSpan() {
9293
String spanName = "test-span";
93-
Map<String, String> attributes = ImmutableMap.of("key1", "value1");
94+
Map<String, Object> attributes = ImmutableMap.of("key1", "value1");
9495

9596
when(tracer.spanBuilder(spanName)).thenReturn(spanBuilder);
9697
when(spanBuilder.setSpanKind(SpanKind.CLIENT)).thenReturn(spanBuilder);
@@ -102,4 +103,19 @@ void testCreateSpan_recordsSpan() {
102103

103104
verify(span).end();
104105
}
106+
107+
@Test
108+
void testCreateSpan_recordsIntegerAttribute() {
109+
String spanName = "test-span";
110+
Map<String, Object> attributes = ImmutableMap.of("port", 443);
111+
112+
when(tracer.spanBuilder(spanName)).thenReturn(spanBuilder);
113+
when(spanBuilder.setSpanKind(SpanKind.CLIENT)).thenReturn(spanBuilder);
114+
when(spanBuilder.setAttribute(anyString(), anyLong())).thenReturn(spanBuilder);
115+
when(spanBuilder.startSpan()).thenReturn(span);
116+
117+
recorder.createSpan(spanName, attributes);
118+
119+
verify(spanBuilder).setAttribute("port", 443L);
120+
}
105121
}

0 commit comments

Comments
 (0)