Add controller execution time tracking to query responses #17709
Add controller execution time tracking to query responses #17709Akanksha-kedia wants to merge 1 commit intoapache:masterfrom
Conversation
…meseries responses - Add sendTimeSeriesRequestRaw method to handle timeseries responses without parsing as BrokerResponseNative - Update sendTimeSeriesRequestToBroker to use raw forwarding for both GET and POST endpoints - Fix GET endpoint method signature to use proper executeTimeSeriesQueryCatching parameters - Ensures controller preserves broker's PinotBrokerTimeSeriesResponse structure with status field - Resolves test failure: 'Status field should not be null in timeseries query response' The issue was that controller was incorrectly trying to parse timeseries responses as BrokerResponseNative and add execution time, but timeseries responses use PinotBrokerTimeSeriesResponse with different structure. This fix preserves the original Prometheus-compatible response format from the broker.
|
curl -X POST http://localhost:9001/sql -H "Content-Type: application/json" -d '{"sql":"SELECT COUNT(*) FROM baseballStats"}' | jq . % Total % Received % Xferd Average Speed Time Time Time Current (#17647 (comment)) |
|
@shauryachats @xiangfu0 @Jackie-Jiang please review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17709 +/- ##
==========================================
Coverage 63.20% 63.21%
- Complexity 1499 1502 +3
==========================================
Files 3173 3179 +6
Lines 190257 190731 +474
Branches 29072 29153 +81
==========================================
+ Hits 120261 120574 +313
- Misses 60662 60775 +113
- Partials 9334 9382 +48
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…meseries responses
The issue was that controller was incorrectly trying to parse timeseries responses as BrokerResponseNative and add execution time, but timeseries responses use PinotBrokerTimeSeriesResponse with different structure. This fix preserves the original Prometheus-compatible response format from the broker.
Instructions:
featurebugfixperformanceuibackward-incompatrelease-notes(**)(*) Other labels to consider:
testingdependenciesdockerkubernetesobservabilitysecuritycode-styleextension-pointrefactorcleanup(**) Use
release-noteslabel for scenarios like: