Add system metrics

[malafeev]

It is based on open-telemetry/opentelemetry-java#1323
Spec: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/metrics/semantic_conventions/system-metrics.md

[malafeev]

cc @carlosalberto

[iNikem]

@jkwatson do you want to review this? Metrics are your cup of tea, aren't they?

[carlosalberto]

Looks good to me! I'm just wondering where/how should we initialize it, as I'd like this to be part of the rest of the instrumentation.

[trask]

hey @malafeev! sorry for the delay, will review this by end of the week

[malafeev]

3.4MB with transitive dependencies

[carlosalberto]

An alternative to this would be to add this but:

1. Disable it by default AND
2. Make oshi a compile dependency.

This way, if users want to leverage it, they'd need to explicitly enable this instrumentation part, AND provide the Oshi dependency themselves.

[anuraaga]

I think it's a good idea to make oshi a compile dependency. How about we model this change exactly like any instrumentation, instrumentation/oshi/{library,javaagent} for example. This code goes in library, giving user a manual way of instrumenting (calling the static method) similar to our other library instrumentations, and auto can call it automatically if oshi is found on the classpath (not sure if we have a pattern for this yet but figure it's possible right @trask?). We can start with just moving this into library and add javaagent later.

For reference, https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/master/instrumentation/grpc-1.5/library/grpc-1.5-library.gradle

[anuraaga]

In which case we should split out the JVM metrics into a separate instrumentation/jvm/library too

[carlosalberto]

This code goes in library, giving user a manual way of instrumenting (calling the static method) similar to our other library instrumentations

Sounds good to me.

In which case we should split out the JVM metrics into a separate instrumentation/jvm/library too

Same.

@malafeev any opinion on this? ;)

[malafeev]

I'm fine with it. any other opinions?

[trask]

I like the idea of adding just library instrumentation in the first PR, and then work on how to integrate it into javaagent after that

[malafeev]

I moved code to instrumentation/oshi/library because both jvm and system metrics depend on oshi

[iNikem]

Why this PR is blocked from merging? We do have one approval from @carlosalberto ... @trask do you still want to review this?

[jkwatson]

@jkwatson do you want to review this? Metrics are your cup of tea, aren't they?

Don't hold this up on my account. :)

[trask]

Why this PR is blocked from merging?

@iNikem it's blocked from merging because it hasn't been approved by someone in the CODEOWNERS file

[malafeev]

@trask could you review this PR again?

[trask]

thx @malafeev! sorry for the delayed review

[trask]

thx @malafeev 👍

[trask]

@malafeev can you merge master into your branch? we updated to otel api 0.10.0-SNAPSHOT, which breaks a couple of metric APIs, just in case it affects this PR

[malafeev]

@trask synced with master

[trask]

@malafeev looks like the branch needs a ./gradlew spotlessApply

[malafeev]

@trask master needs a ./gradlew spotlessApply :)

[trask]

oh no! I think I messed up with the merge enforcement switching over from CircleCI to GHA, which probably led to that

[anuraaga]

Thanks!