Add JFR streaming metrics gatherer #7886
Conversation
|
@roberttoyonaga can you split out the |
Yup I can do that |
roberttoyonaga
changed the title
runtime-telemetry-jfr and runtime-telemetry-jmx
| public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) { | ||
| ConfigProperties config = autoConfiguredSdk.getConfig(); | ||
|
|
||
| boolean defaultEnabled = config.getBoolean("otel.instrumentation.common.default-enabled", true); |
There was a problem hiding this comment.
Does this say that jfr instrumentation is enabled by default?
There was a problem hiding this comment.
Yes, I thought that might be a good choice because its similar to the JMX gatherer, which is also collecting metrics. And by default we are only collecting metrics not already covered by JMX.
There was a problem hiding this comment.
And by default we are only collecting metrics not already covered by JMX.
True.. although the metrics it produces aren't part of the semantic conventions (yet). Wondering if that influences our decision.
Generally, I think they contain useful data, but need just a bit of adjustment to match the metric semantic conventions guidelines. I.e. they are likely to change before being added.
There was a problem hiding this comment.
Hmm I see, yeah I think maybe its better to disable them by default. Then once its decided how the semantic conventions can be updated, they can be enabled by default.
There was a problem hiding this comment.
Also discussed at the March 2 2023 SIG. Decided that the new metrics should not be enabled by default until the semantic conventions can be updated.
.../io/opentelemetry/instrumentation/javaagent/runtimetelemetryjfr/JfrRuntimeMetricsTest.groovy
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| otelJava { | ||
| minJavaVersionSupported.set(JavaVersion.VERSION_17) |
There was a problem hiding this comment.
Curious how this works with the agent. Do we publish different versions of the agent, or do we package the jfr stuff up in the java 8 agent and only allow it to be activated if we detect we're running on java 17+?
There was a problem hiding this comment.
I'm not 100% clear on this either. I tried building with 17, and running with 11 and things run normally, just without the JFR streaming.
There was a problem hiding this comment.
Discussed in the March 2 2023 SIG . Confirmed nothing must be done in order to prevent JDK17 features from breaking things.
…ed. Remove test fixtures and put GC tests in test dir
...library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/JfrTelemetry.java
Show resolved
Hide resolved
roberttoyonaga
requested review from
jack-berg
and removed request for
theletterf
...library/src/test/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/GenerateDocs.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/JfrTelemetryBuilder.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/DurationUtil.java
Outdated
Show resolved
Hide resolved
...java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/RecordedEventHandler.java
Outdated
Show resolved
Hide resolved
...java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/RecordedEventHandler.java
Outdated
Show resolved
Hide resolved
…/opentelemetry/instrumentation/runtimetelemetryjfr/GenerateDocs.java Co-authored-by: Mateusz Rzeszutek <[email protected]>
…/opentelemetry/instrumentation/runtimetelemetryjfr/JfrTelemetryBuilder.java Co-authored-by: Mateusz Rzeszutek <[email protected]>
…/opentelemetry/instrumentation/runtimetelemetryjfr/internal/DurationUtil.java Co-authored-by: Mateusz Rzeszutek <[email protected]>
…aga/opentelemetry-java-instrumentation into jfr-runtime-metrics-gatherer
Summary of changes:
testfolder and set up tasks that specify different GCs based on the type of GC test.Usage
Include
-Dotel.instrumentation.runtime-telemetry-jfr.enabled=trueto enable metrics not already covered by JMX.Include
-Dotel.instrumentation.runtime-telemetry-jfr.enable-all=trueto enable all JFR metrics gathering.Notes
The diff is quite massive, but almost all of it is a direct copy-paste.
A follow up PR will be made to rename
runtime-metricstoruntime-telemetry-jmxI'm not sure why the test
Agent loads in when separate jvm is launchedfails with this output. Note, this only happens when JFR is activated in the Java agent by default (I've currently disabled it which means the test is currently passing. See comments below).