Skip to content
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.

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

Document convention that Counter metrics shouldn't end with .count? #1979

Open
trask opened this issue Mar 10, 2025 · 3 comments
Open

Document convention that Counter metrics shouldn't end with .count? #1979

trask opened this issue Mar 10, 2025 · 3 comments

Comments

@trask
Copy link
Member

trask commented Mar 10, 2025

This isn't mentioned in https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/naming.md#naming-rules-for-counters-and-updowncounters, but the convention has been followed pretty closely to only use .count suffix on UpDownCounter metrics (and not to use it on Counter metrics).

I'm not thinking of reasons right now, maybe related to the ambiguity of Counters being commonly exported as both delta and cumulative, and maybe related to the Prometheus exporter converting these to _count_total(?)

There are just a few metrics that violate this convention:

  • otel.sdk.span.ended.count
  • otel.sdk.processor.span.processed.count
  • otel.sdk.exporter.span.exported.count
  • dotnet.thread_pool.work_item.count - this one is stable, can't rename it

One option for the otel.sdk.* metric names could be:

  • otel.sdk.ended_spans
  • otel.sdk.processor.processed_spans
  • otel.sdk.exporter.exported_spans

cc @JonasKunz

@JonasKunz
Copy link
Contributor

JonasKunz commented Mar 11, 2025

otel.sdk.ended_spans

Sounds good to me, but I think we should also rename otel.sdk.span.live.count -> otel.sdk.live_spans for consistency?

otel.sdk.exporter.exported_spans

Initially it was intended to have different namespaces per signal: E.g. otel.sdk.exporter.span.* otel.sdk.exporter.log.* etc. However I think it is okay to share metrics across signal types for exporters, see my at reasoning #1921.

So this change is fine with me, we might even remove the _spans suffix and introduce an attribute instead to distinguish the telemetry type.

`otel.sdk.processor.processed_spans``

Here I'm not that that happy about dropping the span in the namespace. There is no general concept of processor in the spec, there is just span-processors and log-processors, which are different things and evolve independently. This also shows with metrics: There is no MetricProcessor, just MetricReaders. Tehre's also some more reasoning given in #1921.

So I'm not opposing to removing the .count suffix, I'm just concerned about dropping span from the metric namespace turning it from "span processor" to just "processor"

@trask
Copy link
Member Author

trask commented Mar 11, 2025

I'm just concerned about dropping span from the metric namespace turning it from "span processor" to just "processor"

ah, no problem, my only goal here was to get rid of the .count suffix 👍

@JonasKunz
Copy link
Contributor

I've adjusted the names for log health metrics in #1921. Based on the outcomes of the naming discussions in that PR, I'll open up a follow-up PR to apply the same name pattern to the existing span health metrics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Need triage
Development

No branches or pull requests

2 participants