[exporter/signalfx] Support removal of direction attribute #12642
Conversation
Host metrics have split the direction attribute into multiple metrics. The signalfx exporter has translation functionality that assumed metrics were of the previous format, so this change supports the new format, and drops support of the old. This change also adds an "add_dimensions" translate ability, tests this new functionality, and modifies tests to ensure output metric format matches original.
| direction: | ||
| read: true |
There was a problem hiding this comment.
Why not just ?
| direction: | |
| read: true | |
| direction: read |
There was a problem hiding this comment.
I wasn't sure if they were equivalent. I'll test it and if it works I'll update accordingly.
| // Test uses 4 metrics, each with more than 1 data point. Aggregate by sum reduces each metric to 1 data point. | ||
| // 4 metrics, each with one data point, create 3 delta data points, either how much the current data point is | ||
| // larger than previous, or current data point's value (if smaller than previous). |
There was a problem hiding this comment.
I don't think this is an acceptable solution. We need to make sure that users see the same values in the charts built with disk_ops.total metric. I don't think it's going to be true if we break down disk_ops.total into several delta metrics. Please correct me if I'm wrong.
If it indeed introduces different results, I see the following options to restore the same disk_ops.total metric:
- Introduce a
metricstransformprocessor in the default Splunk Otel distribution config to get a metric combined fromsystem.disk.operations.readandsystem.disk.operations.write. - Refactor the signalfx exporter translation logic to work on a metrics batch instead of just one metric.
1 approach seems simpler and more reliable to me.
There was a problem hiding this comment.
Thanks for the pointers! I've brought this up as a potential concern in #12641 and had thought about possibly doing solution 2, but I was worried that it might have unintended side effects working on other metrics.
I'll look into option 1.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR is stale at this point, I'll open a new one or re-open depending on solution approach taken. |
Description:
Host metrics have split the direction attribute into multiple
metrics. The signalfx exporter has translation functionality
that assumed metrics were of the previous format, so this
change supports the new format, and drops support of the old.
This change also adds an "add_dimensions" translate ability, tests
this new functionality, and modifies tests to ensure output
metric format matches original.
Link to tracking Issue:
#12641
Testing:
Updated existing testing to match new metric format, added testing for
add_dimensionstranslation.Documentation:
Updated README for new translation functionality