[fix] Return empty array instead of nil from QueryService.getServices#7925
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7925 +/- ##
=======================================
Coverage 95.56% 95.56%
=======================================
Files 316 316
Lines 16715 16718 +3
=======================================
+ Hits 15973 15976 +3
Misses 579 579
Partials 163 163
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:
|
Metrics Comparison SummaryTotal changes across all snapshots: 0 Detailed changes per snapshotsummary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
summary_metrics_snapshot_cassandra📊 Metrics Diff SummaryTotal Changes: 0
|
dbcb34c to
1621263
Compare
0937e18 to
aae6e51
Compare
Some storage backends return nil when no services exist, which caused the getServices API to omit the services field or serialize it as null. This breaks the UI schema expectations, which always require an array. Normalize nil to an empty slice in QueryService.GetServices so that all callers receive a consistent, non-nil response. Fixes jaegertracing#7921. Signed-off-by: Sujal Shah <sujalshah28092004@gmail.com>
aae6e51 to
0532162
Compare
|
@yurishkuro could you kindly review :) |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the getServices endpoint returns null instead of an empty array when the backend has no traces, which caused validation errors in the UI.
Changes:
- Modified
GetServicesmethod inQueryServiceto ensure it always returns an empty slice instead of nil - Added a test case to verify that
GetServicesreturns an empty slice when the backend returns nil
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/jaeger/internal/extension/jaegerquery/querysvc/service.go | Added nil-safety check to ensure GetServices always returns an empty slice instead of nil |
| cmd/jaeger/internal/extension/jaegerquery/querysvc/service_test.go | Added test case to verify GetServices returns empty slice when backend returns nil |
Comments suppressed due to low confidence (1)
cmd/jaeger/internal/extension/jaegerquery/querysvc/service.go:134
- For consistency with GetServices, consider adding similar nil-safety handling to GetOperations. The v1adapter's GetOperations (at internal/storage/v2/v1adapter/tracereader.go:71) already checks for nil returns, suggesting that some storage backends may return nil for empty results. While not critical for this PR, adding the same defensive check here would prevent similar issues in the future and maintain consistency across the codebase.
func (qs QueryService) GetOperations(
ctx context.Context,
query tracestore.OperationQueryParams,
) ([]tracestore.Operation, error) {
return qs.traceReader.GetOperations(ctx, query)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…jaegertracing#7925) Fixes jaegertracing#7921. When the backend has no traces, the getServices endpoint returned a nil services field, which is serialized as null in JSON. This caused the UI to fail validation, since it expects an array. This change ensures the services field is always returned as an empty array. wrote a test `TestQueryServiceGetServicesReturnsEmptySlice` in `service_test.go`. - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Sujal Shah <sujalshah28092004@gmail.com>
…jaegertracing#7925) ## Which problem is this PR solving? Fixes jaegertracing#7921. ## Description of the changes When the backend has no traces, the getServices endpoint returned a nil services field, which is serialized as null in JSON. This caused the UI to fail validation, since it expects an array. This change ensures the services field is always returned as an empty array. ## How was this change tested? wrote a test `TestQueryServiceGetServicesReturnsEmptySlice` in `service_test.go`. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Sujal Shah <sujalshah28092004@gmail.com>
…jaegertracing#7925) ## Which problem is this PR solving? Fixes jaegertracing#7921. ## Description of the changes When the backend has no traces, the getServices endpoint returned a nil services field, which is serialized as null in JSON. This caused the UI to fail validation, since it expects an array. This change ensures the services field is always returned as an empty array. ## How was this change tested? wrote a test `TestQueryServiceGetServicesReturnsEmptySlice` in `service_test.go`. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Sujal Shah <sujalshah28092004@gmail.com>
…jaegertracing#7925) ## Which problem is this PR solving? Fixes jaegertracing#7921. ## Description of the changes When the backend has no traces, the getServices endpoint returned a nil services field, which is serialized as null in JSON. This caused the UI to fail validation, since it expects an array. This change ensures the services field is always returned as an empty array. ## How was this change tested? wrote a test `TestQueryServiceGetServicesReturnsEmptySlice` in `service_test.go`. ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Sujal Shah <sujalshah28092004@gmail.com>
Which problem is this PR solving?
Fixes #7921.
Description of the changes
When the backend has no traces, the getServices endpoint returned a nil services field, which is serialized as null in JSON.
This caused the UI to fail validation, since it expects an array.
This change ensures the services field is always returned as an empty array.
How was this change tested?
wrote a test
TestQueryServiceGetServicesReturnsEmptySliceinservice_test.go.Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test