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

stats: reproduce redundant TYPE tags problem in Prometheus. #27239

Merged
merged 2 commits into from
May 8, 2023

Conversation

jmarantz
Copy link
Contributor

@jmarantz jmarantz commented May 7, 2023

Commit Message: Reproduces a scenario where it's difficult to use a streaming optimization for Prometheus stats based on scopes without introducing a bug. Context:

Note that the existing unit tests did not exercise that scenario so this PR adds a testcase.

@rulex123 FYI
@zirain FYI

Additional Description:
Risk Level: low
Testing: just the new test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #27239 was opened by jmarantz.

see: more, trace.

@jmarantz jmarantz marked this pull request as ready for review May 8, 2023 00:01
Copy link
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

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

LGTM

@jmarantz
Copy link
Contributor Author

jmarantz commented May 8, 2023

Thanks; this is just a new unit test so I don't think it needs senior maintainer approval.

@jmarantz jmarantz merged commit ca88f87 into envoyproxy:main May 8, 2023
@jmarantz jmarantz deleted the prom-repro-extra-type-bug branch May 8, 2023 16:59
reskin89 pushed a commit to reskin89/envoy that referenced this pull request Jul 11, 2023
…xy#27239)

Commit Message: Reproduces a scenario where it's difficult to use a streaming optimization for Prometheus stats based on scopes without introducing a bug. Context:

Issue that /stats/prometheus endpoint takes too much much memory: Prometheus stats handler used too much memory. envoyproxy#16139
Solution for non-Prometheus endpoints to use less memory for /stats and run faster: admin: Streaming /stats implementation envoyproxy#19693 which I believe is working well.
This solution mapped to Prometheus: Prom stats perf improvements envoyproxy#24998
A case where this solution has a duplicate# TYPE line due to two stats with the same tag-extracted-stat name from two different scopes: stats: prometheus scrape failed envoyproxy#27173
Note that the existing unit tests did not exercise that scenario so this PR adds a testcase.

Signed-off-by: Joshua Marantz <[email protected]>
Signed-off-by: Ryan Eskin <[email protected]>
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.

3 participants