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

GMP exporter should convert Int values to Double #798

Closed
dashpole opened this issue Jan 24, 2024 · 6 comments · Fixed by #844
Closed

GMP exporter should convert Int values to Double #798

dashpole opened this issue Jan 24, 2024 · 6 comments · Fixed by #844
Assignees
Labels
bug Something isn't working priority: p2

Comments

@dashpole
Copy link
Contributor

dashpole commented Jan 24, 2024

GMP only accepts Double values. Sending it an int will cause the metric to be rejected.
This is not correct. GMP accepts int and double values.

This can be done with the transform processor today, although I think this needs to be filtered so it only applies to int valued datapoints:

      transform/inttodouble:
        metric_statements:
          - context: datapoint
            statements:
              # GMP metrics are expected to be double
              - set(value_double, Double(value_int))

Ideally, the exporter should handle this, rather than requiring users to add processors. I encountered this when using the spanmetrics exporter, which produces int-valued metrics.

@dashpole dashpole added bug Something isn't working priority: p2 labels Jan 24, 2024
@dashpole
Copy link
Contributor Author

dashpole commented Apr 3, 2024

This is tricky. If we do this in the exporter, existing users will encounter errors when they upgrade, since their existing metric descriptors are the wrong type. We would likely need to make this opt-in behavior.

@aabmass
Copy link
Contributor

aabmass commented Apr 3, 2024

If GMP only supports doubles, how could their descriptors have the wrong type on upgrade?

@dashpole
Copy link
Contributor Author

dashpole commented Apr 3, 2024

I think I must have been mistaken. There was a user that was able to write int metrics, and had problems switching to double.

@dashpole
Copy link
Contributor Author

Confirmed that GMP accepts both int and double metrics

@dashpole
Copy link
Contributor Author

In that case, we should probably not change the value type. If users have the same metric in both INT and Double, they can use the above workaround to merge them together.

@dashpole dashpole reopened this Apr 24, 2024
@dashpole
Copy link
Contributor Author

Actually, we should document this behavior on the exporter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants