Skip to content

fix: Clean up attributes for traces and metrics #12677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blakeli0 merged 6 commits into from
Apr 6, 2026
Merged

fix: Clean up attributes for traces and metrics #12677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
86b9e94
fix: update observability attributes and tests
blakeli0 Apr 3, 2026
04bcc1f
fix: format
blakeli0 Apr 3, 2026
a062871
fix: format
blakeli0 Apr 3, 2026
944d05c
fix: fix typo
blakeli0 Apr 3, 2026
f33bbdd
fix: rename
blakeli0 Apr 4, 2026
7d35ea9
fix: rename
blakeli0 Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions ...platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ApiTracerContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,17 +220,8 @@ public Map<String, Object> getAttemptAttributes() {
if (rpcSystemName() != null) {
attributes.put(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, rpcSystemName());
}
if (!libraryMetadata().isEmpty()) {
if (!Strings.isNullOrEmpty(libraryMetadata().repository())) {
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
}
if (!Strings.isNullOrEmpty(libraryMetadata().artifactName())) {
attributes.put(
ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName());
}
if (!Strings.isNullOrEmpty(libraryMetadata().version())) {
attributes.put(ObservabilityAttributes.VERSION_ATTRIBUTE, libraryMetadata().version());
}
if (!libraryMetadata().isEmpty() && !Strings.isNullOrEmpty(libraryMetadata().repository())) {
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
}
if (transport() == Transport.GRPC && !Strings.isNullOrEmpty(fullMethodName())) {
attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
Expand Down Expand Up @@ -264,6 +255,9 @@ Map<String, Object> getMetricsAttributes() {
if (serverPort() != null) {
attributes.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, serverPort());
}
if (!libraryMetadata().isEmpty() && !Strings.isNullOrEmpty(libraryMetadata().repository())) {
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
}
if (!Strings.isNullOrEmpty(serviceName())) {
attributes.put(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, serviceName());
}
Expand Down
16 changes: 7 additions & 9 deletions ...ava/gax-java/gax/src/main/java/com/google/api/gax/tracing/GoldenSignalsMetricsTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,25 +75,23 @@ class GoldenSignalsMetricsTracer implements ApiTracer {
*/
@Override
public void operationSucceeded() {
ObservabilityUtils.populateStatusAttributes(attributes, null, transport);
metricsRecorder.recordOperationLatency(
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
recordMetric(null);
Copy link
Copy Markdown
Member

@lqiu96 lqiu96 Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a blocker for this PR, but I feel that recordMetric(null) gives the wrong impression of the logic (maybe it's just to me and I may be just reading it too fast). At a first glance, I read the method like record no metric. Only after reading the signature did it make sense that the first param correspond to the error.

Maybe ideally something like recordErrorMetric that takes in the error param and recordMetric that records the status? But I guess that requires a bit of duplication in the logic.

Copy link
Copy Markdown
Contributor Author

@blakeli0 blakeli0 Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I can definitely see the potential confusion. Let me give it more thoughts.

}

@Override
public void operationCancelled() {
recordError(new CancellationException());
recordMetric(new CancellationException());
}

@Override
public void operationFailed(Throwable error) {
recordError(error);
recordMetric(error);
}

private void recordError(Throwable error) {
ObservabilityUtils.populateStatusAttributes(attributes, error, transport);
attributes.put(
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, ObservabilityUtils.extractErrorType(error));
private void recordMetric(Throwable error) {
Map<String, Object> responseAttributes =
ObservabilityUtils.getResponseAttributes(error, transport);
attributes.putAll(responseAttributes);
metricsRecorder.recordOperationLatency(
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
}
Expand Down
6 changes: 0 additions & 6 deletions ...m-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityAttributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ public class ObservabilityAttributes {
/** The repository of the client library (e.g., "googleapis/google-cloud-java"). */
public static final String REPO_ATTRIBUTE = "gcp.client.repo";

/** The artifact name of the client library (e.g., "google-cloud-vision"). */
public static final String ARTIFACT_ATTRIBUTE = "gcp.client.artifact";

/** The version of the client library (e.g., "1.2.3"). */
public static final String VERSION_ATTRIBUTE = "gcp.client.version";

/** The full RPC method name, including package, service, and method. */
public static final String GRPC_RPC_METHOD_ATTRIBUTE = "rpc.method";

Expand Down
14 changes: 10 additions & 4 deletions ...atform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/ObservabilityUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.google.rpc.ErrorInfo;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CancellationException;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -170,16 +171,21 @@ private static String redactSensitiveQueryValues(final String rawQuery) {
return Joiner.on('&').join(redactedParams);
}

static void populateStatusAttributes(
Map<String, Object> attributes,
@Nullable Throwable error,
ApiTracerContext.Transport transport) {
static Map<String, Object> getResponseAttributes(
@Nullable Throwable error, ApiTracerContext.Transport transport) {
Map<String, Object> attributes = new HashMap<>();
StatusCode.Code code = extractStatus(error);
attributes.put(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, code.toString());
if (transport == ApiTracerContext.Transport.HTTP) {
attributes.put(
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE, (long) code.getHttpStatusCode());
}
if (error != null) {
attributes.put(
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, ObservabilityUtils.extractErrorType(error));
attributes.put(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE, error.getClass().getName());
}
return attributes;
}

/** Function to extract the ErrorInfo payload from the error, if available */
Expand Down
23 changes: 5 additions & 18 deletions sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/SpanTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,26 +217,13 @@ private void recordErrorAndEndAttempt(Throwable error) {
if (attemptSpan == null) {
return;
}

Map<String, Object> statusAttributes = new HashMap<>();
ObservabilityUtils.populateStatusAttributes(
statusAttributes, error, this.apiTracerContext.transport());
if (!statusAttributes.isEmpty()) {
attemptSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(statusAttributes));
}

if (error == null) {
endAttempt();
return;
Map<String, Object> responseAttributes =
ObservabilityUtils.getResponseAttributes(error, this.apiTracerContext.transport());
if (!responseAttributes.isEmpty()) {
attemptSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes));
}

attemptSpan.setAttribute(
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, ObservabilityUtils.extractErrorType(error));

attemptSpan.setAttribute(
ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE, error.getClass().getName());

if (!Strings.isNullOrEmpty(error.getMessage())) {
if (error != null && !Strings.isNullOrEmpty(error.getMessage())) {
attemptSpan.setAttribute(
ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage());
}
Expand Down
22 changes: 0 additions & 22 deletions ...form-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ApiTracerContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,6 @@ void testGetAttemptAttributes_repo() {
assertThat(attributes).containsEntry(ObservabilityAttributes.REPO_ATTRIBUTE, "test-repo");
}

@Test
void testGetAttemptAttributes_artifact() {
LibraryMetadata libraryMetadata =
LibraryMetadata.newBuilder().setArtifactName("test-artifact").build();
ApiTracerContext context =
ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build();
Map<String, Object> attributes = context.getAttemptAttributes();

assertThat(attributes)
.containsEntry(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, "test-artifact");
}

@Test
void testGetAttemptAttributes_version() {
LibraryMetadata libraryMetadata = LibraryMetadata.newBuilder().setVersion("1.2.3").build();
ApiTracerContext context =
ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build();
Map<String, Object> attributes = context.getAttemptAttributes();

assertThat(attributes).containsEntry(ObservabilityAttributes.VERSION_ATTRIBUTE, "1.2.3");
}

@Test
void testGetAttemptAttributes_httpMethod() {
ApiTracerContext context =
Expand Down
20 changes: 15 additions & 5 deletions ...gax-java/gax/src/test/java/com/google/api/gax/tracing/GoldenSignalsMetricsTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ void operationCancelled_shouldRecordsOKStatus() {
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
"CancellationException",
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
StatusCode.Code.CANCELLED.toString()));
StatusCode.Code.CANCELLED.toString(),
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
java.util.concurrent.CancellationException.class.getName()));
}

@Test
Expand Down Expand Up @@ -174,7 +176,9 @@ void operationFailed_shouldRecordsOKStatus() {
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
"INTERNAL",
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
StatusCode.Code.INTERNAL.toString()));
StatusCode.Code.INTERNAL.toString(),
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
com.google.api.gax.rpc.ApiException.class.getName()));
}

@Test
Expand All @@ -194,7 +198,9 @@ void operationFailed_shouldRecordCancellationException() {
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
"CancellationException",
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
StatusCode.Code.CANCELLED.toString()));
StatusCode.Code.CANCELLED.toString(),
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
java.util.concurrent.CancellationException.class.getName()));
}

@Test
Expand All @@ -213,7 +219,9 @@ void operationFailed_shouldRecordClientTimeout() {
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
"CLIENT_TIMEOUT",
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
StatusCode.Code.UNKNOWN.toString()));
StatusCode.Code.UNKNOWN.toString(),
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
java.net.SocketTimeoutException.class.getName()));
}

@Test
Expand All @@ -232,6 +240,8 @@ void operationFailed_shouldRecordClientRequestError() {
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
"CLIENT_REQUEST_ERROR",
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
StatusCode.Code.UNKNOWN.toString()));
StatusCode.Code.UNKNOWN.toString(),
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
java.lang.IllegalArgumentException.class.getName()));
}
}
42 changes: 21 additions & 21 deletions ...rm-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/ObservabilityUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,45 +115,44 @@ void testToOtelAttributes_shouldMapIntAttributes() {
}

@Test
void testPopulateStatusAttributes_grpc_success() {
Map<String, Object> attributes = new java.util.HashMap<>();
ObservabilityUtils.populateStatusAttributes(attributes, null, ApiTracerContext.Transport.GRPC);
void testGetResponseAttributes_grpc_success() {
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(null, ApiTracerContext.Transport.GRPC);
assertThat(attributes)
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "OK");
}

@Test
void testPopulateStatusAttributes_grpc_apiException() {
Map<String, Object> attributes = new java.util.HashMap<>();
void testGetResponseAttributes_grpc_apiException() {
ApiException error =
new ApiException("fake_error", null, new FakeStatusCode(StatusCode.Code.NOT_FOUND), false);
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.GRPC);
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.GRPC);
assertThat(attributes)
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "NOT_FOUND");
}

@Test
void testPopulateStatusAttributes_grpc_cancellationException() {
Map<String, Object> attributes = new java.util.HashMap<>();
void testGetResponseAttributes_grpc_cancellationException() {
Throwable error = new java.util.concurrent.CancellationException();
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.GRPC);
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.GRPC);
assertThat(attributes)
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "CANCELLED");
}

@Test
void testPopulateStatusAttributes_http_success() {
Map<String, Object> attributes = new java.util.HashMap<>();
ObservabilityUtils.populateStatusAttributes(attributes, null, ApiTracerContext.Transport.HTTP);
void testGetResponseAttributes_http_success() {
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(null, ApiTracerContext.Transport.HTTP);
assertThat(attributes)
.containsEntry(
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
(long) StatusCode.Code.OK.getHttpStatusCode());
}

@Test
void testPopulateStatusAttributes_http_apiExceptionWithIntegerTransportCode() {
Map<String, Object> attributes = new java.util.HashMap<>();
void testGetResponseAttributes_http_apiExceptionWithIntegerTransportCode() {
ApiException error =
new ApiException(
"fake_error",
Expand All @@ -170,16 +169,16 @@ public Object getTransportCode() {
}
},
false);
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.HTTP);
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.HTTP);
assertThat(attributes)
.containsEntry(
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
(long) StatusCode.Code.NOT_FOUND.getHttpStatusCode());
}

@Test
void testPopulateStatusAttributes_http_apiExceptionWithNonIntegerTransportCode() {
Map<String, Object> attributes = new java.util.HashMap<>();
void testGetResponseAttributes_http_apiExceptionWithNonIntegerTransportCode() {
ApiException error =
new ApiException(
"fake_error",
Expand All @@ -196,18 +195,19 @@ public Object getTransportCode() {
}
},
false);
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.HTTP);
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.HTTP);
assertThat(attributes)
.containsEntry(
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
(long) StatusCode.Code.NOT_FOUND.getHttpStatusCode());
}

@Test
void testPopulateStatusAttributes_http_cancellationException() {
Map<String, Object> attributes = new java.util.HashMap<>();
void testGetResponseAttributes_http_cancellationException() {
Throwable error = new java.util.concurrent.CancellationException();
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.HTTP);
Map<String, Object> attributes =
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.HTTP);
assertThat(attributes)
.containsEntry(
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
Expand Down
Loading
Loading