Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/signalfx] Remove direction references from metric translations #12641

Closed
crobert-1 opened this issue Jul 21, 2022 · 5 comments
Closed
Assignees
Labels

Comments

@crobert-1
Copy link
Member

This is a subtask of issue #11815

The signalfxexporter has functionality that "translates" input metrics into output metrics, and the default translation functionality references the direction attributes. These references should be updated for the new metric format.

@crobert-1
Copy link
Member Author

One thing that should be noted: Metrics can't be identically matched to previous form.

Originally, one metric (system.disk.io for example) might have many datapoints associated with it, both of read and write directions. After doing a sum aggregation without the direction and device attribute/dimension, all datapoints would be merged into a single datapoint, with the metric name system.disk.io.total.

This is no longer possible as read and write metrics will be in individual metrics. This means that there may be two metrics labeled system.disk.io.total, where one has a single datapoint that is the sum of the read and the other has a single datapoint that is the sum of the write. Neither would have any label/dimension to show which is which, they'd look identical.

If this is not an acceptable solution I'll have to rework the change presented in #12642.

@github-actions
Copy link
Contributor

Pinging code owners: @pmcollins @dmitryax

@crobert-1
Copy link
Member Author

crobert-1 commented Jul 29, 2022

One thing that should be noted: Metrics can't be identically matched to previous form.

Originally, one metric (system.disk.io for example) might have many datapoints associated with it, both of read and write directions. After doing a sum aggregation without the direction and device attribute/dimension, all datapoints would be merged into a single datapoint, with the metric name system.disk.io.total.

This is no longer possible as read and write metrics will be in individual metrics. This means that there may be two metrics labeled system.disk.io.total, where one has a single datapoint that is the sum of the read and the other has a single datapoint that is the sum of the write. Neither would have any label/dimension to show which is which, they'd look identical.

If this is not an acceptable solution I'll have to rework the change presented in #12642.

Dmitrii and I discussed this problem and decided the best way to solve this is to modify the translation logic to operate on multiple metrics at once.

Currently the translateAndFilter method within the exporter/signalfxexporter/internal/translation/converter.go is called for each metric, so operations can only be done on datapoints within the same metric. Changing the logic to allow operations on metrics within the same ScopeMetric will allow aggregations and operations to be done across metrics, allowing metrics to be combined in a way they were before the direction attribute change. This would make it possible for system.disk.io.total to contain both direction dimensions within the same metric as before.

Once this logic has been updated, the translation rules can be updated to properly map new metrics to the previous format.

This change should make it so that the signalfx exporter also excludes the new format of metrics until the signalfx backend properly supports them.

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Nov 10, 2022
@dmitryax
Copy link
Member

Not needed anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants