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

Change process.open_file_descriptors #1798

Open
braydonk opened this issue Jan 23, 2025 · 3 comments
Open

Change process.open_file_descriptors #1798

braydonk opened this issue Jan 23, 2025 · 3 comments

Comments

@braydonk
Copy link
Contributor

Area(s)

area:process

What's missing?

Note: first discussed in #1797

process.open_file_descriptors has 2 problems:

  1. The open_ is arguably redundant, since a "file descriptor" is itself a representation of an open file (i.e. there's no such thing as counting closed_file_descriptors)
  2. It can be confusing to Windows users, since on Windows you'd be counting Handles, a distinct windows exclusive concept, rather than file_descriptors.

Describe the solution you'd like

I suggest renaming the metric to process.file_descriptors, or process.unix.file_descriptors. At first glance I like unix being in the name, but this would be the first time we use that as a platform name so we'll need to think a bit about that before making that jump.

There will also be a metric called process.windows.handles in the future. Hopefully the presence of this metric will alleviate the platform confusion.

@braydonk
Copy link
Contributor Author

Marking as GA Blocker since open_file_descriptors is an existing metric in use by instrumentation today.

@lmolkova
Copy link
Contributor

lmolkova commented Jan 29, 2025

nit, since it's an up-down counter we should avoid pluralization

#### Use `count` Instead of Pluralization for UpDownCounters
If the value being recorded represents the count of concepts signified
by the namespace then the metric should be named `count` (within its 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`.

So probably process.file_descriptor.count?

I'm a Windows user, and my subjective and not very-well-though-out reaction is to use the same metric if there are no substantial differences except name.

I also wonder if we could be more approachable and just call it process.open_file.count

@trask
Copy link
Member

trask commented Jan 29, 2025

process.open_file.count

I think you can have multiple open file descriptors for the same file, so could be misleading?

process.open_file_handle.count doesn't seem like the worst idea to me

from https://en.wikipedia.org/wiki/File_descriptor (my emphasis):

In Unix and Unix-like computer operating systems, a file descriptor is a process-unique identifier (handle) for a file

I think I'd also be ok with keeping them as separate metrics in order to stay closer to the domain-specific terminology, e.g.

  • unix.process.open_file_descriptor.count
  • windows.process.open_file_handle.count

(trying to follow proposed naming guidance in #1708 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants