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.

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

[jaeger-v2] Change remote storage API to be the same as query service API #5334

Open
james-ryans opened this issue Apr 6, 2024 · 2 comments

Comments

@james-ryans
Copy link
Contributor

Requirement

As Jaeger-V2 is currently being developed, it would be nicer if the remote storage API had the same API with query service so it could be reused between components.

Problem

As discussed in #5322 (comment), by changing the remote storage API to be the same as the query storage API, we can reuse this API for Jaeger-V2 e2e testing purposes.

Proposal

No response

Open questions

  • Remote storage APIs have read & write span, read & write archive span, get capabilities, and read dependencies while query storage APIs only have read span data. So, the scope of this feature is to change the read span, archive span, and dependencies. Is this correct?

  • I have looked into query service API and there are v2 and v3, in which v3 uses OTLP model while v2 uses Jaeger model. V3 handles most v2's API except ArchiveTrace and GetDependencies. Do we need to use partially v3 APIs and v2 for the rest or fully v2 APIs?

@yurishkuro
Copy link
Member

I think we need to do a bit of analysis before we decide, including the v2/v3 question. The plan for Storage V2 API was to base it on OTLP data model, not the old Jaeger model as query/api_v2 today. Maybe we should start a doc where we can do side by side comparison of both existing v2 and v3 (in terms of capabilities) and the existing storage API, so see if we can arrive to a single unified API. There's also another consideration for the write API to be compatible with OTLP receivers. If we can satisfy all these requirements it would be awesome.

@james-ryans
Copy link
Contributor Author

I have compared and analyzed the existing storage API and query service v2 and v3 API here https://docs.google.com/document/d/1IvUcDsdRxMPK-xTUE32w3NSAGTkUcmnDQttN6ijUnWs/edit?usp=sharing.
Please have a look 🙇

yurishkuro pushed a commit that referenced this issue May 1, 2024
## Which problem is this PR solving?
- Part of #5334

## Description of the changes
- An API design of Storage V2 spanstore and its proto file.
- For the detailed discussion and how we arrived to this design, please
take a look at
https://docs.google.com/document/d/1IvUcDsdRxMPK-xTUE32w3NSAGTkUcmnDQttN6ijUnWs/edit?usp=sharing

## How was this change tested?
- This PR hasn't been tested yet since it only contains interfaces and
no actual implementation to be tested.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: James Ryans <james.ryans2012@gmail.com>
varshith257 pushed a commit to varshith257/jaeger that referenced this issue May 3, 2024
…gertracing#5399)

## Which problem is this PR solving?
- Part of jaegertracing#5334

## Description of the changes
- An API design of Storage V2 spanstore and its proto file.
- For the detailed discussion and how we arrived to this design, please
take a look at
https://docs.google.com/document/d/1IvUcDsdRxMPK-xTUE32w3NSAGTkUcmnDQttN6ijUnWs/edit?usp=sharing

## How was this change tested?
- This PR hasn't been tested yet since it only contains interfaces and
no actual implementation to be tested.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: James Ryans <james.ryans2012@gmail.com>
Signed-off-by: Vamshi Maskuri <gwcchintu@gmail.com>
Pushkarm029 pushed a commit to Pushkarm029/jaeger that referenced this issue May 4, 2024
…gertracing#5399)

## Which problem is this PR solving?
- Part of jaegertracing#5334

## Description of the changes
- An API design of Storage V2 spanstore and its proto file.
- For the detailed discussion and how we arrived to this design, please
take a look at
https://docs.google.com/document/d/1IvUcDsdRxMPK-xTUE32w3NSAGTkUcmnDQttN6ijUnWs/edit?usp=sharing

## How was this change tested?
- This PR hasn't been tested yet since it only contains interfaces and
no actual implementation to be tested.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: James Ryans <james.ryans2012@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants