Skip to content

Make atoms and string keys the same in aggregator#51

Open
MikaAK wants to merge 1 commit into
beam-telemetry:mainfrom
MikaAK:patch-2
Open

Make atoms and string keys the same in aggregator#51
MikaAK wants to merge 1 commit into
beam-telemetry:mainfrom
MikaAK:patch-2

Conversation

@MikaAK
Copy link
Copy Markdown
Contributor

@MikaAK MikaAK commented Aug 12, 2022

The reason i think this is important is because they show up the same in the exporter since atoms and string serialize the same.
The end result is you get two metrics with the same labels in your prometheus output which causes prometheus to fail to be able to scrape the endpoint due to bad data (duplicated entries)

It was pretty hard to track down but eventually I noticed the difference by checking the internal aggregation tables with
TelemetryMetricsPrometheus.Core.Aggregator.get_time_series and filtering for the metric i was looking for and seeing what it was being stored as

To resolve this I suggest this PR which makes atoms and strings fundamentally the same in the aggregator to it's internal ETS table

The reason i think this is important is because they show up the same in the exporter since atoms and string serialize the same. 
The end result is you get two metrics with the same labels in your prometheus output which causes prometheus to fail to be able to scrape the endpoint due to bad data (duplicated entries)

To resolve this I suggest this PR which makes atoms and strings fundamentally the same in the aggregator to it's internal ETS table
@MikaAK MikaAK requested a review from bryannaegele as a code owner August 12, 2022 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant