Skip to content

Conversation

@Frapschen
Copy link
Contributor

Which problem is this PR solving?

  • fix jaeger-v2 query extension EnableTracing param not work

Description of the changes

  • new tracerProvider only if EnableTracing is true

How was this change tested?

Checklist

@Frapschen Frapschen requested a review from a team as a code owner June 14, 2025 09:36
@Frapschen Frapschen requested a review from mahadzaryab1 June 14, 2025 09:36
@dosubot dosubot bot added the v2 label Jun 14, 2025
@codecov
Copy link

codecov bot commented Jun 14, 2025

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.61%. Comparing base (34d99f7) to head (95e44a5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...md/jaeger/internal/extension/jaegerquery/server.go 72.22% 3 Missing and 2 partials ⚠️

❌ Your patch status has failed because the patch coverage (77.27%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7226      +/-   ##
==========================================
- Coverage   96.62%   96.61%   -0.01%     
==========================================
  Files         378      378              
  Lines       23146    23154       +8     
==========================================
+ Hits        22364    22371       +7     
  Misses        596      596              
- Partials      186      187       +1     
Flag Coverage Δ
badger_v1 9.04% <ø> (ø)
badger_v2 1.70% <ø> (ø)
cassandra-4.x-v1-manual 11.70% <ø> (ø)
cassandra-4.x-v2-auto 1.69% <ø> (ø)
cassandra-4.x-v2-manual 1.69% <ø> (ø)
cassandra-5.x-v1-manual 11.70% <ø> (ø)
cassandra-5.x-v2-auto 1.69% <ø> (ø)
cassandra-5.x-v2-manual 1.69% <ø> (ø)
elasticsearch-6.x-v1 16.61% <ø> (ø)
elasticsearch-7.x-v1 16.65% <ø> (ø)
elasticsearch-8.x-v1 16.79% <ø> (ø)
elasticsearch-8.x-v2 1.70% <ø> (ø)
elasticsearch-9.x-v2 1.70% <ø> (ø)
grpc_v1 10.24% <ø> (ø)
grpc_v2 1.70% <ø> (ø)
kafka-3.x-v1 9.68% <ø> (ø)
kafka-3.x-v2 1.70% <ø> (ø)
memory_v2 1.70% <ø> (ø)
opensearch-1.x-v1 16.70% <ø> (ø)
opensearch-2.x-v1 16.70% <ø> (ø)
opensearch-2.x-v2 1.70% <ø> (ø)
opensearch-3.x-v2 1.70% <ø> (ø)
query 1.70% <ø> (ø)
tailsampling-processor 0.47% <ø> (ø)
unittests 95.60% <77.27%> (-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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yurishkuro
Copy link
Member

In jaeger v1 we always had self-tracing on by default, so I'm not sure how that property was used. It needs to remain on by default with this change.

@Frapschen
Copy link
Contributor Author

@yurishkuro This PR will disable self-tracing by default. I've checked the latest Jaeger v1, and it is also disabled by default there.

The screenshot is from 1.70
image

@yurishkuro
Copy link
Member

The v1 all-in-one has query tracing on by default, and so is jaeger-v2.

Signed-off-by: Murphy Chen <[email protected]>
@Frapschen
Copy link
Contributor Author

@yurishkuro Please review

Signed-off-by: Murphy Chen <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro enabled auto-merge July 17, 2025 02:35
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You may re-open it if you need more time.

@github-actions github-actions bot added the stale The issue/PR has become stale and may be auto-closed label Sep 22, 2025
@yurishkuro yurishkuro changed the title fix jaeger-v2 query extension EnableTracing param not work [fix] Make EnableTracing param work correctly in jaeger-v2 query extension Sep 22, 2025
@yurishkuro yurishkuro disabled auto-merge September 22, 2025 18:23
@yurishkuro yurishkuro merged commit 264c3f3 into jaegertracing:main Sep 22, 2025
65 of 67 checks passed
@Frapschen Frapschen deleted the fix-jaeger-v2-query-tracing-param branch September 23, 2025 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:bugfix-or-minor-feature stale The issue/PR has become stale and may be auto-closed v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants