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

[orc8r] Copy labels explicitly to avoid rare overwriting in metric conversion to gauges #2700

Merged
merged 1 commit into from
Sep 15, 2020

Conversation

Scott8440
Copy link
Contributor

Signed-off-by: Scott8440 scott8440@gmail.com

Summary

I'll be honest I really don't know why this is necessary.

  • While testing getting new stats from AGWs, I noticed summaries from the AGW were only coming through with the {quantile="1"} metric (while there were 4 other quantiles)
  • It seemed like a pretty clear issue of pointers being overwritten in a loop in gaugeconverter.go, although I still don't really know how that was happening
  • What's even weirder is that this was only happening on metrics from the AGW, and there were quite a few summaries that originated in the cloud where this error never occurred (basically identical metrics too)
  • EVEN weirder is that I cannot replicate this in tests
  • I fixed the problem by just messing around until something worked and now it works so I give up trying to understand

Test Plan

  • Test by pushing a summary through the pipeline, all expected labels show up.
  • No other metrics were affected
    image

Signed-off-by: Scott8440 <scott8440@gmail.com>
@Scott8440 Scott8440 marked this pull request as ready for review September 9, 2020 23:52
Copy link
Contributor

@xjtian xjtian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with changes

@xjtian xjtian merged commit a531393 into magma:master Sep 15, 2020
ulaskozat pushed a commit to ulaskozat/magma that referenced this pull request Sep 16, 2020
…nversion to gauges (magma#2700)

Signed-off-by: Scott8440 <scott8440@gmail.com>
@Scott8440 Scott8440 deleted the fixGaugeconverter branch September 16, 2020 22:17
rdefosse pushed a commit to rdefosse/magma that referenced this pull request Oct 9, 2020
…nversion to gauges (magma#2700)

Signed-off-by: Scott8440 <scott8440@gmail.com>
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