Implement memory metrics #652
Conversation
github-actions
bot
requested review from
breedx-splk,
jack-berg and
kittylyst
...in/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapConfigurationHandler.java
Outdated
Show resolved
Hide resolved
.../main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/MetaspaceSummaryHandler.java
Outdated
Show resolved
Hide resolved
jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/MemoryUsageMetricTest.java
Outdated
Show resolved
Hide resolved
jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/MemoryLimitMetricTest.java
Outdated
Show resolved
Hide resolved
jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/MemoryLimitMetricTest.java
Outdated
Show resolved
Hide resolved
…ontrib into memory-handlers
roberttoyonaga
requested review from
jack-berg
and removed request for
kittylyst and
breedx-splk
| metricReader = InMemoryMetricReader.create(); | ||
| meterProvider = SdkMeterProvider.builder().registerMetricReader(metricReader).build(); | ||
| GlobalOpenTelemetry.set(OpenTelemetrySdk.builder().setMeterProvider(meterProvider).build()); | ||
| JfrMetrics.enable(meterProvider); |
There was a problem hiding this comment.
Side note: This abstract base class needs some work:
- Should be renamed to something more descriptive, maybe
AbstractJfrTest - Should not have state which is shared across all tests. Right now that's required because
JfrMetricscan't be cancelled once started. We should change that. - Should be put in a common place where it can be used by each of the different testing suites, rather than repeated in each test suite
Can do all this in a future PR.
There was a problem hiding this comment.
Yup I agree with all those points.
Right now that's required because JfrMetrics can't be cancelled once started
Maybe @BeforeEach test I can reinitialize JFR streaming and all openTelemetry variables, and @AfterEach test the JFR streaming can be ended. I think that should be enough for tests to fully clean up after themselves.
Should be put in a common place where it can be used by each of the different testing suites
I was trying to figure out a good way of doing this today. Maybe something related to test fixtures. I'll have to investigate more.
There was a problem hiding this comment.
Update:
I renamed the base test class to AbstractJfrTest. I was able to get the testFixtures working. So now the class resides in a single place, instead of being duplicated for each GC type.
...streaming/src/g1GcTest/java/io/opentelemetry/contrib/jfr/metrics/G1GcDurationMetricTest.java
Outdated
Show resolved
Hide resolved
...ming/src/g1GcTest/java/io/opentelemetry/contrib/jfr/metrics/G1MemoryCommittedMetricTest.java
Outdated
Show resolved
Hide resolved
...treaming/src/g1GcTest/java/io/opentelemetry/contrib/jfr/metrics/G1MemoryLimitMetricTest.java
Outdated
Show resolved
Hide resolved
...ing/src/parallelGcTest/java/io/opentelemetry/contrib/jfr/metrics/PsGcDurationMetricTest.java
Outdated
Show resolved
Hide resolved
| .isEqualTo(Attributes.of(ATTR_GC, "MarkSweepCompact", ATTR_ACTION, END_OF_MAJOR_GC)); | ||
| } | ||
| assertThat(pointData.getAttributes()) | ||
| .isEqualTo(Attributes.of(ATTR_GC, "PS MarkSweep", ATTR_ACTION, END_OF_MAJOR_GC)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
There's a comment below that is no longer true:
// TODO: Need a reliable way to test old and young gen GC in isolation.
There was a problem hiding this comment.
I left that comment in because, although we can reliably test different collectors independently, there still isn't any code that enforces young vs old gen collection for GCs that divide up the heap.
…for simulteneous young and old gc.
| @@ -1,11 +1,20 @@ | |||
| plugins { | |||
| id("otel.java-conventions") | |||
| id("java-library") | |||
| id("java-test-fixtures") | |||
There was a problem hiding this comment.
TIL about java-test-fixtures. Nice!
Description:
This PR updates the implementation for the metrics
process.runtime.jvm.memory.limit,process.runtime.jvm.memory.usage_after_last_gc,process.runtime.jvm.memory.committed,process.runtime.jvm.memory.init,process.runtime.jvm.memory.usageThis was a part of a larger PR here.
jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.javahas been kept the same as it was in the larger PR because it is used in all of the smaller constituent PRs. This will hopefully also avoid some conflicts as the PRs are sequentially merged. As a result, there are some constants that are defined, but not used in this PR.Some of the attribute key-value pairs outlined here are not present with the current implementation. This is because they require additional JFR data, or new events. In the future, when JFR is updated, these handlers should be revisited and updated to cover all the missing attribute key-value pairs.