open-telemetry / opentelemetry-go-contrib

Collection of extensions for OpenTelemetry-Go.
https://opentelemetry.io/
Apache License 2.0
1.2k stars 563 forks source link

`otelgrpc` possibly reporting status code in incorrect format #5910

Closed achew22 closed 4 months ago

achew22 commented 4 months ago

Description

Right now, otelgrpc is reporting rpc.grpc.status_code as defined in semconv as an integer. It's entirely possible I'm holding it wrong though. Would love a pointer if that's the case.

https://github.com/open-telemetry/opentelemetry-go-contrib/blob/instrumentation/google.golang.org/grpc/otelgrpc/v0.53.0/instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go#L199-L201

As I read the spec on enumerations, they should be converted to the string type before being uploaded.

Values, which belong to a limited enumerated set (e.g. a Java enum), SHOULD be converted to AnyValue’s string_value field with the value of the string set to the symbolic name of the enumeration.

Environment

Steps To Reproduce

  1. Install the otelgrpc package in your gRPC server
  2. Run the service
  3. See integers in the metrics for the

Expected behavior

I would expect these to be strings describing the gRPC status codes. Could be wrong in my interpretation of the spec.

dmathieu commented 4 months ago

This isn't the proper spec to look at.

The mapping is needed when OpenTelemetry needs to convert a value produced outside OpenTelemetry into a value that can be exported using an OTLP exporter

We are not producing a value from outside OpenTelemetry into a value that can be used by OTLP. We are within OpenTelemetry, and are producing data for any exporter, not specifically OTLP. This spec would apply in a bridge that would allow transforming data from one format into OTLP.

What applies here is semantic conventions. https://github.com/open-telemetry/semantic-conventions/blob/f79fe6b61438bfa0804df9c254c402fd0d71f42a/docs/rpc/grpc.md#grpc-attributes

And the gRPC status code is indeed an integer (which makes sense, since that's the original data type).