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

Fixes a few problems in metrics #2240

Merged
merged 3 commits into from
Aug 7, 2020
Merged

Fixes a few problems in metrics #2240

merged 3 commits into from
Aug 7, 2020

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Aug 6, 2020

Resolves #2239

I found two additional problems in our metrics implementation as I was working on adding MP Metrics 2.3 support, leading to the following groups of changes:

  1. Internally, each metric implementation emits its own Prometheus/OpenMetrics output. The invoking code tells those metrics whether to include HELP and TYPE information. HelidonMeter and HelidonConcurrentGauge which have "sub-metrics" did not honor that flag for all their sub-metrics. These changes fix that.

  2. Added a getMetricsByName method to Registry which returns a List of MetricID/Metric pairs that match a given name.

  3. Changed the MetricsSupport#getOne method, to which /metrics/registryName/metricName requests were routed, to #getByName and changed its implementation to use the new method created in 2 so it fetches and reports on all metrics that match the name, not just one.

Re: # 3, here is how the spec indicates what the output should be:

/metrics//<metric_name> | GET | JSON, OpenMetrics | Returns the metric that matches the metric name for the respective scope
-- | -- | -- | --

It's in the middle of a table describing what the output should be for various paths and it does say "the metric" so it does not seem ambiguous about the case where multiple metrics share the same name. But the 2.3 optional TCK includes at least one test that depends on all metrics matching a name being returned. I am including that change here, rather than in the later PR with all the 2.3 support, to highlight this change. (The 2.2 TCK and all tests and examples passed locally with this change in place.)

@tjquinno tjquinno self-assigned this Aug 6, 2020
@tjquinno tjquinno merged commit cbb88d8 into helidon-io:master Aug 7, 2020
@tjquinno tjquinno deleted the json-format branch August 7, 2020 19:55
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.

/metrics output incorrect when multiple metrics share the same name
2 participants