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

Replace APIv2 with APIv3 client in e2e tests #6424

Merged
merged 17 commits into from
Dec 28, 2024

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Dec 27, 2024

Which problem is this PR solving?

Description of the changes

  • Replace span_reader (api_v2 gRPC client) with trace_reader (api_v3 gRPC client)
  • Set ingester logs to info because kafkareceiver logs the binary content of the messages via debug
  • Move logic related to child process into binary.go
  • Preserve original config file name for readability when modifying it to add storage cleaner extension
  • Add timeout to writeTrace()
  • Bumped into a subtle issue that if Cmd.Env is not nil then the sub-process does not inherit any other env from the parent, which was causing issues with Kafka tests where we pass topic & encoding via env, but for V2 Reader I had to add another env to disable self-tracing. Changed this to always pass EnvOverrides directly via Cmd.Env instead of setting them on the parent process.

How was this change tested?

  • CI

Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Dec 27, 2024
Signed-off-by: Yuri Shkuro <[email protected]>
Copy link

codecov bot commented Dec 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.25%. Comparing base (e6caacb) to head (e372924).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
plugin/storage/memory/factory.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6424   +/-   ##
=======================================
  Coverage   96.24%   96.25%           
=======================================
  Files         368      368           
  Lines       20977    20978    +1     
=======================================
+ Hits        20189    20192    +3     
+ Misses        603      602    -1     
+ Partials      185      184    -1     
Flag Coverage Δ
badger_v1 10.56% <0.00%> (-0.01%) ⬇️
badger_v2 2.42% <0.00%> (-0.62%) ⬇️
cassandra-4.x-v1-manual 16.45% <0.00%> (-0.01%) ⬇️
cassandra-4.x-v2-auto 2.36% <0.00%> (-0.62%) ⬇️
cassandra-4.x-v2-manual 2.36% <0.00%> (-0.62%) ⬇️
cassandra-5.x-v1-manual 16.45% <0.00%> (-0.01%) ⬇️
cassandra-5.x-v2-auto 2.36% <0.00%> (-0.62%) ⬇️
cassandra-5.x-v2-manual 2.36% <0.00%> (-0.62%) ⬇️
elasticsearch-6.x-v1 20.20% <0.00%> (-0.01%) ⬇️
elasticsearch-7.x-v1 20.26% <0.00%> (-0.01%) ⬇️
elasticsearch-8.x-v1 20.43% <0.00%> (+<0.01%) ⬆️
elasticsearch-8.x-v2 2.41% <0.00%> (-0.79%) ⬇️
grpc_v1 12.23% <0.00%> (-0.01%) ⬇️
grpc_v2 8.82% <0.00%> (-0.51%) ⬇️
kafka-3.x-v1 10.40% <0.00%> (-0.01%) ⬇️
kafka-3.x-v2 2.42% <0.00%> (-0.64%) ⬇️
memory_v2 2.42% <0.00%> (-0.63%) ⬇️
opensearch-1.x-v1 20.31% <0.00%> (-0.02%) ⬇️
opensearch-2.x-v1 20.32% <0.00%> (-0.01%) ⬇️
opensearch-2.x-v2 2.41% <0.00%> (-0.64%) ⬇️
tailsampling-processor 0.39% <0.00%> (-0.17%) ⬇️
unittests 95.11% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yurishkuro and others added 13 commits December 27, 2024 00:48
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro marked this pull request as ready for review December 27, 2024 22:13
@yurishkuro yurishkuro requested a review from a team as a code owner December 27, 2024 22:13
@yurishkuro yurishkuro changed the title Apiv3 reader Replace APIv2 with APIv3 client in e2e tests Dec 27, 2024
Co-authored-by: Mahad Zaryab <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro enabled auto-merge (squash) December 28, 2024 04:24
@yurishkuro yurishkuro disabled auto-merge December 28, 2024 04:33
@yurishkuro yurishkuro merged commit e7a0205 into jaegertracing:main Dec 28, 2024
53 of 54 checks passed
@yurishkuro yurishkuro deleted the apiv3-reader branch December 28, 2024 04:33
Manik2708 pushed a commit to Manik2708/jaeger that referenced this pull request Jan 5, 2025
## Which problem is this PR solving?
- Resolves jaegertracing#6422

## Description of the changes
- Replace span_reader (api_v2 gRPC client) with trace_reader (api_v3
gRPC client)
- Set ingester logs to `info` because `kafkareceiver` logs the binary
content of the messages via `debug`
- Move logic related to child process into `binary.go`
- Preserve original config file name for readability when modifying it
to add storage cleaner extension
- Add timeout to `writeTrace()`
- Bumped into a subtle issue that if Cmd.Env is not nil then the
sub-process does not inherit any other env from the parent, which was
causing issues with Kafka tests where we pass topic & encoding via env,
but for V2 Reader I had to add another env to disable self-tracing.
Changed this to always pass EnvOverrides directly via Cmd.Env instead of
setting them on the parent process.

## How was this change tested?
- CI

---------

Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: Mahad Zaryab <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/storage changelog:ci Change related to continuous integration / testing storage/kafka
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[e2e] Use APIv3 gRPC endpoint for remote queries
2 participants