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

Add IsMetricAlreadyCaptured to IMetricsProvider #745

Merged
merged 4 commits into from Mar 3, 2020
Merged

Add IsMetricAlreadyCaptured to IMetricsProvider #745

merged 4 commits into from Mar 3, 2020

Conversation

gregkalapos
Copy link
Contributor

@gregkalapos gregkalapos commented Mar 2, 2020

Solves #729

Prior to GC metrics, all metrics were captured "on the fly", meaning the agent periodically collected metrics (let's say every 30 seconds) and metrics providers were queried for data when the agent wanted to collect the data.

But this does not work in all cases. For collecting GC metrics we subscribe to EventSource (or ETW on Full Framework + Windows) and the GC triggers a method when a GC happens and that is our chance to collect data. GC is just 1 example, I expect similar situations later with other metrics.

It can happen that no GC is triggered for let's say 5 minutes - each time the agent tries to collect data before the 1. GC, it won't see any data.

The MetricsCollector wrongly disabled collecting metrics in such cases - so GC metrics were disabled forever for the given process.

Therefore a bool property IsMetricAlreadyCaptured is added to IMetricsProvider. Until this property does not return true, MetricsCollector won't try to get data from a metrics provider and won't disable it either - it just skips it. The new IsMetricAlreadyCaptured should be set to true when the IMetricsProvider tried to query data or was triggered by e.g. ETW or EventSource the 1. time.

@gregkalapos gregkalapos merged commit 21110d0 into elastic:master Mar 3, 2020
@gregkalapos gregkalapos deleted the MetricsExtendForIsCaptured branch March 3, 2020 19:55
@gregkalapos gregkalapos self-assigned this Mar 3, 2020
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.

None yet

2 participants