open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.72k stars 888 forks source link

semconv: add rpc.type and rpc.status #2419

Open 1046102779 opened 2 years ago

1046102779 commented 2 years ago

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md#attributes

How do we use semantic convention in rpc namespace to represent type(server and client)? How do we use semantic convention in rpc namespace to represent status(OK, Canceled, Unknown and etc.)?

image
arminru commented 2 years ago

Hi @1046102779! The OpenTelemetry Tracing API already defines dedicated properties for both a "Span Kind" and a "Status" on Spans. Take a look here to learn more: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#spankind https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status

1046102779 commented 2 years ago

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc.md#attributes

But how does metrics use tracing's SpanKind and status?

arminru commented 2 years ago

I misunderstood that this was about metrics, not tracing - sorry! In this case it could make sense to introduce generic attributes for both a ~span~ "party/counterpart" kind (likely with the same values as defined for SpanKind) and an operation status so they can also be used for HTTP, FaaS, messaging and others. What do you think? Would you like to drive spec PRs for these?

1046102779 commented 2 years ago

In this case it could make sense to introduce generic attributes for both a ~span~ "party/counterpart" kind (likely with the same values as defined for SpanKind) and an operation status so they can also be used for HTTP, FaaS, messaging and others. What do you think? Would you like to drive spec PRs for these?

Yes. I think it's a general concept and it's appropriate to add to the opentelemetry specification, and I'm willing to drive this PR.

arminru commented 2 years ago

@1046102779 I just looked over the semantic conventions you were referring to again. In your screenshot, only the common metric attributes are shown. The metrics (i.e., the metric names) themselves already distinguish between client- and server-side observations (e.g., rpc.server.duration vs rpc.client.duration), however. Because of that, there is no need for any attribute to distinguish clients from servers, right?

If additional RPC metrics were introduced, they would stick to this naming scheme with server/client in the namespace and this would also be the only way to have disjoint observations. Otherwise you would only be able to get meaningful results when aggregating metrics if you filter or split by the respective attribute.

The status, on the other hand, is indeed missing and I think it would make sense to introduce a generic metric attribute convention for this that matches the definitions for Span Status. This attribute should be re-usable across all technologies/frameworks and not just be defined for RPC alone.

cc @trask @jsuereth @jmacd @reyang @pirgeo for your opinions