Skip to content

Commit 12920a6

Browse files
diegomarquezpgemini-code-assist[bot]blakeli0
authored
impl(o11y): introduce http.request.method and span name for HTTP requests (#4127)
This PR introduces enhancements to the observability layer (tracing and metrics) by capturing additional context during RPC calls. Specifically, it adds support for: 1. **HTTP Method Attribute**: Capturing `http.request.method` for REST-based transports. 2. **Library Metadata**: Capturing `gcp.client.repo` and `gcp.client.artifact` to identify the source library of the RPC. ## Verification Results - Verified that `http.request.method` is correctly populated for REST calls in Showcase integration tests. - Cloud Trace screenshot: <img width="1352" height="753" alt="image" src="https://github.com/user-attachments/assets/b17c171b-704d-4468-9f91-95b3f6363ebc" /> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Blake Li <blakeli@google.com>
1 parent 7eca195 commit 12920a6

File tree

10 files changed

+215
-47
lines changed

10 files changed

+215
-47
lines changed

‎gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import com.google.api.gax.rpc.UnaryCallSettings;
4646
import com.google.api.gax.rpc.UnaryCallable;
4747
import com.google.api.gax.tracing.ApiTracerContext;
48-
import com.google.api.gax.tracing.SpanName;
4948
import com.google.api.gax.tracing.TracedUnaryCallable;
5049
import javax.annotation.Nonnull;
5150

@@ -83,7 +82,7 @@ static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createUnaryCalla
8382
new TracedUnaryCallable<>(
8483
callable,
8584
clientContext.getTracerFactory(),
86-
getSpanName(httpJsonCallSettings.getMethodDescriptor()));
85+
getApiTracerContext(httpJsonCallSettings.getMethodDescriptor()));
8786
return callable.withDefaultCallContext(clientContext.getDefaultCallContext());
8887
}
8988

@@ -222,13 +221,13 @@ ServerStreamingCallable<RequestT, ResponseT> createServerStreamingCallable(
222221
}
223222

224223
@InternalApi("Visible for testing")
225-
static SpanName getSpanName(@Nonnull ApiMethodDescriptor<?, ?> methodDescriptor) {
226-
ApiTracerContext apiTracerContext =
227-
ApiTracerContext.newBuilder()
228-
.setFullMethodName(methodDescriptor.getFullMethodName())
229-
.setTransport(ApiTracerContext.Transport.HTTP)
230-
.setLibraryMetadata(LibraryMetadata.empty())
231-
.build();
232-
return SpanName.of(apiTracerContext);
224+
static ApiTracerContext getApiTracerContext(@Nonnull ApiMethodDescriptor<?, ?> methodDescriptor) {
225+
return ApiTracerContext.newBuilder()
226+
.setFullMethodName(methodDescriptor.getFullMethodName())
227+
.setHttpMethod(methodDescriptor.getHttpMethod())
228+
.setHttpPathTemplate(methodDescriptor.getRequestFormatter().getPathTemplate().toRawString())
229+
.setTransport(ApiTracerContext.Transport.HTTP)
230+
.setLibraryMetadata(LibraryMetadata.empty())
231+
.build();
233232
}
234233
}

‎gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonCallableFactoryTest.java‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
import static com.google.common.truth.Truth.assertWithMessage;
3434

3535
import com.google.api.client.http.HttpMethods;
36+
import com.google.api.gax.tracing.ApiTracerContext;
3637
import com.google.api.gax.tracing.SpanName;
38+
import com.google.api.pathtemplate.PathTemplate;
3739
import com.google.common.collect.ImmutableList;
3840
import com.google.common.collect.ImmutableMap;
3941
import java.util.List;
@@ -43,8 +45,15 @@
4345
import org.mockito.Mockito;
4446

4547
class HttpJsonCallableFactoryTest {
48+
private HttpRequestFormatter createMockRequestFormatter() {
49+
HttpRequestFormatter formatter = Mockito.mock(HttpRequestFormatter.class);
50+
PathTemplate template = PathTemplate.create("/test/path/template");
51+
Mockito.when(formatter.getPathTemplate()).thenReturn(template);
52+
return formatter;
53+
}
54+
4655
@Test
47-
void testGetSpanName() {
56+
void testGetApiTracerContext() {
4857
Map<String, SpanName> validNames =
4958
ImmutableMap.of(
5059
"google.cloud.service.v1.CoolService/CoolRPC", SpanName.of("CoolService", "CoolRPC"),
@@ -56,17 +65,19 @@ void testGetSpanName() {
5665
ApiMethodDescriptor.newBuilder()
5766
.setFullMethodName(entry.getKey())
5867
.setHttpMethod(HttpMethods.POST)
59-
.setRequestFormatter(Mockito.mock(HttpRequestFormatter.class))
68+
.setRequestFormatter(createMockRequestFormatter())
6069
.setResponseParser(Mockito.mock(HttpResponseParser.class))
6170
.build();
6271

63-
SpanName actualSpanName = HttpJsonCallableFactory.getSpanName(descriptor);
64-
assertThat(actualSpanName).isEqualTo(entry.getValue());
72+
ApiTracerContext context = HttpJsonCallableFactory.getApiTracerContext(descriptor);
73+
SpanName actual = SpanName.of(context);
74+
assertThat(actual.getClientName()).isEqualTo(entry.getValue().getClientName());
75+
assertThat(actual.getMethodName()).isEqualTo(entry.getValue().getMethodName());
6576
}
6677
}
6778

6879
@Test
69-
void testGetSpanNameInvalid() {
80+
void testGetApiTracerContextInvalid() {
7081
List<String> invalidNames = ImmutableList.of("no_split", ".no_client");
7182

7283
for (String invalidName : invalidNames) {
@@ -75,16 +86,16 @@ void testGetSpanNameInvalid() {
7586
ApiMethodDescriptor.newBuilder()
7687
.setFullMethodName(invalidName)
7788
.setHttpMethod(HttpMethods.POST)
78-
.setRequestFormatter(Mockito.mock(HttpRequestFormatter.class))
89+
.setRequestFormatter(createMockRequestFormatter())
7990
.setResponseParser(Mockito.mock(HttpResponseParser.class))
8091
.build();
8192

8293
IllegalArgumentException actualError = null;
8394
try {
84-
SpanName spanName = HttpJsonCallableFactory.getSpanName(descriptor);
95+
ApiTracerContext context = HttpJsonCallableFactory.getApiTracerContext(descriptor);
96+
SpanName.of(context);
8597
assertWithMessage(
86-
"Invalid method descriptor should not have a valid span name: %s should not generate the spanName: %s",
87-
invalidName, spanName)
98+
"Invalid method descriptor should not have a valid client name: %s", invalidName)
8899
.fail();
89100
} catch (IllegalArgumentException e) {
90101
actualError = e;

‎gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import com.google.api.gax.rpc.UnaryCallSettings;
5656
import com.google.api.gax.rpc.UnaryCallable;
5757
import com.google.api.gax.rpc.UnknownException;
58+
import com.google.api.pathtemplate.PathTemplate;
5859
import com.google.common.collect.ImmutableSet;
5960
import com.google.common.util.concurrent.Futures;
6061
import com.google.common.util.concurrent.UncheckedExecutionException;
@@ -75,7 +76,7 @@ class RetryingTest {
7576
ApiMethodDescriptor.newBuilder()
7677
.setFullMethodName("google.cloud.v1.Fake/FakeMethodForRequestMutator")
7778
.setHttpMethod(HttpMethods.POST)
78-
.setRequestFormatter(Mockito.mock(HttpRequestFormatter.class))
79+
.setRequestFormatter(createMockRequestFormatter())
7980
.setResponseParser(Mockito.mock(HttpResponseParser.class))
8081
.build();
8182

@@ -113,6 +114,13 @@ class RetryingTest {
113114
.setTotalTimeoutDuration(java.time.Duration.ofMillis(10L))
114115
.build();
115116

117+
private HttpRequestFormatter createMockRequestFormatter() {
118+
HttpRequestFormatter formatter = Mockito.mock(HttpRequestFormatter.class);
119+
PathTemplate template = PathTemplate.create("/test/path/template");
120+
Mockito.when(formatter.getPathTemplate()).thenReturn(template);
121+
return formatter;
122+
}
123+
116124
@BeforeEach
117125
void resetClock() {
118126
fakeClock = new FakeApiClock(System.nanoTime());

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

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,26 @@ String rpcSystemName() {
136136
@Nullable
137137
public abstract OperationType operationType();
138138

139+
/**
140+
* Returns the HTTP method used for the RPC, in case the RPC is an HttpJson method.
141+
*
142+
* <p>Example: {@code PATCH}.
143+
*
144+
* @return the HTTP method, or {@code null} if not set
145+
*/
146+
@Nullable
147+
abstract String httpMethod();
148+
149+
/**
150+
* Returns the HTTP path template used for the RPC, in case the RPC is an HttpJson method.
151+
*
152+
* <p>Example: {@code /users/{user_id}/get}.
153+
*
154+
* @return the HTTP path template, or {@code null} if not set
155+
*/
156+
@Nullable
157+
abstract String httpPathTemplate();
158+
139159
/** The service name of a client (e.g. "bigtable", "spanner"). */
140160
@Nullable
141161
public abstract String serviceName();
@@ -144,10 +164,6 @@ String rpcSystemName() {
144164
@Nullable
145165
public abstract String urlDomain();
146166

147-
/** The url template of the request (e.g. /v1/{name}:access). */
148-
@Nullable
149-
public abstract String urlTemplate();
150-
151167
/**
152168
* @return a map of attributes to be included in attempt-level spans
153169
*/
@@ -174,6 +190,14 @@ public Map<String, Object> getAttemptAttributes() {
174190
if (transport() == Transport.GRPC && !Strings.isNullOrEmpty(fullMethodName())) {
175191
attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
176192
}
193+
if (transport() == Transport.HTTP) {
194+
if (!Strings.isNullOrEmpty(httpMethod())) {
195+
attributes.put(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, httpMethod());
196+
}
197+
if (!Strings.isNullOrEmpty(httpPathTemplate())) {
198+
attributes.put(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, httpPathTemplate());
199+
}
200+
}
177201
return attributes;
178202
}
179203

@@ -198,8 +222,8 @@ Map<String, Object> getMetricsAttributes() {
198222
if (!Strings.isNullOrEmpty(urlDomain())) {
199223
attributes.put(ObservabilityAttributes.URL_DOMAIN_ATTRIBUTE, urlDomain());
200224
}
201-
if (!Strings.isNullOrEmpty(urlTemplate())) {
202-
attributes.put(ObservabilityAttributes.URL_TEMPLATE_ATTRIBUTE, urlTemplate());
225+
if (!Strings.isNullOrEmpty(httpPathTemplate())) {
226+
attributes.put(ObservabilityAttributes.URL_TEMPLATE_ATTRIBUTE, httpPathTemplate());
203227
}
204228
}
205229
return attributes;
@@ -228,18 +252,21 @@ ApiTracerContext merge(ApiTracerContext other) {
228252
if (other.transport() != null) {
229253
builder.setTransport(other.transport());
230254
}
255+
if (!Strings.isNullOrEmpty(other.httpMethod())) {
256+
builder.setHttpMethod(other.httpMethod());
257+
}
258+
if (!Strings.isNullOrEmpty(other.httpPathTemplate())) {
259+
builder.setHttpPathTemplate(other.httpPathTemplate());
260+
}
231261
if (other.operationType() != null) {
232262
builder.setOperationType(other.operationType());
233263
}
234-
if (other.serviceName() != null) {
264+
if (!Strings.isNullOrEmpty(other.serviceName())) {
235265
builder.setServiceName(other.serviceName());
236266
}
237-
if (other.urlDomain() != null) {
267+
if (!Strings.isNullOrEmpty(other.urlDomain())) {
238268
builder.setUrlDomain(other.urlDomain());
239269
}
240-
if (other.urlTemplate() != null) {
241-
builder.setUrlTemplate(other.urlTemplate());
242-
}
243270
return builder.build();
244271
}
245272

@@ -267,12 +294,14 @@ public abstract static class Builder {
267294

268295
public abstract Builder setServerPort(@Nullable Integer serverPort);
269296

297+
public abstract Builder setHttpMethod(@Nullable String httpMethod);
298+
299+
public abstract Builder setHttpPathTemplate(@Nullable String rawString);
300+
270301
public abstract Builder setServiceName(@Nullable String serviceName);
271302

272303
public abstract Builder setUrlDomain(@Nullable String urlDomain);
273304

274-
public abstract Builder setUrlTemplate(@Nullable String urlTemplate);
275-
276305
public abstract ApiTracerContext build();
277306
}
278307
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ public class ObservabilityAttributes {
5959
/** The RPC system name, e.g. 'grpc' or 'http'. */
6060
public static final String RPC_SYSTEM_NAME_ATTRIBUTE = "rpc.system.name";
6161

62+
/** The HTTP method of the request (e.g., "GET"). Only used in HTTP transport. */
63+
public static final String HTTP_METHOD_ATTRIBUTE = "http.request.method";
64+
65+
/**
66+
* The HTTP URL template of the request (e.g. "/v1/{name}:access"). Only used in HTTP transport.
67+
*/
68+
public static final String HTTP_URL_TEMPLATE_ATTRIBUTE = "url.template";
69+
6270
/**
6371
* The error codes of the request. The value will be the string representation of the canonical
6472
* gRPC status code (e.g., "OK", "INTERNAL").

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,22 @@ public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType op
7777

7878
@Override
7979
public ApiTracer newTracer(ApiTracer parent, ApiTracerContext apiTracerContext) {
80-
ApiTracerContext context = this.apiTracerContext.merge(apiTracerContext);
80+
ApiTracerContext mergedContext = this.apiTracerContext.merge(apiTracerContext);
8181

8282
String attemptSpanName;
83-
if (context.transport() == ApiTracerContext.Transport.GRPC) {
84-
attemptSpanName = context.fullMethodName();
83+
if (mergedContext.transport() == ApiTracerContext.Transport.GRPC) {
84+
// gRPC Uses the full method name as span name.
85+
attemptSpanName = mergedContext.fullMethodName();
86+
} else if (mergedContext.httpMethod() == null || mergedContext.httpPathTemplate() == null) {
87+
// HTTP method name without necessary components defaults to the full method name
88+
attemptSpanName = mergedContext.fullMethodName();
8589
} else {
86-
// TODO(diegomarquezp): this is a placeholder for the HTTP span name and will be adjusted as
87-
// the
88-
// feature is developed.
89-
attemptSpanName = context.fullMethodName() + "/attempt";
90+
// We construct the span name with HTTP method and path template.
91+
attemptSpanName =
92+
String.format("%s %s", mergedContext.httpMethod(), mergedContext.httpPathTemplate());
9093
}
9194

92-
return new SpanTracer(traceManager, context, attemptSpanName);
95+
return new SpanTracer(traceManager, mergedContext, attemptSpanName);
9396
}
9497

9598
@Override

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

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,32 @@ void testGetAttemptAttributes_artifact() {
7474
.containsEntry(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, "test-artifact");
7575
}
7676

77+
@Test
78+
void testGetAttemptAttributes_httpMethod() {
79+
ApiTracerContext context =
80+
ApiTracerContext.newBuilder()
81+
.setLibraryMetadata(LibraryMetadata.empty())
82+
.setTransport(ApiTracerContext.Transport.HTTP)
83+
.setHttpMethod("POST")
84+
.build();
85+
Map<String, Object> attributes = context.getAttemptAttributes();
86+
87+
assertThat(attributes).containsEntry(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE, "POST");
88+
}
89+
90+
@Test
91+
void testGetAttemptAttributes_httpMethod_notHttpTransport() {
92+
ApiTracerContext context =
93+
ApiTracerContext.newBuilder()
94+
.setLibraryMetadata(LibraryMetadata.empty())
95+
.setTransport(ApiTracerContext.Transport.GRPC)
96+
.setHttpMethod("POST")
97+
.build();
98+
Map<String, Object> attributes = context.getAttemptAttributes();
99+
100+
assertThat(attributes).doesNotContainKey(ObservabilityAttributes.HTTP_METHOD_ATTRIBUTE);
101+
}
102+
77103
@Test
78104
void testGetAttemptAttributes_fullMethodName_noTransport_notPresent() {
79105
ApiTracerContext context =
@@ -114,6 +140,20 @@ void testGetAttemptAttributes_rpcSystemName() {
114140
assertThat(attributes).containsEntry(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, "grpc");
115141
}
116142

143+
@Test
144+
void testGetAttemptAttributes_httpPathTemplate() {
145+
ApiTracerContext context =
146+
ApiTracerContext.newBuilder()
147+
.setLibraryMetadata(LibraryMetadata.empty())
148+
.setTransport(ApiTracerContext.Transport.HTTP)
149+
.setHttpPathTemplate("the-template")
150+
.build();
151+
Map<String, Object> attributes = context.getAttemptAttributes();
152+
153+
assertThat(attributes)
154+
.containsEntry(ObservabilityAttributes.HTTP_URL_TEMPLATE_ATTRIBUTE, "the-template");
155+
}
156+
117157
@Test
118158
void testGetAttemptAttributes_empty() {
119159
ApiTracerContext context = ApiTracerContext.empty();
@@ -221,7 +261,7 @@ void testGetMetricsAttributes_urlTemplate() {
221261
ApiTracerContext.newBuilder()
222262
.setLibraryMetadata(LibraryMetadata.empty())
223263
.setTransport(ApiTracerContext.Transport.HTTP)
224-
.setUrlTemplate("/v1/test/{template}")
264+
.setHttpPathTemplate("/v1/test/{template}")
225265
.build();
226266
Map<String, Object> attributes = context.getMetricsAttributes();
227267

@@ -248,7 +288,7 @@ void testGetMetricsAttributes_urlTemplate_notHttp() {
248288
ApiTracerContext.newBuilder()
249289
.setLibraryMetadata(LibraryMetadata.empty())
250290
.setTransport(ApiTracerContext.Transport.GRPC)
251-
.setUrlTemplate("/v1/test/{template}")
291+
.setHttpPathTemplate("/v1/test/{template}")
252292
.build();
253293
Map<String, Object> attributes = context.getMetricsAttributes();
254294

@@ -300,4 +340,22 @@ void testMerge_emptyOther() {
300340

301341
assertThat(merged).isEqualTo(context1);
302342
}
343+
344+
@Test
345+
void testMerge_httpFields() {
346+
ApiTracerContext context1 =
347+
ApiTracerContext.newBuilder()
348+
.setLibraryMetadata(LibraryMetadata.empty())
349+
.setHttpMethod("GET")
350+
.build();
351+
ApiTracerContext context2 =
352+
ApiTracerContext.newBuilder()
353+
.setLibraryMetadata(LibraryMetadata.empty())
354+
.setHttpPathTemplate("v1/projects/{project}")
355+
.build();
356+
357+
ApiTracerContext merged = context1.merge(context2);
358+
assertThat(merged.httpMethod()).isEqualTo("GET");
359+
assertThat(merged.httpPathTemplate()).isEqualTo("v1/projects/{project}");
360+
}
303361
}

0 commit comments

Comments
 (0)