Skip to content

Commit 914f97f

Browse files
blakeli0blakeli0
authored
fix: Clean up attributes for traces and metrics (#12677)
This PR - Add `exception.type` and `gcp.client.repo` attributes to metrics. - Remove `gcp.client.artifact` and `gcp.client.version` from Span attributes since they are recorded as instrumentationName and instrumentationScope. - Refactor common logics of getting error attributes to `ObservabilityUtils`
1 parent 00f57f4 commit 914f97f

File tree

10 files changed

+174
-166
lines changed

10 files changed

+174
-166
lines changed

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -220,17 +220,8 @@ public Map<String, Object> getAttemptAttributes() {
220220
if (rpcSystemName() != null) {
221221
attributes.put(ObservabilityAttributes.RPC_SYSTEM_NAME_ATTRIBUTE, rpcSystemName());
222222
}
223-
if (!libraryMetadata().isEmpty()) {
224-
if (!Strings.isNullOrEmpty(libraryMetadata().repository())) {
225-
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
226-
}
227-
if (!Strings.isNullOrEmpty(libraryMetadata().artifactName())) {
228-
attributes.put(
229-
ObservabilityAttributes.ARTIFACT_ATTRIBUTE, libraryMetadata().artifactName());
230-
}
231-
if (!Strings.isNullOrEmpty(libraryMetadata().version())) {
232-
attributes.put(ObservabilityAttributes.VERSION_ATTRIBUTE, libraryMetadata().version());
233-
}
223+
if (!libraryMetadata().isEmpty() && !Strings.isNullOrEmpty(libraryMetadata().repository())) {
224+
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
234225
}
235226
if (transport() == Transport.GRPC && !Strings.isNullOrEmpty(fullMethodName())) {
236227
attributes.put(ObservabilityAttributes.GRPC_RPC_METHOD_ATTRIBUTE, fullMethodName());
@@ -264,6 +255,9 @@ Map<String, Object> getMetricsAttributes() {
264255
if (serverPort() != null) {
265256
attributes.put(ObservabilityAttributes.SERVER_PORT_ATTRIBUTE, serverPort());
266257
}
258+
if (!libraryMetadata().isEmpty() && !Strings.isNullOrEmpty(libraryMetadata().repository())) {
259+
attributes.put(ObservabilityAttributes.REPO_ATTRIBUTE, libraryMetadata().repository());
260+
}
267261
if (!Strings.isNullOrEmpty(serviceName())) {
268262
attributes.put(ObservabilityAttributes.GCP_CLIENT_SERVICE_ATTRIBUTE, serviceName());
269263
}

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,25 +75,23 @@ class GoldenSignalsMetricsTracer implements ApiTracer {
7575
*/
7676
@Override
7777
public void operationSucceeded() {
78-
ObservabilityUtils.populateStatusAttributes(attributes, null, transport);
79-
metricsRecorder.recordOperationLatency(
80-
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
78+
recordMetric(null);
8179
}
8280

8381
@Override
8482
public void operationCancelled() {
85-
recordError(new CancellationException());
83+
recordMetric(new CancellationException());
8684
}
8785

8886
@Override
8987
public void operationFailed(Throwable error) {
90-
recordError(error);
88+
recordMetric(error);
9189
}
9290

93-
private void recordError(Throwable error) {
94-
ObservabilityUtils.populateStatusAttributes(attributes, error, transport);
95-
attributes.put(
96-
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, ObservabilityUtils.extractErrorType(error));
91+
private void recordMetric(Throwable error) {
92+
Map<String, Object> responseAttributes =
93+
ObservabilityUtils.getResponseAttributes(error, transport);
94+
attributes.putAll(responseAttributes);
9795
metricsRecorder.recordOperationLatency(
9896
clientRequestTimer.elapsed(TimeUnit.NANOSECONDS) / 1_000_000_000.0, attributes);
9997
}

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,6 @@ public class ObservabilityAttributes {
5050
/** The repository of the client library (e.g., "googleapis/google-cloud-java"). */
5151
public static final String REPO_ATTRIBUTE = "gcp.client.repo";
5252

53-
/** The artifact name of the client library (e.g., "google-cloud-vision"). */
54-
public static final String ARTIFACT_ATTRIBUTE = "gcp.client.artifact";
55-
56-
/** The version of the client library (e.g., "1.2.3"). */
57-
public static final String VERSION_ATTRIBUTE = "gcp.client.version";
58-
5953
/** The full RPC method name, including package, service, and method. */
6054
public static final String GRPC_RPC_METHOD_ATTRIBUTE = "rpc.method";
6155

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.rpc.ErrorInfo;
3838
import io.opentelemetry.api.common.Attributes;
3939
import io.opentelemetry.api.common.AttributesBuilder;
40+
import java.util.HashMap;
4041
import java.util.Map;
4142
import java.util.concurrent.CancellationException;
4243
import javax.annotation.Nullable;
@@ -170,16 +171,21 @@ private static String redactSensitiveQueryValues(final String rawQuery) {
170171
return Joiner.on('&').join(redactedParams);
171172
}
172173

173-
static void populateStatusAttributes(
174-
Map<String, Object> attributes,
175-
@Nullable Throwable error,
176-
ApiTracerContext.Transport transport) {
174+
static Map<String, Object> getResponseAttributes(
175+
@Nullable Throwable error, ApiTracerContext.Transport transport) {
176+
Map<String, Object> attributes = new HashMap<>();
177177
StatusCode.Code code = extractStatus(error);
178178
attributes.put(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, code.toString());
179179
if (transport == ApiTracerContext.Transport.HTTP) {
180180
attributes.put(
181181
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE, (long) code.getHttpStatusCode());
182182
}
183+
if (error != null) {
184+
attributes.put(
185+
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, ObservabilityUtils.extractErrorType(error));
186+
attributes.put(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE, error.getClass().getName());
187+
}
188+
return attributes;
183189
}
184190

185191
/** Function to extract the ErrorInfo payload from the error, if available */

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

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -217,26 +217,13 @@ private void recordErrorAndEndAttempt(Throwable error) {
217217
if (attemptSpan == null) {
218218
return;
219219
}
220-
221-
Map<String, Object> statusAttributes = new HashMap<>();
222-
ObservabilityUtils.populateStatusAttributes(
223-
statusAttributes, error, this.apiTracerContext.transport());
224-
if (!statusAttributes.isEmpty()) {
225-
attemptSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(statusAttributes));
226-
}
227-
228-
if (error == null) {
229-
endAttempt();
230-
return;
220+
Map<String, Object> responseAttributes =
221+
ObservabilityUtils.getResponseAttributes(error, this.apiTracerContext.transport());
222+
if (!responseAttributes.isEmpty()) {
223+
attemptSpan.setAllAttributes(ObservabilityUtils.toOtelAttributes(responseAttributes));
231224
}
232225

233-
attemptSpan.setAttribute(
234-
ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE, ObservabilityUtils.extractErrorType(error));
235-
236-
attemptSpan.setAttribute(
237-
ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE, error.getClass().getName());
238-
239-
if (!Strings.isNullOrEmpty(error.getMessage())) {
226+
if (error != null && !Strings.isNullOrEmpty(error.getMessage())) {
240227
attemptSpan.setAttribute(
241228
ObservabilityAttributes.STATUS_MESSAGE_ATTRIBUTE, error.getMessage());
242229
}

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

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -62,28 +62,6 @@ void testGetAttemptAttributes_repo() {
6262
assertThat(attributes).containsEntry(ObservabilityAttributes.REPO_ATTRIBUTE, "test-repo");
6363
}
6464

65-
@Test
66-
void testGetAttemptAttributes_artifact() {
67-
LibraryMetadata libraryMetadata =
68-
LibraryMetadata.newBuilder().setArtifactName("test-artifact").build();
69-
ApiTracerContext context =
70-
ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build();
71-
Map<String, Object> attributes = context.getAttemptAttributes();
72-
73-
assertThat(attributes)
74-
.containsEntry(ObservabilityAttributes.ARTIFACT_ATTRIBUTE, "test-artifact");
75-
}
76-
77-
@Test
78-
void testGetAttemptAttributes_version() {
79-
LibraryMetadata libraryMetadata = LibraryMetadata.newBuilder().setVersion("1.2.3").build();
80-
ApiTracerContext context =
81-
ApiTracerContext.newBuilder().setLibraryMetadata(libraryMetadata).build();
82-
Map<String, Object> attributes = context.getAttemptAttributes();
83-
84-
assertThat(attributes).containsEntry(ObservabilityAttributes.VERSION_ATTRIBUTE, "1.2.3");
85-
}
86-
8765
@Test
8866
void testGetAttemptAttributes_httpMethod() {
8967
ApiTracerContext context =

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ void operationCancelled_shouldRecordsOKStatus() {
138138
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
139139
"CancellationException",
140140
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
141-
StatusCode.Code.CANCELLED.toString()));
141+
StatusCode.Code.CANCELLED.toString(),
142+
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
143+
java.util.concurrent.CancellationException.class.getName()));
142144
}
143145

144146
@Test
@@ -174,7 +176,9 @@ void operationFailed_shouldRecordsOKStatus() {
174176
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
175177
"INTERNAL",
176178
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
177-
StatusCode.Code.INTERNAL.toString()));
179+
StatusCode.Code.INTERNAL.toString(),
180+
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
181+
com.google.api.gax.rpc.ApiException.class.getName()));
178182
}
179183

180184
@Test
@@ -194,7 +198,9 @@ void operationFailed_shouldRecordCancellationException() {
194198
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
195199
"CancellationException",
196200
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
197-
StatusCode.Code.CANCELLED.toString()));
201+
StatusCode.Code.CANCELLED.toString(),
202+
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
203+
java.util.concurrent.CancellationException.class.getName()));
198204
}
199205

200206
@Test
@@ -213,7 +219,9 @@ void operationFailed_shouldRecordClientTimeout() {
213219
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
214220
"CLIENT_TIMEOUT",
215221
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
216-
StatusCode.Code.UNKNOWN.toString()));
222+
StatusCode.Code.UNKNOWN.toString(),
223+
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
224+
java.net.SocketTimeoutException.class.getName()));
217225
}
218226

219227
@Test
@@ -232,6 +240,8 @@ void operationFailed_shouldRecordClientRequestError() {
232240
AttributeKey.stringKey(ObservabilityAttributes.ERROR_TYPE_ATTRIBUTE),
233241
"CLIENT_REQUEST_ERROR",
234242
AttributeKey.stringKey(RPC_RESPONSE_STATUS_ATTRIBUTE),
235-
StatusCode.Code.UNKNOWN.toString()));
243+
StatusCode.Code.UNKNOWN.toString(),
244+
AttributeKey.stringKey(ObservabilityAttributes.EXCEPTION_TYPE_ATTRIBUTE),
245+
java.lang.IllegalArgumentException.class.getName()));
236246
}
237247
}

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -115,45 +115,44 @@ void testToOtelAttributes_shouldMapIntAttributes() {
115115
}
116116

117117
@Test
118-
void testPopulateStatusAttributes_grpc_success() {
119-
Map<String, Object> attributes = new java.util.HashMap<>();
120-
ObservabilityUtils.populateStatusAttributes(attributes, null, ApiTracerContext.Transport.GRPC);
118+
void testGetResponseAttributes_grpc_success() {
119+
Map<String, Object> attributes =
120+
ObservabilityUtils.getResponseAttributes(null, ApiTracerContext.Transport.GRPC);
121121
assertThat(attributes)
122122
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "OK");
123123
}
124124

125125
@Test
126-
void testPopulateStatusAttributes_grpc_apiException() {
127-
Map<String, Object> attributes = new java.util.HashMap<>();
126+
void testGetResponseAttributes_grpc_apiException() {
128127
ApiException error =
129128
new ApiException("fake_error", null, new FakeStatusCode(StatusCode.Code.NOT_FOUND), false);
130-
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.GRPC);
129+
Map<String, Object> attributes =
130+
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.GRPC);
131131
assertThat(attributes)
132132
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "NOT_FOUND");
133133
}
134134

135135
@Test
136-
void testPopulateStatusAttributes_grpc_cancellationException() {
137-
Map<String, Object> attributes = new java.util.HashMap<>();
136+
void testGetResponseAttributes_grpc_cancellationException() {
138137
Throwable error = new java.util.concurrent.CancellationException();
139-
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.GRPC);
138+
Map<String, Object> attributes =
139+
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.GRPC);
140140
assertThat(attributes)
141141
.containsEntry(ObservabilityAttributes.RPC_RESPONSE_STATUS_ATTRIBUTE, "CANCELLED");
142142
}
143143

144144
@Test
145-
void testPopulateStatusAttributes_http_success() {
146-
Map<String, Object> attributes = new java.util.HashMap<>();
147-
ObservabilityUtils.populateStatusAttributes(attributes, null, ApiTracerContext.Transport.HTTP);
145+
void testGetResponseAttributes_http_success() {
146+
Map<String, Object> attributes =
147+
ObservabilityUtils.getResponseAttributes(null, ApiTracerContext.Transport.HTTP);
148148
assertThat(attributes)
149149
.containsEntry(
150150
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
151151
(long) StatusCode.Code.OK.getHttpStatusCode());
152152
}
153153

154154
@Test
155-
void testPopulateStatusAttributes_http_apiExceptionWithIntegerTransportCode() {
156-
Map<String, Object> attributes = new java.util.HashMap<>();
155+
void testGetResponseAttributes_http_apiExceptionWithIntegerTransportCode() {
157156
ApiException error =
158157
new ApiException(
159158
"fake_error",
@@ -170,16 +169,16 @@ public Object getTransportCode() {
170169
}
171170
},
172171
false);
173-
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.HTTP);
172+
Map<String, Object> attributes =
173+
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.HTTP);
174174
assertThat(attributes)
175175
.containsEntry(
176176
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
177177
(long) StatusCode.Code.NOT_FOUND.getHttpStatusCode());
178178
}
179179

180180
@Test
181-
void testPopulateStatusAttributes_http_apiExceptionWithNonIntegerTransportCode() {
182-
Map<String, Object> attributes = new java.util.HashMap<>();
181+
void testGetResponseAttributes_http_apiExceptionWithNonIntegerTransportCode() {
183182
ApiException error =
184183
new ApiException(
185184
"fake_error",
@@ -196,18 +195,19 @@ public Object getTransportCode() {
196195
}
197196
},
198197
false);
199-
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.HTTP);
198+
Map<String, Object> attributes =
199+
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.HTTP);
200200
assertThat(attributes)
201201
.containsEntry(
202202
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,
203203
(long) StatusCode.Code.NOT_FOUND.getHttpStatusCode());
204204
}
205205

206206
@Test
207-
void testPopulateStatusAttributes_http_cancellationException() {
208-
Map<String, Object> attributes = new java.util.HashMap<>();
207+
void testGetResponseAttributes_http_cancellationException() {
209208
Throwable error = new java.util.concurrent.CancellationException();
210-
ObservabilityUtils.populateStatusAttributes(attributes, error, ApiTracerContext.Transport.HTTP);
209+
Map<String, Object> attributes =
210+
ObservabilityUtils.getResponseAttributes(error, ApiTracerContext.Transport.HTTP);
211211
assertThat(attributes)
212212
.containsEntry(
213213
ObservabilityAttributes.HTTP_RESPONSE_STATUS_ATTRIBUTE,

0 commit comments

Comments
 (0)