Update instrumentation.md

[logicalhan]

Add a snippet about preferring labels to embedding labels in metric names.

Which issue(s) this PR fixes:

Fixes #

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logicalhan

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:

• contributors/devel/sig-instrumentation/OWNERS [logicalhan]

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

[logicalhan]

/assign @dashpole

[dashpole]

Note that this takes the opposite stance taken by OpenMetrics on this question: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-names-versus-labels

Experience has shown that downstream ingestors find it easier to work with separate total and failure MetricFamiles rather than using {result="success"} and {result="failure"} Labels within one MetricFamily. Also it is usually better to expose separate read & write and send & receive MetricFamiles as full duplex systems are common and downstream ingestors are more likely to care about those values separately than in aggregate.

[dashpole]

I do still prefer what is proposed here. @rexagod @dgrisonnet are you OK with this convention?

[logicalhan]

Note that this takes the opposite stance taken by OpenMetrics on this question: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-names-versus-labels

Experience has shown that downstream ingestors find it easier to work with separate total and failure MetricFamiles rather than using {result="success"} and {result="failure"} Labels within one MetricFamily. Also it is usually better to expose separate read & write and send & receive MetricFamiles as full duplex systems are common and downstream ingestors are more likely to care about those values separately than in aggregate.

Yeah I don't buy it. Ratios are easier with a single metric, because you can do metric["failure or success]" / metric rather than metric1 / sum(metric1 and metric2).

[dgrisonnet]

/assign

[dashpole]

Related blog: https://www.robustperception.io/existential-issues-with-metrics/

[dashpole]

Found where the prometheus docs recommend separate failure and total: https://prometheus.io/docs/practices/instrumentation/#failures

[ricardoapl]

Ratios are easier with a single metric, because you can do metric["failure or success]" / metric rather than metric1 / sum(metric1 and metric2).

It does seem easier to do ratios. However, when evaluating an expression such as metric{result="failure"} / metric in Prometheus, isn't the result going to be a vector with all values equal to 1 because of how Prometheus Vector matching rules work? We could probably rewrite the expression to avoid that, but then is it still easier? This is not what I would expect, but I'm probably misunderstanding something here.

Experience has shown that downstream ingestors find it easier to work with separate total and failure MetricFamiles rather than using {result="success"} and {result="failure"} Labels within one MetricFamily. Also it is usually better to expose separate read & write and send & receive MetricFamiles as full duplex systems are common and downstream ingestors are more likely to care about those values separately than in aggregate.

OpenMetrics authors don't provide much details or examples, which makes it hard (at least for me) to reason/argue. We could try reaching out to them for more information. What do you think?

[ricardoapl]

I think Errors, Successes, Totals: Which Metrics Should I Expose to Prometheus? may also be useful for this discussion, in particular Exposing a single metric with a label

[dashpole]

@juliusv

[juliusv]

Yeah, TL;DR of that article: If you have just have a binary success/failure situation that you want to track, use two different metrics, one for the failures, one for the total (not the successes). If you have something more complicated like many different HTTP response status codes, use a single metric with a label on it instead.

[k8s-ci-robot]

PR needs rebase.

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.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle rotten
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Reopen this PR with /reopen
• Mark this PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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-sigs/prow repository.