Add buildrunname label for custom metric

[zhangtbj]

For the current custom metric, we have two labels: buildStrategy and namespace and we can configure to the controller deployment choose which label we want to collect.

At the beginning, we just have two labels: buildStrategy and namespace because the metric limitation in Sysdig system, but it is not the limitation for other monitoring system and the buildrunname label is also important so that the end user can know more detail about each buildrun status.

By default the default label is still buildStrategy, but after this PR, the end user can have the capability to choose one more label in custom metric to show more detail.

[qu1queee]

@zhangtbj sorry, I almost missed this PR; let me review tomorrow.

[zhangtbj]

Thanks!

[SaschaSchwarze0]

To have a consistent naming schema, the label would be buildrun without the name suffix.

Be aware that within our IBM Code Engine solution, we cannot use this new label, see the very last line in our internal ADR 6.

[zhangtbj]

Hi Sascha,

Yes, it is a CodeEngine or sysdig limitation, but for prometheus or other monitoring systems, it is not a limitation and we should allow end user to see the metric about buildrun name.

I didn't choose to use name because I found maybe in future, there will be multiple names in metrics, like build name or buildrun name, if we use name directly, it is hard to say which name is

[zhangtbj]

Sorry, I mis-understood the comment.

Yes, I think change the label from buildrunname to buildrun make more sense.

I will change it later. Thanks Sascha!

[qu1queee]

@zhangtbj pr looks good, and @SaschaSchwarze0 requested changes should be addressed, +1. I might be having the full context on this PR, which is allow users that might have their own sysdig instance to get metrics based on:

• namespace
• buildstrategy

but also on:

• buildruns

I´m not fully understanding the advantage of introducing "buildrun" . E.g. if you want to estimate an average on builds based on buildstrategy, then buildstrategy is a solid use case. I think same applies for the namespace. For buildruns as they are are always a single entity(e.g. one buildrun at a time), I´m struggling to see the concrete use case, can you provide that pls.

[qu1queee]

Asking for more clarification on the use case behind this new label. See my above comment.

[zhangtbj]

Yes,

I think two reasons:

• We should allow end user to filter what they would like to see, buildrun is the detail of one build, it is an option and we should open it, we didn't open it before is because sydig cannot handle too many metrics by multiple labels, but like Tekton taskrun metric, there are several label so that end user can monitor from different level
• namespace and buildstrategy label are for statistics, but if an end user wants to know which buildrun take longer time, or if there are several buildrun failures, the end user can know the buildrun names and check the buildrun failure directly.

[zhangtbj]

Change the label name from buildruname to buildrun in the second commit: 76d3882

[qu1queee]

Ok, I see the use case on seeing all buildruns per namespace in sysdig. I guess that will help. I would have squash your commits, while renaming something that appeared on this PR doesnt needs another commit, but for you to decide @zhangtbj

[qu1queee]

lgtm

[SaschaSchwarze0]

We should maybe then also add build in another PR to complete the picture.

/lgtm
/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

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 [SaschaSchwarze0]

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