feat: add ability to include reason in count metrics #7060

AverageMarcus · 2023-08-22T15:26:04Z

Changes

Introduces the ability to include a reason label along with the current status label on the PipelineRun and TaskRun count metrics.

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
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

Add ability to include `reason` along with `status` in TaskRun and PipelineRun count metrics

/kind feature

tekton-robot · 2023-08-22T15:27:07Z

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

tekton-robot · 2023-08-22T15:35:22Z

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	70.6%	68.4%	-2.2
pkg/taskrunmetrics/metrics.go	83.0%	82.9%	-0.1

tekton-robot · 2023-08-22T15:38: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	70.6%	73.7%	3.1
pkg/taskrunmetrics/metrics.go	83.0%	82.9%	-0.1

AverageMarcus · 2023-08-22T15:43:53Z

/test pull-tekton-pipeline-go-coverage-df

tekton-robot · 2023-08-22T15:44:07Z

@AverageMarcus: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-tekton-pipeline-go-coverage-df

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 · 2023-08-22T15:49:03Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	80.0%	81.0%	1.0
pkg/taskrunmetrics/metrics.go	83.0%	83.6%	0.6

tekton-robot · 2023-08-22T15:52:14Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	80.0%	81.0%	1.0
pkg/taskrunmetrics/metrics.go	83.0%	83.6%	0.6

khrm

/ok-to-test

tekton-robot · 2023-08-22T18:51:57Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	80.0%	81.0%	1.0
pkg/taskrunmetrics/metrics.go	83.0%	83.6%	0.6

khrm · 2023-08-22T18:56:59Z

/test tekton-pipeline-unit-tests

AverageMarcus · 2023-08-22T19:00:14Z

Sorry, looks like I left in a small typo. 🤦 Updated now.

tekton-robot · 2023-08-22T19:05:37Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	80.0%	81.0%	1.0
pkg/taskrunmetrics/metrics.go	83.0%	83.6%	0.6

tekton-robot · 2023-08-22T19:07:16Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	80.0%	81.0%	1.0
pkg/taskrunmetrics/metrics.go	83.0%	83.6%	0.6

khrm

/approve

AverageMarcus · 2023-08-23T06:15:00Z

/assign @dibyom

AverageMarcus · 2023-09-15T09:40:25Z

Rebased! ✅

Is anyone available for a review?

tekton-robot · 2023-09-15T09:45: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/apis/config/metrics.go	70.6%	73.7%	3.1
pkg/taskrunmetrics/metrics.go	83.4%	84.0%	0.5

tekton-robot · 2023-09-15T09:49:14Z

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	70.6%	73.7%	3.1
pkg/taskrunmetrics/metrics.go	83.4%	84.0%	0.5

tekton-robot · 2023-09-25T07:56:39Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	82.1%	82.9%	0.8
pkg/taskrunmetrics/metrics.go	83.4%	84.0%	0.5

tekton-robot · 2023-09-25T07:59: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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	82.1%	82.9%	0.8
pkg/taskrunmetrics/metrics.go	83.4%	84.0%	0.5

dibyom · 2023-10-02T21:01:57Z

@AverageMarcus thanks for the PR (and sorry for the delay in reviewing) - I think this looks good and only have some naming nitpicks.

@vdemeester @khrm what do you think?

khrm

@dibyom Yes, this looks good to me.

AverageMarcus · 2023-10-03T11:51:01Z

only have some naming nitpicks.

I don't see any other comments. Do you still have them as pending or something gone wrong with GitHub?

dibyom · 2023-10-02T20:58:37Z

docs/metrics.md

@@ -40,6 +40,7 @@ A sample config-map has been provided as [config-observability](./../config/conf
    metrics.taskrun.duration-type: "histogram"
    metrics.pipelinerun.level: "pipeline"
    metrics.pipelinerun.duration-type: "histogram"
+    metrics.count.reason: "false"


nit: maybe metrics.count.enable-reason to be more explicit that this is for enabling this vs something else

Makes sense! 👍 Updated.

dibyom · 2023-10-05T14:45:10Z

I don't see any other comments. Do you still have them as pending or something gone wrong with GitHub?

yup, was pending 🤦

Adds a configuration flag to enable including the `reason` for a TaskRun or PipelineRun status on their count metrics. This allows for more fine-grained monitoring and alerting of run failures. Signed-off-by: Marcus Noble <[email protected]>

tekton-robot · 2023-10-05T15:01:08Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	82.1%	82.9%	0.8
pkg/taskrunmetrics/metrics.go	83.4%	84.0%	0.5

tekton-robot · 2023-10-05T15:02:08Z

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	70.6%	73.7%	3.1
pkg/pipelinerunmetrics/metrics.go	82.1%	82.9%	0.8
pkg/taskrunmetrics/metrics.go	83.4%	84.0%	0.5

AverageMarcus · 2023-10-12T08:59:28Z

@dibyom any chance I could get another review please?

dibyom · 2023-10-12T15:01:41Z

Looks good to me, thanks for your patience @AverageMarcus 🙏

/approve

tekton-robot · 2023-10-12T15:01:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom, khrm

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~~ [dibyom]

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

AverageMarcus · 2023-10-12T15:06:16Z

Thanks @dibyom 🎉 It looks like tide also requires a lgtm label too though. 😅

dibyom

/lgtm

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 22, 2023

tekton-robot requested review from jerop and vdemeester August 22, 2023 15:27

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 Aug 22, 2023

AverageMarcus force-pushed the metrics_count_reason branch from d490073 to e8bb945 Compare August 22, 2023 15:30

AverageMarcus force-pushed the metrics_count_reason branch from e8bb945 to b169232 Compare August 22, 2023 15:41

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 22, 2023

khrm reviewed Aug 22, 2023

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 Aug 22, 2023

AverageMarcus force-pushed the metrics_count_reason branch from b169232 to 17a7fe3 Compare August 22, 2023 18:59

khrm reviewed Aug 23, 2023

View reviewed changes

tekton-robot assigned dibyom Aug 23, 2023

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2023

AverageMarcus force-pushed the metrics_count_reason branch from 17a7fe3 to 4daeb05 Compare September 15, 2023 09:39

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 15, 2023

jerop added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2023

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2023

AverageMarcus force-pushed the metrics_count_reason branch from 4daeb05 to 6d87b80 Compare September 25, 2023 07:49

khrm reviewed Oct 3, 2023

View reviewed changes

dibyom reviewed Oct 5, 2023

View reviewed changes

AverageMarcus force-pushed the metrics_count_reason branch from 6d87b80 to aaa8e41 Compare October 5, 2023 14:52

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

dibyom reviewed Oct 12, 2023

View reviewed changes

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

tekton-robot merged commit d26a45c into tektoncd:main Oct 12, 2023
2 checks passed

AverageMarcus deleted the metrics_count_reason branch October 12, 2023 17:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add ability to include reason in count metrics #7060

feat: add ability to include reason in count metrics #7060

AverageMarcus commented Aug 22, 2023 •

edited

Loading

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

AverageMarcus commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

khrm left a comment

tekton-robot commented Aug 22, 2023

khrm commented Aug 22, 2023

AverageMarcus commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

khrm left a comment

AverageMarcus commented Aug 23, 2023

AverageMarcus commented Sep 15, 2023

tekton-robot commented Sep 15, 2023

tekton-robot commented Sep 15, 2023

tekton-robot commented Sep 25, 2023

tekton-robot commented Sep 25, 2023

dibyom commented Oct 2, 2023

khrm left a comment

AverageMarcus commented Oct 3, 2023

dibyom Oct 2, 2023

AverageMarcus Oct 5, 2023

dibyom commented Oct 5, 2023

tekton-robot commented Oct 5, 2023

tekton-robot commented Oct 5, 2023

AverageMarcus commented Oct 12, 2023

dibyom commented Oct 12, 2023

tekton-robot commented Oct 12, 2023

AverageMarcus commented Oct 12, 2023

dibyom left a comment

feat: add ability to include reason in count metrics #7060

feat: add ability to include reason in count metrics #7060

Conversation

AverageMarcus commented Aug 22, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

AverageMarcus commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

khrm left a comment

Choose a reason for hiding this comment

tekton-robot commented Aug 22, 2023

khrm commented Aug 22, 2023

AverageMarcus commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

tekton-robot commented Aug 22, 2023

khrm left a comment

Choose a reason for hiding this comment

AverageMarcus commented Aug 23, 2023

AverageMarcus commented Sep 15, 2023

tekton-robot commented Sep 15, 2023

tekton-robot commented Sep 15, 2023

tekton-robot commented Sep 25, 2023

tekton-robot commented Sep 25, 2023

dibyom commented Oct 2, 2023

khrm left a comment

Choose a reason for hiding this comment

AverageMarcus commented Oct 3, 2023

dibyom Oct 2, 2023

Choose a reason for hiding this comment

AverageMarcus Oct 5, 2023

Choose a reason for hiding this comment

dibyom commented Oct 5, 2023

tekton-robot commented Oct 5, 2023

tekton-robot commented Oct 5, 2023

AverageMarcus commented Oct 12, 2023

dibyom commented Oct 12, 2023

tekton-robot commented Oct 12, 2023

AverageMarcus commented Oct 12, 2023

dibyom left a comment

Choose a reason for hiding this comment

AverageMarcus commented Aug 22, 2023 •

edited

Loading