feat(query): Expose metricsStorage to UI as storage capabilities#8154
Conversation
Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
461f947 to
8d0d371
Compare
There was a problem hiding this comment.
Pull request overview
Injects metricsStorage into the JAEGER_STORAGE_CAPABILITIES object served with the Jaeger UI so the frontend can reliably detect whether metrics querying is configured (per #3540 step 1).
Changes:
- Adds
MetricsStorageto the query-serviceStorageCapabilitiesstruct and JSON output. - Sets
caps.MetricsStorageduring static UI handler registration in the query server router. - Updates static handler tests to assert the new JSON serialization shape including
metricsStorage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
cmd/jaeger/internal/extension/jaegerquery/querysvc/service.go |
Extends the server-to-UI capability model with metricsStorage. |
cmd/jaeger/internal/extension/jaegerquery/internal/server.go |
Computes and injects the metricsStorage capability into the UI static handler pipeline. |
cmd/jaeger/internal/extension/jaegerquery/internal/static_handler_test.go |
Updates assertions to match the new injected capabilities JSON. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Do not say "resolves" if the PR is only part of an issue. Saying "Resolves" causes GH to close the issue once the PR is merged. Say |
Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
@yurishkuro sorry sir , I will take care of it in future . now I am proceeding with the github sugestions . thank you |
Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@yurishkuro Copilot AI has left some review suggestions regarding testing the new capability extraction logic and fixing some test cleanups. Since it's an automated review, I wanted to double-check with you , would you like me to implement these suggestions as well, or is the current state good to go |
17751d7 to
79f3daa
Compare
|
copilot is correct 95% of the time in my experience. If you think it's not then leave a comment explaining why. |
79f3daa to
ce0f291
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ce0f291 to
b376dc3
Compare
… caller Signed-off-by: hharshhsaini <sainiharsh3311@gmail.com>
b376dc3 to
c7b77ce
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@yurishkuro I have resolved all the comments and removed the redundant capability tests to keep the diff completely clean. The tests pass perfectly now. I believe the PR is ready for another look whenever you have time. Thanku for the guidance. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8154 +/- ##
==========================================
- Coverage 95.67% 95.67% -0.01%
==========================================
Files 317 317
Lines 16747 16746 -1
==========================================
- Hits 16023 16022 -1
Misses 571 571
Partials 153 153
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:
|
Which problem is this PR solving?
isATMActivated) in favor of trusting the server'sJAEGER_STORAGE_CAPABILITIESexplicitly. Currently, the UI receives{"archiveStorage": false, "metricsStorage": false}by default from index.html. This PR computes and injectsmetricsStorage: true/falsedirectly from the query-service into the UI static handler so the frontend understands when metrics storage is actively configured.Description of the changes
MetricsStorage boolfield to the StorageCapabilities struct incmd/jaeger/internal/extension/jaegerquery/querysvc/service.go.cmd/jaeger/internal/extension/jaegerquery/internal/server.goto explicitly setcaps.MetricsStorage = trueifmetricsQuerySvcis not nil during the static UI handler registration.cmd/jaeger/internal/extension/jaegerquery/internal/static_handler_test.goto assert the newly updated JSON serialization format:{"archiveStorage":false,"metricsStorage":false}.How was this change tested?
make testinside the/cmd/jaeger/internal/extension/jaegerquery/module.TestRegisterStaticHandlertests were updated and pass, verifying the frontend correctly receives the augmented capabilities struct.Checklist
make lint testAI Usage in this PR (choose one)
See AI Usage Policy.