[metricstransform] Add resource attribute copy to metric label

[seanhoughton]

Description:

Add the ability to copy resource attributes into metric labels. This allows users to use resource attributes like "hostname" or "pod name" as labels in metrics without having to re-instrument the application.

This is a WIP as I can't get the tests to pass yet and I would like some feedback before spending any more time on it.

Link to tracking Issue:

#2568

Testing:

I added a test case for the new operation

Documentation:

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Sean Houghton (618f235670d147d72425dec8ec7724e28a6919d2)

[seanhoughton]

I can't figure out how to reference the v1.Resource instance from inside a transform op processor but it's not available inside the processor. I've also tried putting an instance on the internaldata.MetricsData instance but it seems to be stripped away inside ConsumeMetrics and is nil by the time it reaches the processor.

[codecov]

Codecov Report

Merging #2630 (b0d74d7) into main (047b1d9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2630   +/-   ##
=======================================
  Coverage   91.47%   91.47%           
=======================================
  Files         439      440    +1     
  Lines       21855    21870   +15     
=======================================
+ Hits        19991    20005   +14     
- Misses       1394     1395    +1     
  Partials      470      470           

Flag	Coverage Δ
integration	69.18% <ø> (-0.07%)	⬇️
unit	90.39% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
processor/metricstransformprocessor/config.go	100.00% <ø> (ø)
processor/metricstransformprocessor/factory.go	98.88% <100.00%> (+0.02%)	⬆️
...stransformprocessor/metrics_transform_processor.go	99.30% <100.00%> (+<0.01%)	⬆️
...formprocessor/operation_copy_resource_attribute.go	100.00% <100.00%> (ø)
receiver/carbonreceiver/transport/tcp_server.go	66.00% <0.00%> (-1.00%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 047b1d9...b0d74d7. Read the comment docs.

[seanhoughton]

This PR works in my tests, is documented, and has tests.

[mxiamxia]

This should be done by enabling resource_to_telemetry_conversion config in each metric exporter, which has been designed for this specific purpose.

exporterhelper.ResourceToTelemetrySettings `mapstructure:"resource_to_telemetry_conversion"`

https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/exporterhelper/resource_to_label.go#L23

[seanhoughton]

The resource_to_telemetry_conversion copies every resource attribute to metric labels which is not desirable if you want to manage the number of unique metric series produced. Would it be better to extend that option to include an optional list of labels to be copied instead of using the transform approach?

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[seanhoughton]

The resource_to_telemetry_conversion is not supported by many exporters (including the prometheus exporter). I'll close this PR in favor of using the resource_to_telemetry_conversion config - but it will require some work to implement in multiple exporters.

[pmalek-sumo]

resource_to_telemetry_conversion

What should one do if further data processing is required? This being at the exporter stage doesn't allow that.

Perhaps we could revisit this and think of adding similar feature at the processor stage? WDYT @mxiamxia ?

[awiddersheim]

I still see value in this functionality. I can certainly see how adding this functionality can be confusing when resource_to_telemetry_conversion exists but think there is room for both.

Let me see if I can describe my use case. I'm scraping metrics from services using the prometheus receiver and enriching those metrics with the resourcedetectionprocessor. With the resource_to_telemetry_conversion option enabled this tends to greatly increase cardinality.

One option I considered was to instead use the target informational metric the prometheusremotewrite exporter emits. Each attribute seems to get added to target as a label. You could then turn off the resource_to_telemetry_conversion configuration option and each metric scraped would only have the labels added by the services without the enrichment.

It seems a common pattern to do joins against an information metric to help expand context when necessary while also reducing cardinality/active series. Typically you would do this with some unique node ID.

The problem is I would now need to build logic into each service to have a node ID label as a default for every metric emitted to do that join. Essentially, in each service I would need to do what the otel-collector is doing when it is detecting resources so the target metric's node label has the same value as all of my metrics. Instead, it would be nice to just have the collector add that label for me with the metricstransformprocessor.

Perhaps there are better approaches for what I'm looking to achieve, so open to other options.

[singku]

I'd love to see this feature as well. resource_to_telemetry_conversion doesn't meet our needs if we do not need that extra metrics label.

[TylerHelmuth]

@awiddersheim and @singku the transformprocessor may be able to meet your needs. It is able to set the attribute of data points using a resource attribute. It looks like set(attributes["the name the new attribute"], resource.attributes["the name of the resource attribute you want to copy"]). It also has the ability to delete attributes using the delete_key function.

[singku]

Thank you Tyler, I will give it a try