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

Use consistent suffix for JVM Metrics #288

Closed
lenin-jaganathan opened this issue Aug 29, 2023 · 15 comments · Fixed by #536 or #1265
Closed

Use consistent suffix for JVM Metrics #288

lenin-jaganathan opened this issue Aug 29, 2023 · 15 comments · Fixed by #536 or #1265
Assignees

Comments

@lenin-jaganathan
Copy link
Contributor

While going through

- [JVM Memory](#jvm-memory)
* [Metric: `jvm.memory.usage`](#metric-jvmmemoryusage)
* [Metric: `jvm.memory.committed`](#metric-jvmmemorycommitted)
* [Metric: `jvm.memory.limit`](#metric-jvmmemorylimit)
* [Metric: `jvm.memory.usage_after_last_gc`](#metric-jvmmemoryusage_after_last_gc)
- [JVM Garbage Collection](#jvm-garbage-collection)
* [Metric: `jvm.gc.duration`](#metric-jvmgcduration)
- [JVM Threads](#jvm-threads)
* [Metric: `jvm.thread.count`](#metric-jvmthreadcount)
- [JVM Classes](#jvm-classes)
* [Metric: `jvm.class.loaded`](#metric-jvmclassloaded)
* [Metric: `jvm.class.unloaded`](#metric-jvmclassunloaded)
* [Metric: `jvm.class.count`](#metric-jvmclasscount)
- [JVM CPU](#jvm-cpu)
* [Metric: `jvm.cpu.time`](#metric-jvmcputime)
* [Metric: `jvm.cpu.count`](#metric-jvmcpucount)
* [Metric: `jvm.cpu.recent_utilization`](#metric-jvmcpurecent_utilization)
- [Very experimental](#very-experimental)
* [Metric: `jvm.memory.init`](#metric-jvmmemoryinit)
* [Metric: `jvm.system.cpu.utilization`](#metric-jvmsystemcpuutilization)
* [Metric: `jvm.system.cpu.load_1m`](#metric-jvmsystemcpuload_1m)
* [Metric: `jvm.buffer.memory.usage`](#metric-jvmbuffermemoryusage)
* [Metric: `jvm.buffer.memory.limit`](#metric-jvmbuffermemorylimit)
* [Metric: `jvm.buffer.count`](#metric-jvmbuffercount)
, it seems like the suffices for some of the metrics seem to be inconsistent with one another and are also not so straight forward.

I propose replacing the suffix "usage" (noun) in the following metrics with "used" (verb),

  • jvm.memory.usage
  • jvm.memory.usage_after_last_gc
  • jvm.buffer.memory.usage

All these represent the amount being used. In addition, the description already uses the word "used" to describe these metrics and the JMX methods also use the word "used". Also, the sibling metrics use the verb form. E.g: "jvm.memory.committed"

@trask
Copy link
Member

trask commented Aug 31, 2023

hi @lenin-jaganathan!

.usage is a common suffix used in otel semconv, check out

so we would probably need to have that discussion (usage -> used) more broadly than only JVM metrics

I personally think either nouns or adjectives (when it describes the namespace which is a noun) seem like good metric names, e.g.

  • jvm.memory.committed - measure "committed memory"
  • jvm.class.loaded - measures "loaded classes"

@trask
Copy link
Member

trask commented Oct 19, 2023

I realized, I don't think we would want to rename usage -> used generally, e.g. system.memory.usage has dimensions:

  • total
  • used <--
  • free
  • shared
  • buffers
  • cached

@lenin-jaganathan wdyt? do you want to keep this issue open? thx

@lenin-jaganathan
Copy link
Contributor Author

I would still like this to be addressed. There is a subtle difference between system.memory.usage and jvm.memory.usage.

As you mentioned, the first metric has multiple states - used, free, cached, etc. But the JVM metric is all about using part of it and we have separate metrics for committed, limit, etc. But even in the system metrics use-case, total as a dimension is always a questionable choice since we would expect adding up all the dimensions should be total.

@trask
Copy link
Member

trask commented Oct 24, 2023

But even in the system metrics use-case, total as a dimension is always a questionable choice since we would expect adding up all the dimensions should be total.

ya, this is being addressed in #409:

Removes total from list of well-known values for system.memory.state

can you re-summarize your reasoning for renaming jvm.memory.usage to jvm.memory.used? I think that will help focus this issue, since we've discussed a few different things above, thx

@trask
Copy link
Member

trask commented Nov 9, 2023

ah, I think I understand now

jvm.memory.usage isn't really a parallel to system.memory.usage, since jvm.memory.usage is only one dimension of "usage", namely the used dimension

@trask
Copy link
Member

trask commented Nov 9, 2023

same would apply to

@trask
Copy link
Member

trask commented Nov 9, 2023

looking at other *.usage metrics

  • db.client.connections.usage / db.client.connections.limit
    • has state dimension (used / idle)
  • hw.gpu.memory.usage / hw.gpu.memory.limit
    • no state dimension <---
  • hw.logical_disk.usage / hw.logical_disk.utilization / hw.logical_disk.limit
    • has state dimension (used / free)
  • process.memory.usage
    • no state dimension <---
  • system.memory.usage / system.memory.utilization
    • has state dimension (used / free / shared / buffers / cached)
  • system.paging.usage / system.paging.utilization
    • has state dimension (used / free)
  • system.filesystem.usage / system.filesystem.utilization
    • has state dimension (used / free / reserved)

@trask
Copy link
Member

trask commented Nov 13, 2023

There seem to be two options for jvm.memory.usage:

Option 1

Rename jvm.memory.usage to jvm.memory.used

Option 2

Keep the name, but add jvm.memory.state dimension:

  • used
  • free (which would be committed limit minus used)

and remove the jvm.memory.committed metric

@trask
Copy link
Member

trask commented Nov 15, 2023

I don't think I got Option 2 right above.

I think free should be jvm.memory.limit minus used, see https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#instrument-naming:

usage - an instrument that measures an amount used out of a known total (limit) amount should be called entity.usage. For example, system.memory.usage with attribute state = used | cached | free | ... for the amount of memory in a each state. Where appropriate, the sum of usage over all attribute values SHOULD be equal to the limit.

In which case we'd probably leave jvm.memory.committed metric as-is.

@trask
Copy link
Member

trask commented Nov 17, 2023

Discussed in Java SIG meeting today:

@lenin-jaganathan
Copy link
Contributor Author

@trask / @joaopgrassi Do you guys think jvm.buffer.memory.usage was missed as part of this change?

The metric is still experimental which should allow the re-naming.

@trask
Copy link
Member

trask commented Jul 19, 2024

yes I think it was missed, re-opening, if you'd like to send a PR that would be great!

@lenin-jaganathan
Copy link
Contributor Author

Submitted #1265. Will follow it up with one for Java instrumentation once the sem-conv is merged.

@joaopgrassi
Copy link
Member

What about the other metrics mentioned above:

jvm.memory.usage_after_last_gc
jvm.buffer.memory.usage

Are those also suppose to change? Just want to make sure the issue isn't closed again once #1265 is merged.

@lenin-jaganathan
Copy link
Contributor Author

@joaopgrassi The first 2 metrics memory.usage/memory.usage_after_last_gc where already changed before JVM sem conv stabilisation. Only, jvm.buffer.usage was left behind.

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