Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Create v2 QueryService #6337

Closed
yurishkuro opened this issue Dec 11, 2024 · 0 comments
Closed

Create v2 QueryService #6337

yurishkuro opened this issue Dec 11, 2024 · 0 comments

Comments

@yurishkuro
Copy link
Member

The components of /cmd/query all depend on a QueryService which is a higher level abstraction that spanstore.Reader as it performs post-query adjustments on the trace before it is returned to the client.

In order to upgrade all the components in query to V2 Storage API we need to create a QueryServiceV2 that will operate on OTLP data instead of legacy model.Span.

The proposal is:

  • clone QueryService and update to OTLP data model
  • change query app to instantiate both of these structs temporarily
  • then we can start upgrading the endpoint handlers to work with QueryServiceV2, starting with apiv3 handler which already operates on OTLP and thus would require minimal changes
yurishkuro pushed a commit that referenced this issue Dec 25, 2024
## Which problem is this PR solving?
- Towards #6337 

## Description of the changes
- This PR implements a helper `AggregateTraces` in the `jptrace` package
to aggregate a sequence of `[]ptrace.Traces` to `ptrace.Traces`. This
was done by combining contiguous trace chunks together based on the
traceID.

## How was this change tested?
- Added 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>
yurishkuro pushed a commit that referenced this issue Dec 31, 2024
## Which problem is this PR solving?
- Towards #6337 

## Description of the changes
- Created a `v2` directory under querysvc and moved the new adjusters
into that package. This will also allow us to make a separate package
for the v2 query service under this new directory.

## How was this change tested?
- CI

## 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 added a commit that referenced this issue Dec 31, 2024
…6343)

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

## Description of the changes
- Implement a v2 version of the query service that operates on the OTLP
data model. This PR will be followed up by a series of PRs where this
this new query service will be updated with the existing handlers. Once
all the handlers have been migrated to use this query service, we can
remove the old one.

## How was this change tested?
- Added 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
mahadzaryab1 added a commit that referenced this issue Jan 1, 2025
…es (#6453)

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

## Description of the changes
- The query service was currently performing aggregation when the
`raw_traces` flag is set. This PR fixes that by removing aggregation
when the flag is set.

## How was this change tested?
- Adding 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: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
yurishkuro pushed a commit that referenced this issue Jan 1, 2025
…6452)

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

## Description of the changes
- This PR migrates the v3_api GRPC handler to use the new v2 query
service.

## How was this change tested?
- CI

## 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>
Manik2708 pushed a commit to Manik2708/jaeger that referenced this issue Jan 2, 2025
…aegertracing#6452)

## Which problem is this PR solving?
- Towards jaegertracing#6337

## Description of the changes
- This PR migrates the v3_api GRPC handler to use the new v2 query
service.

## How was this change tested?
- CI

## 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>
Manik2708 pushed a commit to Manik2708/jaeger that referenced this issue Jan 5, 2025
## Which problem is this PR solving?
- Towards jaegertracing#6337 

## Description of the changes
- Created a `v2` directory under querysvc and moved the new adjusters
into that package. This will also allow us to make a separate package
for the v2 query service under this new directory.

## How was this change tested?
- CI

## 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>
Manik2708 pushed a commit to Manik2708/jaeger that referenced this issue Jan 5, 2025
…aegertracing#6343)

## Which problem is this PR solving?
- Towards jaegertracing#6337 

## Description of the changes
- Implement a v2 version of the query service that operates on the OTLP
data model. This PR will be followed up by a series of PRs where this
this new query service will be updated with the existing handlers. Once
all the handlers have been migrated to use this query service, we can
remove the old one.

## How was this change tested?
- Added 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
Manik2708 pushed a commit to Manik2708/jaeger that referenced this issue Jan 5, 2025
…es (jaegertracing#6453)

## Which problem is this PR solving?
- Towards jaegertracing#6337

## Description of the changes
- The query service was currently performing aggregation when the
`raw_traces` flag is set. This PR fixes that by removing aggregation
when the flag is set.

## How was this change tested?
- Adding 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: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Manik2708 pushed a commit to Manik2708/jaeger that referenced this issue Jan 5, 2025
…ing#6401)

## Which problem is this PR solving?
- Towards jaegertracing#6337 

## Description of the changes
- This PR implements a helper `AggregateTraces` in the `jptrace` package
to aggregate a sequence of `[]ptrace.Traces` to `ptrace.Traces`. This
was done by combining contiguous trace chunks together based on the
traceID.

## How was this change tested?
- Added 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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

1 participant