Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[query] Update v1 Query Service To Accept v2 Storage API #6480

Closed
mahadzaryab1 opened this issue Jan 4, 2025 · 0 comments · Fixed by #6485
Closed

[query] Update v1 Query Service To Accept v2 Storage API #6480

mahadzaryab1 opened this issue Jan 4, 2025 · 0 comments · Fixed by #6485
Assignees
@mahadzaryab1
Labels
area/storage enhancement v2

Comments

@mahadzaryab1
Copy link
Collaborator

mahadzaryab1 commented Jan 4, 2025 •
edited
Loading

As per #6460 (comment),

  • keep v1 query service as is, operating on v1 storage
  • make the constructor of query service accept v2 storage only
  • the constructor should check if v2 storage is actually an adapter over v1, if so retrieve v1 reader and operate as usual
  • otherwise, apply another adapter that wraps native v2 storage into v1 reader API
@mahadzaryab1 mahadzaryab1 self-assigned this Jan 4, 2025
@mahadzaryab1 mahadzaryab1 mentioned this issue Jan 4, 2025
Merged
4 tasks
yurishkuro pushed a commit that referenced this issue Jan 4, 2025
@mahadzaryab1
[query] Update v1 query service to check for adapter at construction (#…
59b544a 
…6482)

## Which problem is this PR solving?
- Towards #6480

## Description of the changes
- This PR changes the v1 query service to check for the v1 adapter at
construction rather than at method invocation.
- Currently, the constructor will panic if its provided a native v2
storage implementation. This will be remedied in a follow-up PR that
will implement a reverse adapter to go from v2->v1.

## How was this change tested?
- CI and added a unit test

## 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 <mahadzaryab1@gmail.com>
@mahadzaryab1 mahadzaryab1 mentioned this issue Jan 5, 2025
Merged
4 tasks
adityachopra29 pushed a commit to adityachopra29/jaeger that referenced this issue Jan 9, 2025
@mahadzaryab1 @adityachopra29
[v2][storage] Implement reverse adapter to translate v2 storage api t…
9b4467a 
…o v1 (jaegertracing#6485)

## Which problem is this PR solving?
- Resolves jaegertracing#6480

## Description of the changes
- This PR implements a reverse adapter (`SpanReader`) that wraps a
native v2 storage interface (`tracestore.Reader`) and downgrades it to
implement the v1 storage interface (`spanstore.Reader`).
- The reverse adapter was integrated with the v1 query service. This
code path will only get executed once we start upgrading the existing
storage implementations to implement the new `tracestore.Reader`
interface as a part of
jaegertracing#6458

## How was this change tested?
- CI
- 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 <mahadzaryab1@gmail.com>
Signed-off-by: adityachopra29 <adityachopra2912@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mahadzaryab1 mahadzaryab1

Labels
area/storage enhancement v2
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[v2][storage] Implement reverse adapter to translate v2 storage api to v1 mahadzaryab1/jaeger
1 participant
@mahadzaryab1