Skip to content

Commit ea86930

Browse files
diegomarquezpdiegomarquezp
committed
Merge remote-tracking branch 'origin/main' into observability/tracing-attr/rpc.method-and-rpc.system
2 parents 1b47eb9 + 56aa343 commit ea86930

14 files changed

Lines changed: 186 additions & 42 deletions

File tree

‎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();

‎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
}

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

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.google.api.gax.rpc.LibraryMetadata;
3535
import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
3636
import com.google.auto.value.AutoValue;
37+
import com.google.common.base.Strings;
3738
import java.util.HashMap;
3839
import java.util.Map;
3940
import javax.annotation.Nullable;
@@ -69,6 +70,16 @@ public enum Transport {
6970
@Nullable
7071
abstract String serverAddress();
7172

73+
/**
74+
* Returns the server port of the RPC.
75+
*
76+
* <p>Example: 1337. This maps to the {@code server.port} attribute.
77+
*
78+
* @return the server port, or {@code null} if not set
79+
*/
80+
@Nullable
81+
public abstract Integer serverPort();
82+
7283
/**
7384
* Returns the library metadata associated with the RPC.
7485
*
@@ -128,25 +139,29 @@ String rpcSystemName() {
128139
/**
129140
* @return a map of attributes to be included in attempt-level spans
130141
*/
131-
Map<String, String> getAttemptAttributes() {
132-
Map<String, String> attributes = new HashMap<>();
133-
if (serverAddress() != null) {
142+
public Map<String, Object> getAttemptAttributes() {
143+
Map<String, Object> attributes = new HashMap<>();
144+
if (!Strings.isNullOrEmpty(serverAddress())) {
134145
attributes.put(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE, serverAddress());
135146
}
147+
if (serverPort() != null) {
148+
attributes.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, serverPort());
149+
}
136150
if (rpcSystemName() != null) {
137151
attributes.put(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, rpcSystemName());
138152
}
139-
if (libraryMetadata().repository() != null) {
140-
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
141-
}
142-
if (libraryMetadata().artifactName() != null) {
143-
attributes.put(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName());
144-
}
145-
if (transport() == Transport.GRPC) {
146-
if (fullMethodName() != null) {
147-
attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
153+
if (!libraryMetadata().isEmpty()) {
154+
if (!Strings.isNullOrEmpty(libraryMetadata().repository())) {
155+
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
156+
}
157+
if (!Strings.isNullOrEmpty(libraryMetadata().artifactName())) {
158+
attributes.put(
159+
ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName());
148160
}
149161
}
162+
if (transport() == Transport.GRPC && fullMethodName() != null) {
163+
attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
164+
}
150165
return attributes;
151166
}
152167

@@ -158,13 +173,16 @@ Map<String, String> getAttemptAttributes() {
158173
*/
159174
ApiTracerContext merge(ApiTracerContext other) {
160175
Builder builder = toBuilder();
161-
if (other.serverAddress() != null) {
176+
if (!Strings.isNullOrEmpty(other.serverAddress())) {
162177
builder.setServerAddress(other.serverAddress());
163178
}
179+
if (other.serverPort() != null) {
180+
builder.setServerPort(other.serverPort());
181+
}
164182
if (!other.libraryMetadata().isEmpty()) {
165183
builder.setLibraryMetadata(other.libraryMetadata());
166184
}
167-
if (other.fullMethodName() != null) {
185+
if (!Strings.isNullOrEmpty(other.fullMethodName())) {
168186
builder.setFullMethodName(other.fullMethodName());
169187
}
170188
if (other.transport() != null) {
@@ -198,6 +216,8 @@ public abstract static class Builder {
198216

199217
public abstract Builder setOperationType(@Nullable OperationType operationType);
200218

219+
public abstract Builder setServerPort(@Nullable Integer serverPort);
220+
201221
public abstract ApiTracerContext build();
202222
}
203223
}

‎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

‎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();

‎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
}

‎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();

‎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
}

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

Lines changed: 21 additions & 7 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");
@@ -81,7 +81,7 @@ void testGetAttemptAttributes_fullMethodName_noTransport_notPresent() {
8181
.setLibraryMetadata(LibraryMetadata.empty())
8282
.setFullMethodName("google.pubsub.v1.Publisher/Publish")
8383
.build();
84-
Map<String, String> attributes = context.getAttemptAttributes();
84+
Map<String, Object> attributes = context.getAttemptAttributes();
8585

8686
assertThat(attributes).doesNotContainKey(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE);
8787
}
@@ -94,7 +94,7 @@ void testGetAttemptAttributes_fullMethodName_grpcTransport_present() {
9494
.setFullMethodName("google.pubsub.v1.Publisher/Publish")
9595
.setTransport(ApiTracerContext.Transport.GRPC)
9696
.build();
97-
Map<String, String> attributes = context.getAttemptAttributes();
97+
Map<String, Object> attributes = context.getAttemptAttributes();
9898

9999
assertThat(attributes)
100100
.containsEntry(
@@ -109,15 +109,29 @@ void testGetAttemptAttributes_rpcSystemName() {
109109
.setLibraryMetadata(LibraryMetadata.empty())
110110
.setTransport(ApiTracerContext.Transport.GRPC)
111111
.build();
112-
Map<String, String> attributes = context.getAttemptAttributes();
112+
Map<String, Object> attributes = context.getAttemptAttributes();
113113

114114
assertThat(attributes).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "grpc");
115115
}
116116

117117
@Test
118118
void testGetAttemptAttributes_empty() {
119119
ApiTracerContext context = ApiTracerContext.empty();
120-
Map<String, String> attributes = context.getAttemptAttributes();
120+
Map<String, Object> attributes = context.getAttemptAttributes();
121+
122+
assertThat(attributes).isEmpty();
123+
}
124+
125+
@Test
126+
void testGetAttemptAttributes_emptyStrings() {
127+
LibraryMetadata libraryMetadata =
128+
LibraryMetadata.newBuilder().setRepository("").setArtifactName("").build();
129+
ApiTracerContext context =
130+
ApiTracerContext.newBuilder()
131+
.setLibraryMetadata(libraryMetadata)
132+
.setServerAddress("")
133+
.build();
134+
Map<String, Object> attributes = context.getAttemptAttributes();
121135

122136
assertThat(attributes).isEmpty();
123137
}

0 commit comments

Comments
 (0)