-
Notifications
You must be signed in to change notification settings - Fork 763
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
Implement the metrics dispose/shutdown logic #2404
Implement the metrics dispose/shutdown logic #2404
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2404 +/- ##
==========================================
+ Coverage 80.21% 80.26% +0.04%
==========================================
Files 232 232
Lines 7492 7500 +8
==========================================
+ Hits 6010 6020 +10
+ Misses 1482 1480 -2
|
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
var sw = Stopwatch.StartNew(); | ||
result = this.Collect(timeoutMilliseconds) && result; | ||
var timeout = timeoutMilliseconds - sw.ElapsedMilliseconds; | ||
result = this.exporter.Shutdown((int)Math.Max(timeout, 0)) && result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll need to modify PrometheusExporter to override shutdown, and shut its httpserver, right?
And obviously, for pull based exporters, any pending data is simply lost on shutdown right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
PrometheusExporter need to override onshutdown to cleanup httpserver. but can be done after this
counter.Add(1, new("name", "lemon"), new("color", "yellow")); | ||
counter.Add(2, new("name", "apple"), new("color", "green")); | ||
counter.Add(5, new("name", "apple"), new("color", "red")); | ||
counter.Add(4, new("name", "lemon"), new("color", "yellow")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this new example topic!
With #2389 merged, now we can fix the shutdown logic.
Changes