add file descriptor metrics #11876
add file descriptor metrics #11876
Conversation
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
...va/io/opentelemetry/instrumentation/runtimemetrics/java8/internal/FileDescriptorMethods.java
Outdated
Show resolved
Hide resolved
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
...ntelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptorTest.java
Outdated
Show resolved
Hide resolved
...ntelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptorTest.java
Outdated
Show resolved
Hide resolved
|
@xiangtianyu are you able to still take a look at this? Thanks! |
Nobody response my issue open-telemetry/semantic-conventions#1275 . Is there any update about the metrics specification? |
hi @xiangtianyu! that's ok, just add a comment in the code pointing to that semantic convention issue, and we can still merge the PR because it's under "experimental" metrics anyways |
|
I've add comments, please take a look @breedx-splk @trask @laurit |
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
| if (openFileDescriptorCount != null) { | ||
| observables.add( | ||
| meter | ||
| .upDownCounterBuilder("process.open_file_descriptor.count") |
There was a problem hiding this comment.
Since this name isn't exactly the same as process.open_file_descriptors that is used by the host metric receiver we might as well go with a different name. I think it would be best to discuss these names and descriptions at the SIG meeting.
There was a problem hiding this comment.
Ok, how can i propose this as a topic?
There was a problem hiding this comment.
based on https://github.com/open-telemetry/semantic-conventions/pull/1618/files
as a working group we believe it is important that
processnamespace and runtime namespace metrics remain separate, becauseprocessmetrics are meant to represent an OS-level process as the instrumentation source, whereas runtime metrics represent the language runtime as the instrumentation source.
I'd suggest using jvm.open_file_descriptor.count
There was a problem hiding this comment.
@trask do you also have suggestions for the metric descriptions and unit. Currently the unit is {count}.
There was a problem hiding this comment.
good catch: open-telemetry/semantic-conventions#1662
There was a problem hiding this comment.
for description, I think we can use
Number of file descriptors in use by the process.
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
| if (openFileDescriptorCount != null) { | ||
| observables.add( | ||
| meter | ||
| .upDownCounterBuilder("process.open_file_descriptor.count") |
There was a problem hiding this comment.
based on https://github.com/open-telemetry/semantic-conventions/pull/1618/files
as a working group we believe it is important that
processnamespace and runtime namespace metrics remain separate, becauseprocessmetrics are meant to represent an OS-level process as the instrumentation source, whereas runtime metrics represent the language runtime as the instrumentation source.
I'd suggest using jvm.open_file_descriptor.count
.../opentelemetry/instrumentation/runtimemetrics/java8/internal/ExperimentalFileDescriptor.java
Outdated
Show resolved
Hide resolved
related to #9340 Add file descriptor metrics to agent (support openj9)