featzhang opened a new pull request, #27773:
URL: https://github.com/apache/flink/pull/27773
## Purpose
This PR fixes fundamental architectural issues in the Top N Metrics
Dashboard implementation that were identified during CI analysis. The previous
implementation had critical design flaws that prevented it from working
correctly.
## Changes
1. **Fixed REST Handler inheritance** - Now properly extends
`AbstractRestHandler` instead of using incorrect base class
2. **Fixed MessageHeaders implementation** - Now implements
`RuntimeMessageHeaders` with correct method signatures (getRequestClass,
getResponseClass, getResponseStatusCode, getHttpMethod)
3. **Fixed MetricStore access** - Using public APIs:
- `metricStore.getRepresentativeAttempts()` to get job tasks
- `taskMetricStore.getAllSubtaskMetricStores()` to get subtasks
- Instead of attempting to access private members (JobMetricStore,
TaskMetricStore.subtasks)
4. **Fixed HTTP method references** - Using `HttpMethodWrapper` instead of
non-existent `HttpMethod`
5. **Added proper logging** - Added `Logger` instance for better error
tracking
6. **Added handler registration** - Registered `TopNMetricsHandler` in
`WebMonitorEndpoint`
7. **Moved response body to correct package** - Moved from `legacy.messages`
to proper `job.metrics` package
## Implementation Details
The implementation now follows Flink's standard REST API architecture
pattern:
- Extends `AbstractRestHandler<RestfulGateway, EmptyRequestBody,
TopNMetricsResponseBody, TopNMetricsMessageParameters>`
- Implements proper request handling with `MetricFetcher` integration
- Uses public MetricStore APIs to safely access metrics data
- Returns Top N metrics for:
- CPU consumers (Top 5)
- Backpressured operators (Top 5)
- GC-intensive tasks (Top 5)
## Verifying this change
- [x] Code compiles successfully (excluding unrelated upstream compilation
issues)
- [x] Follows Flink REST API architecture patterns
- [x] Uses proper public APIs for MetricStore access
- [x] Code formatted with Spotless
- [ ] Integration tests (to be added)
## Documentation
This adds a new REST endpoint: `GET /jobs/:jobid/metrics/top-n` that returns
Top N metrics for a job.
## Notes
The previous PR #27771 was closed due to fundamental architectural issues.
This implementation addresses all identified issues and follows Flink's
standard patterns.
--
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]
