-
Notifications
You must be signed in to change notification settings - Fork 191
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
base: main
Are you sure you want to change the base?
Conversation
docs/general/naming.md
Outdated
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` |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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 ofos.linux.distro.name
orsystem.linux.distro.name
- [Real]
jvm.system.cpu.load_1m
instead ofsystem.jvm.cpu.load_1m
- [Real]
dotnet.process.cpu.count
instead ofprocess.dotnet.cpu.count
- [Real, but different pattern]
linux.memory.slab.usage
(orlinux.system.memory.slab.usage
) instead ofsystem.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
There was a problem hiding this 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!
Co-authored-by: Trask Stalnaker <[email protected]>
This is blocked on #1734 which implements this guidance for databases.
Fixes #1494, #608
Related:
azure_
andaz.
toazure.
across all conventions #1698Documents:
{system_name}.{thing}.{property}
pattern{system_name}.client|server.{metric_name}
client
orserver
Merge requirement checklist
[chore]