gRPC: initial stable semconv support and testing

[trask]

No description provided.

[zeitlinger]

Thanks for the PR!

One issue I noticed: the stable gRPC semconv spec specifies that rpc.response.status_code should use the named status code (OK, CANCELLED, UNAVAILABLE, etc.), not the numeric value (0, 2, 14).

Suggest changing:

attributes.put(RPC_RESPONSE_STATUS_CODE, String.valueOf(status.getCode().value()));

to:

attributes.put(RPC_RESPONSE_STATUS_CODE, status.getCode().name());

(and updating the test assertions accordingly)

[zeitlinger]

In rpc/dup mode, span attributes can only hold one value for rpc.method. Since stable semconv wins (line 42-53 of RpcCommonAttributesExtractor), old metrics would silently get the stable method value (example.Greeter/SayHello) instead of the old one (SayHello).

The dualEmitContextCustomizer from #16298 was designed to fix this — it stashes the old rpc.method in context so RpcClientMetrics/RpcServerMetrics can swap it back for old metrics. But it's not wired into GrpcTelemetryBuilder.

Suggest adding to GrpcTelemetryBuilder.build(), after addOperationMetrics for each instrumenter:

clientInstrumenterBuilder
    .addOperationMetrics(RpcClientMetrics.get())
    .addContextCustomizer(
        RpcMetricsContextCustomizers.dualEmitContextCustomizer(rpcAttributesGetter));

serverInstrumenterBuilder
    .addOperationMetrics(RpcServerMetrics.get())
    .addContextCustomizer(
        RpcMetricsContextCustomizers.dualEmitContextCustomizer(rpcAttributesGetter));

(The customizer is a no-op when not in dup mode, so it's safe to always register.)

[zeitlinger]

Follow-up on the dualEmitContextCustomizer comment: the reason this wasn't caught is that there's no rpc/dup integration test for gRPC. The testBothSemconv task in instrumentation-api-incubator exercises RpcClientMetrics/RpcServerMetrics in isolation (manually calling the context customizer), but nothing tests the full gRPC pipeline in dup mode.

Adding a testBothSemconv task to instrumentation/grpc-1.6/javaagent/build.gradle.kts with -Dotel.semconv-stability.opt-in=rpc/dup would catch this: the test assertions would see old metrics with rpc.method=example.Greeter/SayHello instead of rpc.method=SayHello and fail, revealing the missing context customizer wiring.

[zeitlinger]

Two more things I noticed by comparing with #15879:

1. Captured request metadata uses old attribute key prefix in stable mode

GrpcAttributesExtractor.onEnd() always stores captured request metadata under rpc.grpc.request.metadata.<key>, even when stable semconv is active. The stable semconv spec defines the attribute as rpc.request.metadata.<key> (no grpc. infix). The big PR handles this via CapturedGrpcMetadataUtil.stableRequestAttributeKey() and branching on semconv mode — that's missing here.

2. Missing testStableSemconv task for grpc-1.6/library

The testStableSemconv Gradle task is only added to grpc-1.6/javaagent/build.gradle.kts, but the big PR also adds one to grpc-1.6/library/build.gradle.kts. The library tests exercise GrpcTelemetryBuilder.build() directly, which is the code path where things like the dualEmitContextCustomizer wiring would be caught.

[zeitlinger]

This PR is "PR 2" (rpc-grpc) from the split plan in #15879. PR 1 (rpc-api-incubator) was merged as #16298.

[zeitlinger]

Done reviewing. Summary of findings:

1. rpc.response.status_code should use named status codes (status.getCode().name()) not numeric (String.valueOf(status.getCode().value())) per the stable gRPC semconv spec
2. dualEmitContextCustomizer from #16298 is not wired into GrpcTelemetryBuilder, so rpc/dup mode would produce wrong rpc.method values on old metrics
3. A testBothSemconv Gradle task with rpc/dup would have caught the above
4. Captured request metadata always uses old rpc.grpc.request.metadata.* prefix — stable semconv requires rpc.request.metadata.* (missing CapturedGrpcMetadataUtil.stableRequestAttributeKey())
5. Missing testStableSemconv task for grpc-1.6/library module (only javaagent has one)

Compared against the original big PR #15879 to identify gaps. Items 1 and 2 are also present in the big PR, so they're pre-existing rather than regressions from the split.