feat(tracing): inject component ID as instrumentation scope attribute

[hainenber]

PR Description

Which issue(s) this PR fixes

Fixes #5285

Notes to the Reviewer

PR Checklist

• CHANGELOG.md updated

[clayton-cornell]

No docs changes included, so I'll step away from this one :-)

[rfratto]

I'm testing this change locally right now, and I'm trying to figure out where the attributes show up in Tempo.

[rfratto]

I'm not able to see the instrumentation scope in Tempo; trying to figure out why.

[rfratto]

I chatted with the Tempo team and apparently Tempo doesn't support instrumentation scope attributes yet, so setting this would be a no-op for Tempo. For now, we have to decide if this attribute belongs more on the span or on the resource (which Tempo would support today).

WDYT @ptodev @hainenber?

[ptodev]

We have to decide if this attribute belongs more on the span or on the resource (which Tempo would support today).

I suppose a span attribute would be more appropriate. As defined in the spec, a "resource" is supposed to combine multiple instrumentation scopes. Hence it doesn't seem appropriate to assume that only one instrumentation scope is part of the resource.

I think we should proceed like this:

• Add span attributes with the full componentID/controllerID as we do now.
  • Those attributes will include the component instance name. E.g. they could look like otelcol.receiver.otlp.default.
  • We could keep those attributes in the long term, because specifying the component instance makes more sense as a span attribute than as an instrumentation scope.
• Add instrumentation scope which does not include the component instance name
  • Those attributes will not include the component instance name. E.g. they could look like otelcol.receiver.otlp. I think this is more in line with Otel's expectations.
  • Even though Tempo doesn't support instrumentation scope, I see no harm in adding it. I guess Tempo will just ignore it? And Tempo will presumably support it soon anyway?

In the long run, after Tempo supports instrumentation scope, we could even optimise by deprecating the grafana_agent.component_id span attribute and instead having just a span attribute with the component instance name (e.g. "default" instead of "otelcol.receiver.otlp.default"). Maybe this would speed up some queries?

BTW I wonder whether we should use the source code attribute conventions as well. I cannot think of a clean way to use it. We could have "code.namespace" = "grafana_agent.component.otelcol.exporter.loadbalancing", but I think it's easier just to have a "grafana_agent.component_id" = "otelcol.receiver.otlp.default". That way we also specify that "default" is the component instance name.

[hainenber]

hi there, long week for me, hence the long time for reply :)

First of all, thanks @rfratto for going to extended length to reach out Tempo team to test it out. That was awesome!

Basically, I agree with @ptodev's proposal here. We can keep the span attribute as is and try to add instrumentation scope without the component's instance name (I suppose that means we need to omit the last dot-separated part, e.g. "a.b.c" -> "a.b")

Although Tempo might not support instrumentation scope attribute, other OTEL-compatible tracing backend might already support it and so it could be more than just no-op ;)

Let me implement and test the proposal. Will keep you posted

[hainenber]

Hi folks, I've made a draft implementation as suggested above. PTAL when you have time. Thanks!

[rfratto]

Does the exact ID of the component still get set on the span?

[hainenber]

Yes, the old function isn't removed, yet ;)

[rfratto]

Cool, just double checking :)

[rfratto]

Thanks for your work!