open-telemetry / build-tools

Building tools provided by OpenTelemetry
https://opentelemetry.io
Apache License 2.0
37 stars 54 forks source link

Improvements for attributes semconv generation #133

Closed lmolkova closed 1 year ago

lmolkova commented 1 year ago

https://github.com/open-telemetry/opentelemetry-specification/pull/3183 declares attributes separately from traces and metrics and then reuses attributes in corresponding semconvs. There are two issues:

  1. Grandparent attributes are not populated in the table. E.g.

    • http.common group defines http.method
    • http.server group extends http.common and defines http.route
    • metric.http.server.duration extends http.server. When metric.http.server.duration(full) semconv table is rendered, it does not include http.method, but includes http.route
  2. Referenced attributes description should come from extended semconv first. E.g.

    • net.peer.name is defined in span-general semconv
    • it's redefined for HTTP in http.client.common providing long HTTP-specific description
    • it needs sampling_relevant: true for tracing (and none of it for metrics), so it has to be referenced again in trace.http.client and long HTTP-specific description should be repeated
    • Proposal: when resolving net.peer.name for trace.http.client semconv, it should prioritize properties from trace.http.client, then read them from parent (http.client.common) and finally fill in the gaps from original span-general spec.
trask commented 1 year ago

stable attribute ordering would be wonderful also

marcalff commented 1 year ago

This not only affect table generation, but also affects code generation.

Oberon00 commented 1 year ago

This seems like a bug more than a feature request. The extends implementation attempts to iterate repeatedly until a "fixpoint" is reached, i.e. it aims to support grandparent attributes, they should even be indistinguishable from parent attributes.

lmolkova commented 1 year ago

This is fixed now with #204. Here https://github.com/open-telemetry/semantic-conventions/pull/367 we have inherited properties (p2) and grandparent attributes populated (p1).