Skip to content

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 <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Dec 27, 2024
Signed-off-by: Yuri Shkuro <github@ysh.us>
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 <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@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 <43658574+mahadzaryab1@users.noreply.github.com>
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
@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 <github@ysh.us>
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com>
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