Skip to content

[chore] Clarify .count for updowncounters guidance #2317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[chore] Clarify .count for updowncounters guidance #2317

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions docs/general/naming.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ aliases: [attribute-naming]
- [Metrics](#metrics)
- [Naming rules for counters and UpDownCounters](#naming-rules-for-counters-and-updowncounters)
- [Pluralization](#pluralization)
- [Use `count` instead of pluralization for UpDownCounters](#use-count-instead-of-pluralization-for-updowncounters)
- [Use `count` instead of pluralization for UpDownCounters (when a namespace exists)](#use-count-instead-of-pluralization-for-updowncounters-when-a-namespace-exists)
- [Do not use `total`](#do-not-use-total)
- [Instrument naming](#instrument-naming)
- [Client and server metrics](#client-and-server-metrics)
Expand Down Expand Up @@ -285,15 +285,19 @@ should not be pluralized, even if many data points are recorded.
* `system.paging.faults`, `system.disk.operations`, and `system.network.packets`
should be pluralized, even if only a single data point is recorded.

#### Use `count` instead of pluralization for UpDownCounters
#### Use `count` instead of pluralization for UpDownCounters (when a namespace exists)

If the value being recorded represents the count of concepts signified
by the namespace then the metric should be named `count` (within its namespace).
by the namespace then the metric SHOULD be named `count` within that namespace.

For example if we have a namespace `system.process` which contains all metrics related
to the processes then to represent the count of the processes we can have a metric named
`system.process.count`.

Using `count` is not required for UpDownCounters when there is no meaningful namespace to preserve.
For instance, `k8s.deployment.desired_pods` is acceptable, as there is no need to use singular
`k8s.deployment.desired_pod.*` as a namespace.
Comment on lines +297 to +299
Copy link
Contributor

@lmolkova lmolkova Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, we've been actively aligning updowncounters to follow k8s.deployment.desired_pod.count pattern, why would we change direction now? it'd be even more confusing than today

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from the discussion at #1800 (comment).

If we still want to enforce .count to all updowncounters then we should change the guidance accordingly to make this clear. However, I doubt that's sth we want to do now if what @trask shared at #1800 (comment) was the original motivation.

In any case, whatever we decide on we need to fix the guidance accordingly to avoid confusion.

ps: I won't be able to join the Mon SIG meeting but please share any decision here so I can update the PR accordingly :)

Copy link
Contributor

@lmolkova lmolkova Jun 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be happy to completely drop this part of guidance 2 years ago, but we've been actively pushing everyone to follow it and implemented it in some stable conventions.

Changing or removing it now would mean some chaos and more inconsistencies to come. I'll try to prepare a list of metrics so we can evaluate how many irreversible decisions we've made so far.

To some extent it's a matter of taste and not a technical/rational decision to call something foo.bar.active_requests or foo.bar.active_request.count - it's like tab vs space - make decision once, add lint, and everyone should follow the repo policy. Having mixture is the worst situation to be in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stable plural metrics violating the current no-pluralization guidance (8, self-contained within .NET):

  • aspnetcore.rate_limiting.active_request_leases
  • aspnetcore.rate_limiting.queued_requests
  • kestrel.active_connections
  • kestrel.active_tls_handshakes
  • kestrel.queued_connections
  • kestrel.queued_requests
  • kestrel.upgraded_connections
  • signalr.server.active_connections

development metrics violating the current guidance (29, 24 of them as k8s):

  • db.client.connection.pending_requests
  • http.client.active_requests
  • http.client.open_connections
  • http.server.active_requests
  • k8s.cronjob.active_jobs
  • k8s.daemonset.current_scheduled_nodes
  • k8s.daemonset.desired_scheduled_nodes
  • k8s.daemonset.misscheduled_nodes
  • k8s.daemonset.ready_nodes
  • k8s.deployment.available_pods
  • k8s.deployment.desired_pods
  • k8s.hpa.current_pods
  • k8s.hpa.desired_pods
  • k8s.hpa.max_pods
  • k8s.hpa.min_pods
  • k8s.job.active_pods
  • k8s.job.desired_successful_pods
  • k8s.job.failed_pods
  • k8s.job.max_parallel_pods
  • k8s.job.successful_pods
  • k8s.replicaset.available_pods
  • k8s.replicaset.desired_pods
  • k8s.replicationcontroller.available_pods
  • k8s.replicationcontroller.desired_pods
  • k8s.statefulset.current_pods
  • k8s.statefulset.desired_pods
  • k8s.statefulset.ready_pods
  • k8s.statefulset.updated_pods
  • system.network.connections

metric following current guidance:

  • 55 development
  • 16 stable

I'd prefer the outcome that's is

  1. minimally disruptive
  2. does not make things more confusing

I have two ideas:

  1. Just say to avoid pluralization (just because it's a common practice).

    - Use count instead of pluralization for UpDownCounters
+ Do not pluralize UpDownCounter names

  2. Just remove this part from the guidance. If pluralization is sometimes ok and sometimes not, just pick whatever name works, we don't need guidance.


#### Do not use `total`

UpDownCounters SHOULD NOT use `_total` because then they will look like
Expand Down