arshilkxwork opened a new pull request, #3344:
URL: https://github.com/apache/hugegraph/pull/3344
## Purpose of the PR
- close #3142
`GET /metrics` writes Cache gauges with names like
`org.apache.hugegraph.Cache.schema~DEFAULT-hugegraph.hits`.
`replaceDotDashInKey()` only replaced `.`, `-`, `/` and `$`, so the `~` went
out unchanged and Prometheus refused the entire scrape. `/metrics/statistics`
was worse off, since `replaceSlashInKey()` only replaced `/`.
## Main Changes
This follows the approach @imbajin described in the issue.
- `MetricsUtil.sanitizePrometheusMetricName()` replaces every character
outside `[a-zA-Z0-9_:]` with `_`, prefixes `_` when the result starts with a
digit, and returns `_` for an empty name. The regex is compiled once into a
static `Pattern`.
- `replaceDotDashInKey()` and `replaceSlashInKey()` are removed. Their call
sites now use the new helper: the four metric loops in
`MetricsUtil.writePrometheusFormat()`, the gauge/histogram/meter/timer loops in
`MetricsAPI.baseMetricPrometheusAll()`, and `MetricsAPI.statisticsProm()`.
- Names are sanitized only when Prometheus text is written. Registry and
Cache keys don't change.
Names that were already valid come out the same. A name with any other
character (`~`, or `.`/`-` in a REST path under `/metrics/statistics`) now gets
`_` in its place instead of producing an invalid name.
## 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:
- Added `MetricsUtilTest` and registered it in `UnitTestSuite`. It
checks that `.`, `-`, `/`, `$` and `~` become `_`, that valid names are
unchanged, that a leading digit gets a `_` prefix, and that an empty string
maps to `_`. It also renders a fresh `MetricRegistry` through
`writePrometheusFormat()`, checking that a Cache-like gauge with `~` uses the
same sanitized name in `# HELP`, `# TYPE` and the sample line, and that every
name emitted for a gauge, histogram, meter and timer matches the Prometheus
name pattern.
- `mvn test -pl hugegraph-server/hugegraph-test -am -P unit-test
-Dtest=MetricsUtilTest`
## Does this PR potentially affect the following parts?
- [ ] Dependencies
- [ ] Modify configurations
- [ ] The public API
- [x] Other affects (typed here): Prometheus metric names that previously
contained invalid characters are now emitted with `_` in their place.
- [ ] Nope
--
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]
