Add Metrics for Running PipelinesRuns at Pipeline and Namespace level #8280

pramodbindal · 2024-09-19T06:23:22Z

Changes

Currently metrics shown for Running Pipeline Count is at cluster level.
There is no way we can get that metric at namespace or pipeline level.
We have added the PipelineRun metric at pipeline and namespace level.
Level of PipelineRun Can be configured via ConfigMap. Default is "" . but it can be set to namespace or Pipelinerun.

Submitter Checklist

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

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
pre-commit Passed
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). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

tekton-robot · 2024-09-19T06:23:31Z

Hi @pramodbindal. 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.

khrm

/ok-to-test

khrm · 2024-09-19T06:25:32Z

/kind feature

khrm · 2024-09-19T06:25:45Z

/test check-pr-has-kind-label

tekton-robot · 2024-09-19T06:25:48Z

@khrm: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-beta-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test pull-tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

In response to this:

/test check-pr-has-kind-label

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.

tekton-robot · 2024-09-19T06:33:37Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

tekton-robot · 2024-09-19T06:35:11Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

tekton-robot · 2024-09-19T07:03:34Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

tekton-robot · 2024-09-19T07:05:23Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

pramodbindal · 2024-09-19T07:24:31Z

/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests

vdemeester

@pramodbindal can you squash your commits ? Otherwise LGTM.

pramodbindal · 2024-09-19T08:51:12Z

@pramodbindal can you squash your commits ? Otherwise LGTM.

Done

tekton-robot · 2024-09-19T08:53:22Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

tekton-robot · 2024-09-19T08:56:56Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

vdemeester

We might want to add a release note entry
It seems it needs to be rebased ?

tekton-robot · 2024-09-25T08:20:09Z

[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

pramodbindal · 2024-09-26T07:25:41Z

@pritidesai @abayer
This PR is Approved by @vdemeester .
Please provide /lgtm tag so I can merge this.

tekton-robot · 2024-09-26T11:14:49Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	83.7%	-1.7

tekton-robot · 2024-10-09T09:41: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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-09T09:42:38Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-10T08:56:50Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-10T08:58:43Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-10T09:54:55Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-10T09:55:32Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-10T09:55:45Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

pramodbindal · 2024-10-10T10:12:36Z

/retest

tekton-robot · 2024-10-10T10:31:22Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

tekton-robot · 2024-10-10T11:02:28Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	84.0%	-1.4

afrittoli

Thank you, this looks good!
My only ask is to extend the test to cover the various config options that you introduced, since right now only one out of four options is tested.

afrittoli · 2024-10-17T08:25:42Z

pkg/pipelinerunmetrics/metrics_test.go

+		newPipelineRun(corev1.ConditionUnknown, "testns1"),
+		newPipelineRun(corev1.ConditionUnknown, "testns2"),
+		newPipelineRun(corev1.ConditionUnknown, "testns2"),
+		newPipelineRun(corev1.ConditionUnknown, "testns3"),
+		newPipelineRun(corev1.ConditionUnknown, "testns3"),
+		newPipelineRun(corev1.ConditionUnknown, "testns3"),
+		newPipelineRun(corev1.ConditionUnknown, "testns3"),


Thank you for introducing more namespaces. This way the test checks that aggregation is happening at the namespace level. It does not check anymore that the filtering of the completed pipeline runs - is there a reason you removed them?

afrittoli · 2024-10-17T08:26:52Z

pkg/pipelinerunmetrics/metrics_test.go

+		}
+	}
+
+	ctx = getConfigContextRunningPRLevel("pipeline")


The configuration is set to "pipeline" but all PipelineRuns use the same "anonymous" pipeline, so this setting is not fully tested. What is verified is aggregation at namespace level and the tagging at pipeline level.

pkg/pipelinerunmetrics/metrics.go

afrittoli · 2024-10-17T08:51:05Z

pkg/pipelinerunmetrics/metrics.go

 		if !pr.IsDone() {
+			countMap[pipelineRunKey]++
+			metrics.Record(ctx_, runningPRs.M(float64(countMap[pipelineRunKey])))


As discussed, we'll keep this as it is for now.
I still think that recording once the correct value would be better than recording incremental values, but we can address that separately.

afrittoli · 2024-10-17T08:54:19Z

pkg/pipelinerunmetrics/metrics_test.go

+	if err := recorder.RunningPipelineRuns(informer.Lister()); err != nil {
+		t.Errorf("RunningPipelineRuns: %v", err)
+	}
+	metricstest.CheckLastValueData(t, "running_pipelineruns", map[string]string{"namespace": "testns", "pipeline": "anonymous"}, 1)


We should have one test per level in this PR. You can easily make this a matrix test with three fields:

the list of pipelineruns

the config level

a list of map[string]string -> int for expected values

tekton-robot · 2024-10-17T10:10:29Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	86.5%	1.2

tekton-robot · 2024-10-17T10:11:45Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	86.5%	1.2

afrittoli

Thanks for adding this and for all the updates!
/lgtm

afrittoli · 2024-10-17T10:22:58Z

/hold cancel

We have added the PipelineRun metric at pipeline and namespace level. Fix Lint Error Fix Lint Error Adding Tests for better coverage of different running-pipelinerun use cases. Fix Lint

afrittoli · 2024-10-17T10:41:56Z

/lgtm

tekton-robot · 2024-10-17T10:45:44Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	86.5%	1.2

tekton-robot · 2024-10-17T10:47:41Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	86.5%	1.2

tekton-robot · 2024-10-17T10:58:07Z

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/apis/config/metrics.go	76.2%	78.3%	2.1
pkg/pipelinerunmetrics/metrics.go	85.4%	86.5%	1.2

tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Sep 19, 2024

tekton-robot requested review from abayer and pritidesai September 19, 2024 06:23

tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 19, 2024

khrm reviewed Sep 19, 2024

View reviewed changes

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 Sep 19, 2024

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 19, 2024

vdemeester reviewed Sep 19, 2024

View reviewed changes

pramodbindal force-pushed the SRVKP-6184 branch from b802e66 to 6951973 Compare September 19, 2024 08:41

vdemeester approved these changes Sep 25, 2024

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2024

pramodbindal force-pushed the SRVKP-6184 branch 2 times, most recently from 5c2ef54 to 4a05727 Compare September 26, 2024 11:01

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 8, 2024

pramodbindal force-pushed the SRVKP-6184 branch from 785e2e6 to d1757a6 Compare October 10, 2024 10:19

afrittoli reviewed Oct 17, 2024

View reviewed changes

tekton-robot assigned afrittoli Oct 17, 2024

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2024

tekton-robot removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Oct 17, 2024

Add Metrics for Running PipelinesRuns at Pipeline and Namespace level

e596d5e

We have added the PipelineRun metric at pipeline and namespace level. Fix Lint Error Fix Lint Error Adding Tests for better coverage of different running-pipelinerun use cases. Fix Lint

pramodbindal force-pushed the SRVKP-6184 branch from 673ad56 to e596d5e Compare October 17, 2024 10:34

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2024

tekton-robot merged commit 0350a6e into tektoncd:main Oct 17, 2024
14 checks passed

pramodbindal deleted the SRVKP-6184 branch October 18, 2024 06:07

Add Metrics for Running PipelinesRuns at Pipeline and Namespace level #8280

Add Metrics for Running PipelinesRuns at Pipeline and Namespace level #8280

Conversation

pramodbindal commented Sep 19, 2024 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Sep 19, 2024

khrm left a comment

Choose a reason for hiding this comment

khrm commented Sep 19, 2024

khrm commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

pramodbindal commented Sep 19, 2024

vdemeester left a comment

Choose a reason for hiding this comment

pramodbindal commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

tekton-robot commented Sep 19, 2024

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Sep 25, 2024

pramodbindal commented Sep 26, 2024

tekton-robot commented Sep 26, 2024

tekton-robot commented Oct 9, 2024

tekton-robot commented Oct 9, 2024

tekton-robot commented Oct 10, 2024

tekton-robot commented Oct 10, 2024

tekton-robot commented Oct 10, 2024

tekton-robot commented Oct 10, 2024

tekton-robot commented Oct 10, 2024

pramodbindal commented Oct 10, 2024

tekton-robot commented Oct 10, 2024

tekton-robot commented Oct 10, 2024

afrittoli left a comment

Choose a reason for hiding this comment

afrittoli Oct 17, 2024

Choose a reason for hiding this comment

afrittoli Oct 17, 2024

Choose a reason for hiding this comment

afrittoli Oct 17, 2024

Choose a reason for hiding this comment

afrittoli Oct 17, 2024

Choose a reason for hiding this comment

tekton-robot commented Oct 17, 2024

tekton-robot commented Oct 17, 2024

afrittoli left a comment

Choose a reason for hiding this comment

afrittoli commented Oct 17, 2024

afrittoli commented Oct 17, 2024

tekton-robot commented Oct 17, 2024

tekton-robot commented Oct 17, 2024

tekton-robot commented Oct 17, 2024

pramodbindal commented Sep 19, 2024 •

edited

Loading