-
Notifications
You must be signed in to change notification settings - Fork 2.6k
[v2][remote-storage] Implement remote storage extension #7043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v2][remote-storage] Implement remote storage extension #7043
Conversation
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (93.18%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7043 +/- ##
==========================================
- Coverage 96.06% 96.03% -0.03%
==========================================
Files 352 355 +3
Lines 20890 20973 +83
==========================================
+ Hits 20067 20141 +74
- Misses 620 626 +6
- Partials 203 206 +3
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:
|
} | ||
|
||
tm := tenancy.NewManager(&s.config.Tenancy) | ||
s.server, err = app.NewServer(&s.config.Options, tf, df, tm, telset) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we avoid depending on cmd/remote-storage ? What are you getting from there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yurishkuro The server implementation is already defined there so I was reusing it. We do the same thing in the jaeger_query
extension as well. Let me know if you want to take a different approach here though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
012568c
to
767e981
Compare
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
Signed-off-by: Mahad Zaryab <[email protected]>
…g#7043) ## Which problem is this PR solving? - Towards jaegertracing#7038 ## Description of the changes - Introduces a new remote_storage extension that wraps the existing remote-storage binary. This extension allows a remote storage gRPC server to be started within jaeger-v2, delegating to one of the configured backends. - Adds a new configuration file (config-remote-storage-backend.yaml) used by the integration tests. This replaces the need to manually start the test remote storage server. Integration tests are updated to use this config and remove related boilerplate. ## How was this change tested? - Updated the integration tests and added new unit tests ## 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: Mahad Zaryab <[email protected]>
…g#7043) ## Which problem is this PR solving? - Towards jaegertracing#7038 ## Description of the changes - Introduces a new remote_storage extension that wraps the existing remote-storage binary. This extension allows a remote storage gRPC server to be started within jaeger-v2, delegating to one of the configured backends. - Adds a new configuration file (config-remote-storage-backend.yaml) used by the integration tests. This replaces the need to manually start the test remote storage server. Integration tests are updated to use this config and remove related boilerplate. ## How was this change tested? - Updated the integration tests and added new unit tests ## 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: Mahad Zaryab <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:npm run lint
andnpm run test