TEP-0006: New (sub-)metrics

[NavidZ]

This is a barebone TEP for adding new metrics as
suggested by the TEP template. At this point it
has only the problem statement and a testing plan.
I will iterate more to add the actual (sub-)metrics
and what they measure and where should they be
measured in the code in subsequent PRs.

tektoncd/pipeline#2814

[NavidZ]

/assign afrittoli

[mrutkows]

I believe that moving to OpenTelemetry should be a goal if we are attempting to put metrics on the correct path for the future as that is the direction of OpenCensus.

[NavidZ]

I believe that moving to OpenTelemetry should be a goal if we are attempting to put metrics on the correct path for the future as that is the direction of OpenCensus.

I agree. But at the moment we use the Knative reporting libraries. We could alternatively wait for them to migrate and just rely on their migration as I mentioned in this comment. What do you think?

[mrutkows]

I agree. But at the moment we use the Knative reporting libraries. We could alternatively wait for them to migrate and just rely on their migration as I mentioned in this comment. What do you think?

I would assume that it would be the goal of Tekton to have its own metrics/reporting (exporter/observability config) code and not use Knative's in order to have the ability to move independently of Knative. If Knative were a dedicated monitoring/metrics project that intended other projects to directly reuse its metrics-related libraries, I would feel differently, but instead believe that Knative's mission is not to provide their metrics libs. as first-class, supported code for downstream projects to use and rely upon.

[NavidZ]

I would assume that it would be the goal of Tekton to have its own metrics/reporting (exporter/observability config) code and not use Knative's in order to have the ability to move independently of Knative. If Knative were a dedicated monitoring/metrics project that intended other projects to directly reuse its metrics-related libraries, I would feel differently, but instead believe that Knative's mission is not to provide their metrics libs. as first-class, supported code for downstream projects to use and rely upon.

That's a very good point. I now am more inclined to indeed have our own reporting system after your comment and decouple it from the Knative at some point. One quick problem is that OpenTelemetry APIs are in alpha (and recently some in beta). So I'm not sure it is at the stage that we accept using in this project as is. I defer this decision to you and others (cc @afrittoli @vdemeester) who have more experience and I will follow your guidance. Let me know.

[vdemeester]

I would assume that it would be the goal of Tekton to have its own metrics/reporting (exporter/observability config) code and not use Knative's in order to have the ability to move independently of Knative. If Knative were a dedicated monitoring/metrics project that intended other projects to directly reuse its metrics-related libraries, I would feel differently, but instead believe that Knative's mission is not to provide their metrics libs. as first-class, supported code for downstream projects to use and rely upon.

So I am not fully sure I agree with that. We are using knative.dev/pkg as a library (as knative component does), and it provides some metrics capabilities. From my perspective, anything that is in knative.dev/pkg is meant to be used to knative components and any other project that wants to benefit from it.

Knative's mission is not to provide their metrics libs. as first-class, supported code for downstream projects to use and rely upon.

That's a good question 🙃. I see knative.dev/pkg as exactly this : library that can be used for downstream projects 🙃 (cc @mattmoor )

[mattmoor]

cc @evankanderson has been poking at OpenCensus + OpenTelemetry stuff in the context of knative/pkg.

[evankanderson]

My suggestion would be to use the upstream opencensus libraries directly with the opencensus collector, and plan at some point to move to opentelemetry when their metrics support is production quality.

The knative/pkg metrics package as currently implemented addresses a number of problems which Tekton probably doesn't need to handle. (The one part which may be useful is the support for exporting metrics for multiple Resources from a single process. If that is useful, it may be better to upstream it.) The capabilities that make the package a poor fit are:

• There's a lot of code for dynamically changing the reporting backend at runtime. This is a lot of code compared with the alternative of restarting the binary (or the opencensus collector) if you switch from one metrics system to another.
• The pkg library only supports three backends: Stackdriver (with a bunch of Knative-specific overrides), Prometheus, and OpenCensus. The collector supports many more with a simpler config model.
• Most of the library is heavily entwined with the Knative config-observability ConfigMap, and the external interfaces have not been carefully designed.

[NavidZ]

@vdemeester @mrutkows based on what Evan said are we okay at least with the (non-)goals and other sections so far?

[NavidZ]

/assign vdemeester

[vdemeester]

based on what Evan said are we okay at least with the (non-)goals and other sections so far?

@NavidZ I think we are. Let's focus this TEP around the metrics we want to gather/report 👍

[vdemeester]

We will need to assign a number 👼

[NavidZ]

Done. I picked 0006 as that doesn't seem to exist yet in the repo.

[NavidZ]

/assign @afrittoli

[NavidZ]

ping @afrittoli

[afrittoli]

This looks good to me as a starting point.
I like the idea of describing the general intention in the 'proposed' state and add the bulk of the content in the next iteration.
/lgtm

/cc @vdemeester @imjasonh @hrishin @mrutkows

[tekton-robot]

@afrittoli: GitHub didn't allow me to request PR reviews from the following users: mrutkows.

Note that only tektoncd members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

This looks good to me as a starting point.
I like the idea of describing the general intention in the 'proposed' state and add the bulk of the content in the next iteration.
/lgtm

/cc @vdemeester @imjasonh @hrishin @mrutkows

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.

[afrittoli]

/hold
There are already two trigger TEPs which are competing for #6 - @NavidZ would you mind picking a different number and adding it to the PR title too so that no-one else takes it?

[afrittoli]

About using knative/pkg I would not say that moving off that dependency is the primary goal of this TEP, but it will probably happen as part of the technical design of the solution.

Something worth noting is that some part of the controller code are generated usingknative/pkg - and those include instrumentation with metrics about the controller queue. It would be good to have those available even if we decided to not depend on knative/pkg for Tekton own metrics.

[NavidZ]

/hold
There are already two trigger TEPs which are competing for #6 - @NavidZ would you mind picking a different number and adding it to the PR title too so that no-one else takes it?

The trigger ones took 8 and 9 not 6. Do you still want me to change this to say TEP0010?

[afrittoli]

/hold cancel

[pritidesai]

/kind tep

[pritidesai]

Please let us know the status and any blockers? @NavidZ

We are looking for examples and types of metrics, thanks

[NavidZ]

@pritidesai do we want to land this and I'll follow up with more PRs regarding the details?

[bobcatfish]

Had a brief discussion in the API working group today, @NavidZ suggested merging this with less details fleshed out as proposed, then incrementally adding more; we'd want to all be satisfied with the level of detail before moving to implmentable

In the meeting @imjasonh took an action to take a look as well

[imjasonh]

Sorry for the delay in getting to this.

This TEP as proposed lgtm, as others have said we'll definitely want to add more detail in future revisions before we get to implementable, but in general the overall idea of "we need better metrics" is pretty universally agreed upon AFAIK.

/lgtm

[afrittoli]

/approve

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli

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:

• teps/OWNERS [afrittoli]

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