arshilkxwork opened a new pull request, #3348:
URL: https://github.com/apache/hugegraph/pull/3348
## Purpose of the PR
- part of #2354
#3344 fixed the invalid metric names that #2354 reported. While checking
whether #2354 could be closed, I found two more problems in the Prometheus text
output on master.
`GET /metrics/statistics` writes the label value without quotes:
```
graphspaces_graphspace_graphs_hugegraph_graph_vertices_GET{name=FAILED_REQUEST,}
0
```
Prometheus's scrape parser stops at the first such line with `expected label
value, got "F" ("INVALID")`, so this endpoint never yields any data.
`GET /metrics` writes each gauge, histogram, meter and timer family twice
once the `ServerReporter` has run its first report, 60s after startup.
`baseMetricPrometheusAll()` wrote the reporter's snapshot of `MetricManager`'s
registry and then called `MetricsUtil.writePrometheusFormat()` on the same
registry. Prometheus counts the repeats as duplicate samples, and
`expfmt`/promtool reject the page with `second TYPE line for metric name`.
## Main Changes
- `MetricsAPI.baseMetricPrometheusAll()` no longer writes the reporter
snapshot. It writes the version line, then calls `writePrometheusFormat()` on
the live registry. The endpoint already returned exactly this during the first
60 seconds, so no series are added or renamed.
- The statistics writer moved from `MetricsAPI.statisticsProm()` to
`MetricsUtil.writeStatisticsPrometheusFormat()` so it can be unit tested.
`LEFT_NAME_STR` and `RIGHT_NAME_STR` now include the quotes; `statisticsProm()`
was their only user.
- Unused imports in `MetricsAPI` are removed.
I left out the other half of #2354, emitting `summary` with `quantile`
labels instead of a `histogram` with `{name="p99"}` samples. That renames
series dashboards may depend on, so it should be agreed in the issue first.
Both Prometheus parsers accept the current shape.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
- `MetricsUtilTest.testWriteStatisticsPrometheusFormatQuotesLabelValues`
checks the exact statistics output for one path with two values. `mvn test -pl
hugegraph-server/hugegraph-test -am -P unit-test -Dtest=MetricsUtilTest` passes
(7 tests).
- `MetricsApiTest.testBaseMetricsPromAll` now fails if a `# TYPE` name
repeats, and `testStatisticsMetricsPromAll` checks that every labelled sample
has a quoted value. Both problems only show up after the reporter's first tick,
so on a server younger than 60s these assertions pass either way.
- Started a RocksDB server from the dist, sent a few REST requests,
waited 75s and scraped both endpoints, then ran the output through Prometheus's
`textparse` scrape parser and `expfmt` (`github.com/prometheus/prometheus`
v0.315.0). I did this twice on the same dist, once with master's
`MetricsAPI`/`MetricsUtil` classes swapped in:
| | master | this PR |
|---|---|---|
| `/metrics` before the first tick | 2811 samples, parses | 2811
samples, parses |
| `/metrics` after the first tick | 5643 samples, 2821 duplicate
series, expfmt: `second TYPE line` | 2811 samples, no duplicates, parses |
| `/metrics/statistics` | scrape parser: `expected label value` | 16
samples, parses |
## Does this PR potentially affect the following parts?
- [ ] Dependencies
- [ ] Modify configurations
- [ ] The public API
- [x] Other affects (typed here): `/metrics/statistics` label values are
now quoted, and `/metrics` no longer repeats metric families.
- [ ] Nope
## Documentation Status
- [ ] `Doc - TODO`: required documentation is pending; complete it before
merging.
- [ ] `Doc - Done`: documentation is included here or linked below.
- [x] `Doc - No Need`: no user-visible documentation is affected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
