Skip to content

Commit a3832a0

Browse files
bric3bric3
committed
fix(ccs): Use captured gRPC status code numeric in metric aggregation
When client-computed stats (CCS) are enabled, the agent **merges** stats it computes itself from raw spans with stats pre-computed by the tracer. For gRPC spans, without Client Computed Stats (metrics) the agent resolves the status code from the span's tags via [`getGRPCStatusCode()`](https://github.com/DataDog/datadog-agent/blob/47938ea8c9b9894dcb03dc3f81cf2c6e408f1b6c/pkg/trace/stats/aggregation.go#L167-L221), which always returns a numeric string (e.g. `4`) or an empty string. With CCS enabled, the code uses [`GRPCStatusCode`](https://github.com/DataDog/datadog-agent/blob/47938ea8c9b9894dcb03dc3f81cf2c6e408f1b6c/pkg/trace/stats/aggregation.go#L160) without translation. This change mimics the aggregation of the agent, and what is expected from the agent, in [`NewAggregationFromGroup`](https://github.com/DataDog/datadog-agent/blob/47938ea8c9b9894dcb03dc3f81cf2c6e408f1b6c/pkg/trace/stats/aggregation.go#L146-L165). Protocol wise [ClientGroupedStats.GRPC_status_code](https://github.com/DataDog/datadog-agent/blob/47938ea8c9b9894dcb03dc3f81cf2c6e408f1b6c/pkg/proto/datadog/trace/stats.proto#L103) is a `string`.
1 parent ae1a03c commit a3832a0

File tree

12 files changed

+31
-6
lines changed

12 files changed

+31
-6
lines changed

‎dd-java-agent/instrumentation/armeria/armeria-grpc-0.84/src/main/java/datadog/trace/instrumentation/armeria/grpc/client/GrpcClientDecorator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public AgentSpan onClose(final AgentSpan span, final Status status) {
116116

117117
span.setTag("status.code", status.getCode().name());
118118
span.setTag("grpc.status.code", status.getCode().name());
119+
span.setTag("rpc.grpc.status_code", status.getCode().value());
119120
span.setTag("status.description", status.getDescription());
120121

121122
// TODO why is there a mismatch between client / server for calling the onError method?

‎dd-java-agent/instrumentation/armeria/armeria-grpc-0.84/src/main/java/datadog/trace/instrumentation/armeria/grpc/server/GrpcServerDecorator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public <RespT, ReqT> AgentSpan onCall(final AgentSpan span, ServerCall<ReqT, Res
9797
public AgentSpan onStatus(final AgentSpan span, final Status status) {
9898
span.setTag("status.code", status.getCode().name());
9999
span.setTag("grpc.status.code", status.getCode().name());
100+
span.setTag("rpc.grpc.status_code", status.getCode().value());
100101
span.setTag("status.description", status.getDescription());
101102
return span.setError(
102103
SERVER_ERROR_STATUSES.get(status.getCode().value()), ErrorPriorities.HTTP_SERVER_DECORATOR);

‎dd-java-agent/instrumentation/armeria/armeria-grpc-0.84/src/test/groovy/ArmeriaGrpcStreamingTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ abstract class ArmeriaGrpcStreamingTest extends VersionedNamingTestBase {
178178
"$Tags.RPC_SERVICE" "example.Greeter"
179179
"status.code" "OK"
180180
"grpc.status.code" "OK"
181+
"rpc.grpc.status_code" 0
181182
"request.type" "example.Helloworld\$Response"
182183
"response.type" "example.Helloworld\$Response"
183184
peerServiceFrom(Tags.RPC_SERVICE)
@@ -215,6 +216,7 @@ abstract class ArmeriaGrpcStreamingTest extends VersionedNamingTestBase {
215216
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
216217
"status.code" "OK"
217218
"grpc.status.code" "OK"
219+
"rpc.grpc.status_code" 0
218220
defaultTags(true)
219221
}
220222
}

‎dd-java-agent/instrumentation/armeria/armeria-grpc-0.84/src/test/groovy/ArmeriaGrpcTest.groovy‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
182182
"$Tags.RPC_SERVICE" "example.Greeter"
183183
"status.code" "OK"
184184
"grpc.status.code" "OK"
185+
"rpc.grpc.status_code" 0
185186
"request.type" "example.Helloworld\$Request"
186187
"response.type" "example.Helloworld\$Response"
187188
if ({ isDataStreamsEnabled() }) {
@@ -221,6 +222,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
221222
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
222223
"status.code" "OK"
223224
"grpc.status.code" "OK"
225+
"rpc.grpc.status_code" 0
224226
if ({ isDataStreamsEnabled() }) {
225227
"$DDTags.PATHWAY_HASH" { String }
226228
}
@@ -319,6 +321,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
319321
"$Tags.RPC_SERVICE" "example.Greeter"
320322
"status.code" "${status.code.name()}"
321323
"grpc.status.code" "${status.code.name()}"
324+
"rpc.grpc.status_code" status.code.value()
322325
"status.description" description
323326
"request.type" "example.Helloworld\$Request"
324327
"response.type" "example.Helloworld\$Response"
@@ -343,6 +346,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
343346
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
344347
"status.code" "${status.code.name()}"
345348
"grpc.status.code" "${status.code.name()}"
349+
"rpc.grpc.status_code" status.code.value()
346350
"status.description" description
347351
"canceled" { true } // 1.0.0 handles cancellation incorrectly so accesting any value
348352
if (status.cause != null) {
@@ -432,6 +436,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
432436
"$Tags.RPC_SERVICE" "example.Greeter"
433437
"status.code" status.code.name()
434438
"grpc.status.code" status.code.name()
439+
"rpc.grpc.status_code" status.code.value()
435440
if (status.description != null) {
436441
"status.description" status.description
437442
}
@@ -459,6 +464,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
459464
errorTags error.class, error.message
460465
"status.code" "${status.code.name()}"
461466
"grpc.status.code" "${status.code.name()}"
467+
"rpc.grpc.status_code" status.code.value()
462468
"status.description" { it == null || String}
463469
"canceled" { true } // 1.0.0 handles cancellation incorrectly so accesting any value
464470
if ({ isDataStreamsEnabled() }) {
@@ -574,6 +580,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
574580
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
575581
"status.code" "OK"
576582
"grpc.status.code" "OK"
583+
"rpc.grpc.status_code" 0
577584
if ({ isDataStreamsEnabled() }) {
578585
"$DDTags.PATHWAY_HASH" { String }
579586
}
@@ -650,6 +657,7 @@ abstract class ArmeriaGrpcTest extends VersionedNamingTestBase {
650657
"$Tags.RPC_SERVICE" "example.Greeter"
651658
"status.code" "OK"
652659
"grpc.status.code" "OK"
660+
"rpc.grpc.status_code" 0
653661
"request.type" "example.Helloworld\$Request"
654662
"response.type" "example.Helloworld\$Response"
655663
if ({ isDataStreamsEnabled() }) {

‎dd-java-agent/instrumentation/google-pubsub-1.116/src/test/groovy/PubSubTest.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ abstract class PubSubTest extends VersionedNamingTestBase {
284284
"$Tags.RPC_SERVICE" { String }
285285
"status.code" { String }
286286
"grpc.status.code" { String }
287+
"rpc.grpc.status_code" { Integer }
287288
if ({ isDataStreamsEnabled() }) {
288289
"$DDTags.PATHWAY_HASH" { String }
289290
}

‎dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/client/GrpcClientDecorator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public <C> void injectContext(Context context, final C request, CarrierSetter<C>
116116
public AgentSpan onClose(final AgentSpan span, final Status status) {
117117
span.setTag("status.code", status.getCode().name());
118118
span.setTag("grpc.status.code", status.getCode().name());
119+
span.setTag("rpc.grpc.status_code", status.getCode().value());
119120
span.setTag("status.description", status.getDescription());
120121

121122
// TODO why is there a mismatch between client / server for calling the onError method?

‎dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcServerDecorator.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ public <RespT, ReqT> AgentSpan onCall(final AgentSpan span, ServerCall<ReqT, Res
9898
public AgentSpan onStatus(final AgentSpan span, final Status status) {
9999
span.setTag("status.code", status.getCode().name());
100100
span.setTag("grpc.status.code", status.getCode().name());
101+
span.setTag("rpc.grpc.status_code", status.getCode().value());
101102
span.setTag("status.description", status.getDescription());
102103
return span.setError(
103104
SERVER_ERROR_STATUSES.get(status.getCode().value()), ErrorPriorities.HTTP_SERVER_DECORATOR);

‎dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcStreamingTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ abstract class GrpcStreamingTest extends VersionedNamingTestBase {
162162
"$Tags.RPC_SERVICE" "example.Greeter"
163163
"status.code" "OK"
164164
"grpc.status.code" "OK"
165+
"rpc.grpc.status_code" 0
165166
"request.type" "example.Helloworld\$Response"
166167
"response.type" "example.Helloworld\$Response"
167168
peerServiceFrom(Tags.RPC_SERVICE)
@@ -198,6 +199,7 @@ abstract class GrpcStreamingTest extends VersionedNamingTestBase {
198199
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
199200
"status.code" "OK"
200201
"grpc.status.code" "OK"
202+
"rpc.grpc.status_code" 0
201203
defaultTags(true)
202204
}
203205
}

‎dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcTest.groovy‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
173173
"$Tags.PEER_PORT" server.port
174174
"status.code" "OK"
175175
"grpc.status.code" "OK"
176+
"rpc.grpc.status_code" 0
176177
"request.type" "example.Helloworld\$Request"
177178
"response.type" "example.Helloworld\$Response"
178179
if ({ isDataStreamsEnabled() }) {
@@ -212,6 +213,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
212213
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
213214
"status.code" "OK"
214215
"grpc.status.code" "OK"
216+
"rpc.grpc.status_code" 0
215217
if ({ isDataStreamsEnabled() }) {
216218
"$DDTags.PATHWAY_HASH" { String }
217219
}
@@ -317,6 +319,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
317319
"$Tags.RPC_SERVICE" "example.Greeter"
318320
"status.code" "${status.code.name()}"
319321
"grpc.status.code" "${status.code.name()}"
322+
"rpc.grpc.status_code" status.code.value()
320323
"status.description" description
321324
"request.type" "example.Helloworld\$Request"
322325
"response.type" "example.Helloworld\$Response"
@@ -341,6 +344,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
341344
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
342345
"status.code" "${status.code.name()}"
343346
"grpc.status.code" "${status.code.name()}"
347+
"rpc.grpc.status_code" status.code.value()
344348
"status.description" description
345349
if (status.cause != null) {
346350
errorTags status.cause.class, status.cause.message
@@ -424,6 +428,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
424428
"$Tags.PEER_PORT" server.port
425429
"status.code" "UNKNOWN"
426430
"grpc.status.code" "UNKNOWN"
431+
"rpc.grpc.status_code" 2
427432
"request.type" "example.Helloworld\$Request"
428433
"response.type" "example.Helloworld\$Response"
429434
"status.description" { it == null || String}
@@ -448,6 +453,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
448453
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
449454
"status.code" "${status.code.name()}"
450455
"grpc.status.code" "${status.code.name()}"
456+
"rpc.grpc.status_code" status.code.value()
451457
"status.description" { it == null || String}
452458
errorTags error.class, error.message
453459
if ({ isDataStreamsEnabled() }) {
@@ -554,6 +560,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
554560
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
555561
"status.code" "OK"
556562
"grpc.status.code" "OK"
563+
"rpc.grpc.status_code" 0
557564
if ({ isDataStreamsEnabled() }) {
558565
"$DDTags.PATHWAY_HASH" { String }
559566
}
@@ -621,6 +628,7 @@ abstract class GrpcTest extends VersionedNamingTestBase {
621628
"$Tags.RPC_SERVICE" "example.Greeter"
622629
"status.code" "OK"
623630
"grpc.status.code" "OK"
631+
"rpc.grpc.status_code" 0
624632
"request.type" "example.Helloworld\$Request"
625633
"response.type" "example.Helloworld\$Response"
626634
if ({ isDataStreamsEnabled() }) {

‎dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public final class ConflatingMetricsAggregator implements MetricsAggregator, Eve
8080
DDCaches.newFixedSizeCache(512),
8181
value -> UTF8BytesString.create(key + ":" + value));
8282
private static final CharSequence SYNTHETICS_ORIGIN = "synthetics";
83-
private static final String GRPC_STATUS_TAG = "grpc.status.code";
83+
private static final String GRPC_STATUS_TAG = "rpc.grpc.status_code";
8484

8585
private static final Set<String> ELIGIBLE_SPAN_KINDS_FOR_METRICS =
8686
unmodifiableSet(

0 commit comments

Comments
 (0)