test: add e2e test for OpenCensus to OpenTelemetry metrics migration#1735
test: add e2e test for OpenCensus to OpenTelemetry metrics migration#1735khrm wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
tekton-robot
added
the
size/L
khrm
force-pushed
the
e2e-otel-metrics-test
branch
5 times, most recently
from
73f1a37 to
292afe2
Compare
Adds TestOTelMetrics, a consolidated e2e test for the OC→OTel metrics migration in Chains (PR tektoncd#1550). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
ko v0.19.0 introduced strict enforcement that kodata symlinks must not resolve outside the kodata root (ko-build/ko#1619). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
khrm
force-pushed
the
e2e-otel-metrics-test
branch
from
292afe2 to
e521dd8
Compare
| removedMetrics := []string{ | ||
| "tekton_chains_taskrun_signed_total", | ||
| "tekton_chains_pipelinerun_signed_total", | ||
| } |
There was a problem hiding this comment.
We can probably remove these, these metrics never existed I believe. This will pass in all cases.
There was a problem hiding this comment.
@khrm as per earlier opencensus based implementation Knative's sharedmain uses the component name as the OC metrics domain/namespace, which gets prepended as a Prometheus prefix. So the old OC Prometheus names were already watcher_taskrun_sign_created_total, watcher_taskrun_payload_stored_total, etc. identical to the new OTel names. If we prefer to check for removed metric names recommend that to be on the knative metric names which were like watcher_workqueue_, etc., instead of kn_ now. Otherwise please ignore this block to verify removed metrics altogether
|
/retest |
|
/ok-to-test |
|
/retest |
|
@khrm e2e are failing, can you have a look please |
|
Closed and reopened to rerun the CI |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1735 +/- ##
=======================================
Coverage ? 60.12%
=======================================
Files ? 65
Lines ? 4050
Branches ? 0
=======================================
Hits ? 2435
Misses ? 1355
Partials ? 260
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| removedMetrics := []string{ | ||
| "tekton_chains_taskrun_signed_total", | ||
| "tekton_chains_pipelinerun_signed_total", | ||
| } |
There was a problem hiding this comment.
@khrm as per earlier opencensus based implementation Knative's sharedmain uses the component name as the OC metrics domain/namespace, which gets prepended as a Prometheus prefix. So the old OC Prometheus names were already watcher_taskrun_sign_created_total, watcher_taskrun_payload_stored_total, etc. identical to the new OTel names. If we prefer to check for removed metric names recommend that to be on the knative metric names which were like watcher_workqueue_, etc., instead of kn_ now. Otherwise please ignore this block to verify removed metrics altogether
6 participants
Adds TestOTelMetrics, a consolidated e2e test for the OC→OTel metrics migration in Chains (PR #1550). The test creates a TaskRun and a PipelineRun, waits for Chains to sign them, then scrapes the controller /metrics endpoint via port-forward to assert:
Uses expfmt/dto.MetricFamily for proper Prometheus text parsing and a baseline scrape before creating resources to compute deltas, avoiding false positives from earlier test runs.
Relates to #1550
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes