ctrlaltdilj opened a new pull request, #29564:
URL: https://github.com/apache/flink/pull/29564
## What is the purpose of the change
The JobManager's `MetricStore` only ever adds metrics; entries are retained
until the job is removed. Metrics registered under a dynamically created group
therefore accumulate indefinitely in the REST API. The per-split watermark
metrics introduced by FLIP-513 are keyed by split id, so for sources with many
splits `/jobs/:jobid/vertices/:vertexid/subtasks/metrics` keeps returning
entries for every split the source has ever read, even though the TaskManager
side closes the split metric group once the split finishes (FLINK-37596,
FLINK-40549).
This PR introduces `metrics.internal.query-service.filter.excludes`, a
filter applied before a metric is handed to the `MetricQueryService`. It
mirrors the existing per-reporter `filter.excludes` syntax. Excluded metrics
are still delivered to metric reporters, which receive proper unregister
notifications and are therefore unaffected by the leak. The option defaults to
excluding the split watermark metrics (`*.operator.split.watermark:*:*`); an
empty value exposes them again.
This is a mitigation for the specific unbounded case rather than a fix of
the add-only `MetricStore`. Fixing the store to drop metrics that no longer
appear in a TaskManager dump is a larger change that can be tracked separately.
## Brief change log
- Add `MetricOptions.QUERY_SERVICE_EXCLUDES`
(`metrics.internal.query-service.filter.excludes`), a list option using the
reporter filter format, defaulting to the split watermark scope.
- Add `DefaultReporterFilters.queryServiceMetricsFromConfiguration`, which
builds a `ReporterFilter<Metric>` with an implicit include-all and the
configured excludes.
- `MetricRegistryConfiguration` parses the filter once at startup, logs
the resolved exclude list at INFO, and falls back to the default with a WARN if
the value cannot be parsed.
- `MetricRegistryImpl.register` evaluates the filter against the metric's
logical scope before calling `MetricQueryService.addMetric`. Reporter
notification and unregistration are unchanged.
- Document the option (regenerated `metric_configuration.html`) and note
on the Split metrics table in `docs/content/docs/ops/metrics.md` that these
metrics reach reporters only by default.
## Verifying this change
This change added tests and can be verified as follows:
-
`MetricRegistryImplTest#testQueryServiceExcludesSplitWatermarkMetricsByDefault`
registers a gauge under a real `taskmanager.job.task.operator.split.watermark`
group hierarchy and asserts that a reporter receives it while the query service
dump does not.
- `MetricRegistryImplTest#testQueryServiceExcludesAreConfigurable` asserts
that a configured pattern excludes a matching metric from the query service
dump while others remain.
-
`DefaultReporterFiltersTest#testQueryServiceFilterExcludesSplitWatermarkMetricsByDefault`
and `#testQueryServiceFilterFromConfiguration` cover the default filter,
multiple entries, type-scoped entries, and that a configured list replaces the
default rather than extending it.
-
`DefaultReporterFiltersTest#testQueryServiceFilterWithoutExcludesExposesSplitWatermarkMetrics`
covers both the empty list and the empty flat string form.
- Both registry tests were verified to fail with the filter check removed
and to pass with it.
- `ConfigOptionsDocsCompletenessITCase` passes against the regenerated
documentation.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: yes, a new option is added to `MetricOptions`
(`@PublicEvolving`). No existing option or signature changes.
- The serializers: no
- The runtime per-record code paths (performance sensitive): no. The
filter runs once per metric registration, in the same place the reporter
filters already run.
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? docs (generated config reference
and the Split section of the metrics page) and JavaDocs
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Claude Fable 5.1)
--
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]
