chore(metrics): Switch from literals at emit! to span context

[MOZGIII]

A correction to #3888 after #3888 (comment)

I have a suspicion that events_processed will get mixed up in an unwanted way at the components that are implemented with nesting (i.e. a source that incorporates a transform, both increment events_processed). CC @lukesteensen

[binarylogic]

Very nice!

[lukesteensen]

I have a suspicion that events_processed will get mixed up in an unwanted way at the components that are implemented with nesting (i.e. a source that incorporates a transform, both increment events_processed). CC @lukesteensen

I was hoping that this would be avoided by only setting the span data at the topology level, not every time any of the component structs are instantiated, which it seems like we're doing. Does that sound right? We should definitely verify that the data is still accurate before merging.

[MOZGIII]

@lukesteensen yes, that's correct. We only set the span at the topology level, but the span remains entered for the whole runtime of the said component (note the .instrument(span); calls), and it affects everything that happens inside.

Consider a source, that includes a transform as an implementation detail. Like how docker source uses json_parse transform to parse the docker log format internally.

Before, processing one event would result in counter increments for:

• events_processed with labels component_kind=source (as literal), component_type=<source type> (as literal) - issued from source (i.e. docker)
• events_processed with labels component_kind=transform (as literal), component_type=<transform type> (as literal) - issued from the transform (i.e. json_parse) that's used inside of the said source

After:

• events_processed with labels component_kind=source (from span in topology), component_type=<source type> (from span in topology) - issued from source (i.e. docker)
• events_processed with labels component_kind=source (from span in topology), component_type=<source type as in topology> (from span in topology) - issued from the transform (i.e. json_parse) that's used inside of the said source, but still nested under the tracing span defined at the topology for the source

(component_name in the "after" is omitted for brevity, and because it doesn't help highlight the difference I'm trying to point out here - but it'll also be added in this new approach).

We bump the value twice either way - which is probably an unwanted outcome, either way. However, in this new way, the exact same metric (same name, same labels) is bumped, and two increments will be indistinguishable. Before, we could, at least, filter them out.

I think we need to something like wrap all the nested transforms with a span that would add an extra label, allowing us (and users) to differentiate between the nested and topmost transforms. This problem is kind of unique to transforms, because they can be re-used as an implementation detail of sinks and sources.

[binarylogic]

@MOZGIII can we merge this? #3211 is blocked by this and it'd be nice to get this merged. Thanks!

[MOZGIII]

Yes, let's merge this. It looks like if this causes issues due the points outlined above it is expected. We can deal with this later, once it lands and we see the effects.
It also seems to be a way to progress from here, since the discussion has stalled.

I'll resolve the conflicts and merge this

[binarylogic]

Sounds good, I opened #4454 to represent the issue.

[lukesteensen]

We bump the value twice either way - which is probably an unwanted outcome, either way. However, in this new way, the exact same metric (same name, same labels) is bumped, and two increments will be indistinguishable. Before, we could, at least, filter them out.

For the record, I suggested a fix for this as part of #4427 (emit these events from a topology-level wrapper rather in inside of each component).