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

Add jaeger tracing backend support #2562

Closed

Conversation

haubenr
Copy link

@haubenr haubenr commented Jul 26, 2022

Changes

This PR adds Jaeger tracing backend support to the tracing package (in addition to the already existing Zipkin support).
Parts of the code in this PR were taken from an earlier (and seemingly abandoned code change proposed here: #1079

/kind enhancement

Fixes #2561

Signed-off-by: Dirk Haubenreisser <[email protected]>
Signed-off-by: Dirk Haubenreisser <[email protected]>
@knative-prow knative-prow bot added kind/enhancement size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 26, 2022
@knative-prow
Copy link

knative-prow bot commented Jul 26, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: haubenr
Once this PR has been reviewed and has the lgtm label, please assign dsimansk for approval by writing /assign @dsimansk in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #2562 (4152844) into main (3764d73) will decrease coverage by 0.23%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2562      +/-   ##
==========================================
- Coverage   81.61%   81.37%   -0.24%     
==========================================
  Files         163      163              
  Lines        9715     9747      +32     
==========================================
+ Hits         7929     7932       +3     
- Misses       1549     1579      +30     
+ Partials      237      236       -1     
Impacted Files Coverage Δ
tracing/opencensus.go 37.40% <0.00%> (-12.10%) ⬇️
controller/controller.go 88.78% <0.00%> (+0.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed7ab1d...4152844. Read the comment docs.

@haubenr
Copy link
Author

haubenr commented Jul 26, 2022

/test unit-tests_pkg_main

@haubenr
Copy link
Author

haubenr commented Jul 26, 2022

The Downstream Tekton 'tektoncd/pipeline' fails due to an unrelated issue with the knative.dev/pkg/changeset module where the interface expectation of tekton (wants to receive two return values) is not matched by the actual knative function implementation (returns one value).

@dprotaso
Copy link
Member

dprotaso commented Jul 26, 2022

/hold - see my comment on the issue

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2022
@haubenr
Copy link
Author

haubenr commented Jul 27, 2022

Alright, moving from OpenCensus to OpenTelemetry sound reasonable. I'll close this PR.

@haubenr haubenr closed this Jul 27, 2022
@haubenr haubenr deleted the haubenr_add-jaeger-tracing-backend branch July 27, 2022 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/enhancement size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Jaeger tracing backends
2 participants