-
Notifications
You must be signed in to change notification settings - Fork 764
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
[prometheus] Fix collection output buffer management when its resized #5676
Conversation
When buffer is too small to fit its initial 85kB size, its resized to fit the output, however the resized buffer is not used in final output. This ultimately results in overflow exception as cursor is correct and follows resized buffer. I also changed to use the same buffer variable for WriteTargetInfo, it also can resize the buffer.
@jcin193, could you please sign CLA? Without this your contribution cannot be accepted. |
Yeah, I know, unfortunately I wrote this at work, so I'll see what I can do |
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
still working on it |
src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Reiley Yang <reyang@microsoft.com>
can we get these tests running? |
…etrics and plain text formats (#5623) Co-authored-by: Piotr Kiełkowicz <pkiekowicz@splunk.com> Co-authored-by: Vishwesh Bankwar <vishweshbankwar@users.noreply.github.com> Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5676 +/- ##
==========================================
+ Coverage 83.38% 86.17% +2.79%
==========================================
Files 297 254 -43
Lines 12531 11054 -1477
==========================================
- Hits 10449 9526 -923
+ Misses 2082 1528 -554
Flags with carried forward coverage won't be shown. Click here to find out more.
|
test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs
Outdated
Show resolved
Hide resolved
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Vishwesh Bankwar <vishweshbankwar@users.noreply.github.com>
Thanks for this fix. 🎉 I just spent over an hour trying to find out why my metrics were broken and then finding the issue fixed by a package update, thanks to this PR. You saved me a day of frustration. |
Fixes #5661
Changes
When buffer is too small to fit its initial 85kB size its resized to fit the output, however the resized buffer is not used in final output. This ultimately results in overflow exception as cursor is correct and follows resized buffer. I also changed to use the same buffer variable for WriteTargetInfo, it also can resize the buffer.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes