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

Remove metricType from runtime metric mapping definition #25

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

liustanley
Copy link
Contributor

@liustanley liustanley commented Mar 22, 2023

What does this PR do?

This is a change from #19 that fixes a breaking issue when sent runtime metrics have the incorrect metric type. #19 has been delayed so I wanted to merge this change first to prevent potential issues.

When a runtime metric gets sent as a Gauge, but it's defined as a Sum in the runtimeMetricsMappings variable, the metric will get sent to the mapSumRuntimeMetricWithAttributes function. This will cause a runtime error because the function tries to call md.Sum() when md is actually a Gauge. To avoid this, I decided to decouple the metric type from the runtimeMetricMapping and use md.Type() instead. The metric type can be further specified in the metadata on our end.

@liustanley liustanley requested a review from a team as a code owner March 22, 2023 13:58
@liustanley liustanley requested a review from gbbr March 22, 2023 13:58
@mx-psi mx-psi mentioned this pull request Mar 22, 2023
@liustanley liustanley merged commit d73565e into main Mar 22, 2023
@liustanley liustanley deleted the stanley.liu/remove-metric-type-from-mapping branch March 22, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants