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

Add system-specific naming guidance #1708

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

lmolkova
Copy link
Contributor

@lmolkova lmolkova commented Dec 21, 2024

This is blocked on #1734 which implements this guidance for databases.

Fixes #1494, #608

Related:

Documents:

  • Systems-specific attribute names follow {system_name}.{thing}.{property} pattern
  • Systems-specific metric names follow {system_name}.client|server.{metric_name}
  • Metrics for remote call operations should include some indication if they are client or server
  • System names should not be ambiguous and match the project name or, in most cases, are prefixed with company/org name

Merge requirement checklist

@lmolkova lmolkova requested review from a team as code owners December 21, 2024 00:53
the system name should be included in the instrument name using the pattern:
`{domain}.{client|server}.{system}.*.{property}` pattern.

For example, `db.client.cosmosdb.operation.request_charge`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the difference with signalr.server.connection.duration below makes me think if we should simplify it and do cosmosdb.client.operation.request_charge. What's the benefit of having db in front of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

summoning @open-telemetry/semconv-system-approvers

It's the same question as for system.linux vs linux metrics in #1403 or #1618

Copy link
Member

@ChrsMark ChrsMark Jan 28, 2025

Choose a reason for hiding this comment

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

The difference with process.linux.cgroup is that the name describes effectively a cgroup that belongs to a linux process. While cosmosdb.client.operation.request_charge can stand on its own without the need of db in front.

Maybe we can try and standardize this difference as part of this doc?

Note: if we had linux.cgroup it won't be clear if we refer to a linux system/host or a linux process

Copy link
Member

Choose a reason for hiding this comment

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

what about linux.process.cgroup?

Copy link
Contributor Author

@lmolkova lmolkova Jan 29, 2025

Choose a reason for hiding this comment

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

Examples:

  • [Imaginary] linux.distro.name (linux.distro.version, linux.kernel.version) instead of os.linux.distro.name or system.linux.distro.name
  • [Real] jvm.system.cpu.load_1m instead of system.jvm.cpu.load_1m
  • [Real] dotnet.process.cpu.count instead of process.dotnet.cpu.count
  • [Real, but different pattern] linux.memory.slab.usage (or linux.system.memory.slab.usage) instead of system.linux.memory.slab.usage

Justification for linux.process.cgroup - cgroup is a property of a process, not linux. E.g. we have process.pid. What if there was a linux-specific process id, would we do linux.process.pid or process.linux.pid ? I think former is more friendly.

And since linux-specific metrics fall into 'specialist case' bucket, they don't necessarily need to be discoverable when you type process and let autocomplete guide you

docs/general/naming.md Outdated Show resolved Hide resolved
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

really appreciate all of the guidance documentation!

docs/general/naming.md Outdated Show resolved Hide resolved
docs/general/naming.md Outdated Show resolved Hide resolved
docs/general/naming.md Outdated Show resolved Hide resolved
docs/general/naming.md Outdated Show resolved Hide resolved
docs/general/naming.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added the enhancement New feature or request label Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Needs More Approval
Development

Successfully merging this pull request may close these issues.

Document *.client.{system}.* ordering in the metric naming
4 participants