[TEP-0124] implement opentelemetry Jaeger tracing #5746

kmjayadeep · 2022-11-11T20:38:58Z

Adds opentelemetry instrumentation code to pipelinerun and taskrun reconcilers. Also made required changes in the main.go to include jaeger as tracing backend

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Note to reviewers: There are pipeline failures (looks like they are not related to the changes). Once the review is complete and ready to approve, I will address the pipeline issues, rebase with main and squash all commits into one.

Release Notes

Added support for tracing using Jaeger and OpenTelemetry. It can be enabled by adding the following environment variables to controller manifest
* `OTEL_EXPORTER_JAEGER_ENDPOINT` is the HTTP endpoint for sending spans directly to a collector.
* `OTEL_EXPORTER_JAEGER_USER` is the username to be sent as authentication to the collector endpoint. (optional)
* `OTEL_EXPORTER_JAEGER_PASSWORD` is the password to be sent as authentication to the collector endpoint. (optional)

tekton-robot · 2022-11-11T20:39:07Z

Hi @kmjayadeep. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

kmjayadeep · 2022-11-11T20:39:08Z

/kind feature

afrittoli · 2022-11-11T22:11:17Z

/ok-to-test

pkg/reconciler/pipelinerun/pipelinerun.go

tekton-robot · 2022-11-11T22:18:20Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	85.6%	83.7%	-1.9
pkg/reconciler/pipelinerun/tracing.go	Do not exist	88.9%
pkg/reconciler/taskrun/taskrun.go	81.5%	82.5%	1.0
pkg/reconciler/taskrun/tracing.go	Do not exist	89.3%

tekton-robot · 2022-11-12T05:15:26Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	85.6%	85.8%	0.2
pkg/reconciler/pipelinerun/tracing.go	Do not exist	88.9%
pkg/reconciler/taskrun/taskrun.go	81.5%	82.5%	1.0
pkg/reconciler/taskrun/tracing.go	Do not exist	89.3%

tekton-robot · 2022-11-12T07:01:09Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	85.6%	85.8%	0.2
pkg/reconciler/pipelinerun/tracing.go	Do not exist	88.9%
pkg/reconciler/taskrun/taskrun.go	81.5%	82.5%	1.0
pkg/reconciler/taskrun/tracing.go	Do not exist	89.3%

afrittoli · 2022-11-13T23:18:05Z

From the error in the build tests it might be that something went wrong in the rebase again, but you could try and run the codegen script to see if it fixes the error

kmjayadeep · 2022-11-14T07:41:17Z

Hi @afrittoli I tried the codegen script already. It has introduced some formatting changes, but didn't fix the build.

afrittoli · 2022-11-14T10:24:59Z

Hi @afrittoli I tried the codegen script already. It has introduced some formatting changes, but didn't fix the build.

I had a look and your commit actually includes formatting changes to generated files, which causes the build job to fail.

pkg/client/clientset/versioned/fake/register.go                                               |   14 +-
pkg/client/clientset/versioned/scheme/register.go                                             |   14 +-
pkg/client/resolution/clientset/versioned/fake/register.go                                    |   14 +-
pkg/client/resolution/clientset/versioned/scheme/register.go                                  |   14 +-
pkg/client/resource/clientset/versioned/fake/register.go                                      |   14 +-
pkg/client/resource/clientset/versioned/scheme/register.go                                    |   14 +-

I tried reverting those and running the codegen again and they appear back, so I suspect this might be related to a change in go version between CI and your/mine local go version.

My go version is:

go version go1.19.1 darwin/arm64

while CI runs on 1.18.

For this specific CI, I would recommend reverting those changes and adding them to your commit. You can skip the codegen script since there is no change in the PR which requires it:

git checkout origin/main -- pkg/client/clientset/versioned/fake/register.go
git checkout origin/main -- pkg/client/clientset/versioned/scheme/register.go
git checkout origin/main -- pkg/client/resolution/clientset/versioned/fake/register.go
git checkout origin/main -- pkg/client/resolution/clientset/versioned/scheme/register.go
git checkout origin/main -- pkg/client/resource/clientset/versioned/fake/register.go
git checkout origin/main -- pkg/client/resource/clientset/versioned/scheme/register.go

tekton-robot · 2022-11-14T11:59:31Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	85.6%	85.8%	0.2
pkg/reconciler/pipelinerun/tracing.go	Do not exist	88.9%
pkg/reconciler/taskrun/taskrun.go	81.5%	82.5%	1.0
pkg/reconciler/taskrun/tracing.go	Do not exist	89.3%

tekton-robot · 2022-11-14T13:53:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	85.6%	85.8%	0.2
pkg/reconciler/pipelinerun/tracing.go	Do not exist	88.9%
pkg/reconciler/taskrun/taskrun.go	81.5%	82.5%	1.0
pkg/reconciler/taskrun/tracing.go	Do not exist	89.3%

pkg/reconciler/pipelinerun/pipelinerun.go

tekton-robot · 2022-11-14T14:41:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	85.6%	85.8%	0.2
pkg/reconciler/pipelinerun/tracing.go	Do not exist	88.9%
pkg/reconciler/taskrun/taskrun.go	81.5%	82.5%	1.0
pkg/reconciler/taskrun/tracing.go	Do not exist	89.3%

tekton-robot · 2023-01-20T12:04:38Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T12:33:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T12:35:15Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T12:54:42Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T12:55:50Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

Signed-off-by: Jayadeep KM <kmjayadeep@gmail.com>

tekton-robot · 2023-01-20T13:38:52Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T13:40:31Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T14:34:54Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T14:36:34Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

and fix a few linter issues. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

tekton-robot · 2023-01-20T15:03:47Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

tekton-robot · 2023-01-20T15:04:29Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	86.2%	86.6%	0.4
pkg/reconciler/pipelinerun/tracing.go	Do not exist	75.8%
pkg/reconciler/taskrun/taskrun.go	83.6%	84.5%	0.9
pkg/reconciler/taskrun/tracing.go	Do not exist	90.9%

afrittoli · 2023-01-23T12:10:57Z

docs/pipeline-api.md

+</tr><tr><td><p>&#34;TaskRunResultLargerThanAllowedLimit&#34;</p></td>
+<td><p>TaskRunReasonResultLargerThanAllowedLimit is the reason set when one of the results exceeds its maximum allowed limit of 1 KB</p>
+</td>


It looks like these were missed in some previous PR, but I wonder by CI did not catch them.
@vdemeester @abayer any idea?

afrittoli · 2023-01-23T12:11:06Z

docs/pipeline-api.md

+Supported formats differ based on the KMS system used.
+One example of a KMS url could be:
+gcpkms://projects/[PROJECT]/locations/[LOCATION]&gt;/keyRings/[KEYRING]/cryptoKeys/[KEY]/cryptoKeyVersions/[KEY_VERSION]
+For more examples please refer <a href="https://docs.sigstore.dev/cosign/kms_support">https://docs.sigstore.dev/cosign/kms_support</a>.
+Note that the KMS is not supported yet.</p>


afrittoli · 2023-01-23T12:16:00Z

pkg/reconciler/pipelinerun/tracing.go

+	spanContext := make(map[string]string)
+
+	// SpanContext was propogated through annotations
+	if pr.Annotations != nil && pr.Annotations[SpanContextAnnotation] != "" {


Nice! These means that a PipelineRun too can receive an input context, which means we can support existing contexts for pipelines, like in the case of pipeline in pipeline, or a pipeline triggered via CloudEvent with a tracing context in it.

afrittoli

Thank you @kmjayadeep
/lgtm

tekton-robot · 2023-01-23T13:04:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [vdemeester]

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

tekton-robot requested review from abayer and pritidesai November 11, 2022 20:39

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 11, 2022

kmjayadeep force-pushed the tep-0124 branch from ea852e2 to 4407664 Compare November 11, 2022 20:40

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 11, 2022

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 11, 2022

afrittoli reviewed Nov 11, 2022

View reviewed changes

pkg/reconciler/pipelinerun/pipelinerun.go Outdated Show resolved Hide resolved

kmjayadeep force-pushed the tep-0124 branch from 4407664 to 5c0a308 Compare November 12, 2022 05:10

kmjayadeep force-pushed the tep-0124 branch from 5c0a308 to 7e1c63b Compare November 12, 2022 06:56

kmjayadeep force-pushed the tep-0124 branch from 7e1c63b to cef7aa9 Compare November 14, 2022 11:54

kmjayadeep force-pushed the tep-0124 branch from cef7aa9 to d99fe3b Compare November 14, 2022 13:48

github-advanced-security bot found potential problems Nov 14, 2022

View reviewed changes

pkg/reconciler/pipelinerun/pipelinerun.go Fixed Show fixed Hide fixed

kmjayadeep force-pushed the tep-0124 branch from d99fe3b to cc6f852 Compare November 14, 2022 14:36

afrittoli force-pushed the tep-0124 branch from f38f41c to 46333e5 Compare January 20, 2023 12:48

kmjayadeep added 2 commits January 20, 2023 13:32

Ran make generated command

d8aff7d

Signed-off-by: Jayadeep KM <kmjayadeep@gmail.com>

formatting and go.mod changes

dde7837

Signed-off-by: Jayadeep KM <kmjayadeep@gmail.com>

afrittoli force-pushed the tep-0124 branch from 46333e5 to 8ab4181 Compare January 20, 2023 13:32

afrittoli force-pushed the tep-0124 branch from 8ab4181 to 1e06bb0 Compare January 20, 2023 14:29

Update generated docs/pipeline-api.md

adc1d04

and fix a few linter issues. Signed-off-by: Andrea Frittoli <andrea.frittoli@uk.ibm.com>

afrittoli force-pushed the tep-0124 branch from 1e06bb0 to adc1d04 Compare January 20, 2023 14:57

afrittoli reviewed Jan 23, 2023

View reviewed changes

tekton-robot assigned afrittoli Jan 23, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2023

vdemeester approved these changes Jan 23, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 23, 2023

tekton-robot merged commit 000c3af into tektoncd:main Jan 23, 2023

kmjayadeep deleted the tep-0124 branch January 23, 2023 14:18

gabemontero mentioned this pull request Apr 8, 2024

Enable Native OpenTelemetry Tracing ADR konflux-ci/architecture#177

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TEP-0124] implement opentelemetry Jaeger tracing #5746

[TEP-0124] implement opentelemetry Jaeger tracing #5746

kmjayadeep commented Nov 11, 2022 •

edited

Loading

tekton-robot commented Nov 11, 2022

kmjayadeep commented Nov 11, 2022

afrittoli commented Nov 11, 2022

tekton-robot commented Nov 11, 2022

tekton-robot commented Nov 12, 2022

tekton-robot commented Nov 12, 2022

afrittoli commented Nov 13, 2022

kmjayadeep commented Nov 14, 2022

afrittoli commented Nov 14, 2022

tekton-robot commented Nov 14, 2022

tekton-robot commented Nov 14, 2022

tekton-robot commented Nov 14, 2022

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

afrittoli Jan 23, 2023

afrittoli Jan 23, 2023

afrittoli Jan 23, 2023

afrittoli left a comment

tekton-robot commented Jan 23, 2023

[TEP-0124] implement opentelemetry Jaeger tracing #5746

[TEP-0124] implement opentelemetry Jaeger tracing #5746

Conversation

kmjayadeep commented Nov 11, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Nov 11, 2022

kmjayadeep commented Nov 11, 2022

afrittoli commented Nov 11, 2022

tekton-robot commented Nov 11, 2022

tekton-robot commented Nov 12, 2022

tekton-robot commented Nov 12, 2022

afrittoli commented Nov 13, 2022

kmjayadeep commented Nov 14, 2022

afrittoli commented Nov 14, 2022

tekton-robot commented Nov 14, 2022

tekton-robot commented Nov 14, 2022

tekton-robot commented Nov 14, 2022

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

tekton-robot commented Jan 20, 2023

afrittoli Jan 23, 2023

Choose a reason for hiding this comment

afrittoli Jan 23, 2023

Choose a reason for hiding this comment

afrittoli Jan 23, 2023

Choose a reason for hiding this comment

afrittoli left a comment

Choose a reason for hiding this comment

tekton-robot commented Jan 23, 2023

kmjayadeep commented Nov 11, 2022 •

edited

Loading