Warn about metrics whose values don't match their gauge/counter type - #153
Open
eastagiletracker wants to merge 1 commit into
Open
Conversation
Mixing up counter and gauge types is a common problem in new integrations and nothing in the agent notices it, even though the agent sees the values those metrics produce. MetricTypeMismatchDetector watches values as they are read from the source (before counter values are turned into deltas) and warns when a metric defined as a gauge only ever grows, or when a metric defined as a counter keeps dropping. A metric is reported only after 30 of its values were collected, so an occasional counter reset is not reported, and every metric is reported at most once. Tracking keeps per-metric state, so it is done only when the agent runs with SPM_MONITOR_LOGGING_LEVEL=DEBUG, and the number of tracked metrics is capped. Nothing changes for agents running on other logging levels.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes making the agent warn about metrics whose collected values don't match the gauge/counter type they were defined with (Fixes #32). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/308. You can sign in with your GitHub ID to claim ownership of the project.
What changed and why
A metric defined as
gaugethat is really cumulative (or acounterthat is really a point-in-time value) is easy to write into a new integration YAML and very hard to notice afterwards, because the agent collects and ships it without complaint. The agent is, however, in the one position to notice: it sees every value the source produces.MetricTypeMismatchDetector(new, inspm-monitor) watches the values as they are read from the source — insideAttributeObservation.getMetricValue(), before counter values are turned into deltas, so the observation is made at the source and not in the agent's output — and logs a warning when a metric defined as agaugeonly ever grew, or when a metric defined as acounterkept dropping. A metric is reported only after at least 30 of its values were collected, and a counter is reported only when the drops make up at least 1/10 of its collected values, so a service restart resetting a real counter stays quiet. Every metric is reported at most once. Because the tracking keeps per-metric state and runs on the collection path, it is enabled only when the agent runs withSPM_MONITOR_LOGGING_LEVEL=DEBUG, and the number of tracked metrics is capped at 5000. On any other logging level the added work is a single boolean read per collected value and behavior is unchanged.docs/how-to.mddescribes how to turn it on and what the warning looks like.The hook sits on the shared value path of every data source type:
getMetricValue()is whatJsonAttributeObservation,DbAttributeObservation,MBeanAttributeObservationandMBeanFunctionInvokerall call, so JSON, DB and JMX metrics are covered by the one change.Reproduced on
masterat 4d68b2c (Java 8,mvn -B test): collecting a JSON metric declaredtype: gaugewhose source value only grows (10, 20, 30, …) 60 times throughJsonObservation.collectStats(), with the agent logging onDEBUG, produces an agent log of exactly 0 characters — the misconfiguration is completely silent today.Verification:
mvn -B testwas run on the clean tree before the change and again after it — same result, no new failures, andspm-monitorgoes from 89 to 103 tests. The new tests areMetricTypeMismatchDetectorTest(thresholds, counter resets, fluctuating gauges, non-numeric and NaN values, one-report-per-metric, the tracking cap) andMetricTypeMismatchCollectionTest, which drives realJsonObservation.collectStats()collections and asserts on the agent log. Removing only the two-line call ingetMetricValue()and re-running turnstestGrowingGaugeIsReportedWhileCollectingred (expected:<1> but was:<0>) while the "correctly declared counter", "fluctuating gauge" and "detector disabled" controls stay green, so the tests really do measure the new behavior.Nothing existing changed shape: no public signature was modified, no default was altered, no dependency was touched.
MetricTypeMismatchDetector.setEnabled(...)is deliberately public so the state can be forced in tests; its default comes from the agent's own logging level. The thresholds are constructor parameters if you would rather tune them than take the defaults.How this was managed
This work was tracked as Make agent warn about possibly misconfigured gauge/counter on the board at https://eastagiletracker.com/projects/308, which was imported from this repository's issues and pull requests (151 stories) and used to manage the change.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com