From 17651ce30f1c7c27fec83c722bb2d61744d63459 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Mon, 21 Nov 2022 11:26:08 -0500 Subject: [PATCH 1/6] update handlers --- .../jfr/metrics/internal/Constants.java | 19 +- .../internal/cpu/OverallCPULoadHandler.java | 54 +++--- .../internal/memory/G1HeapSummaryHandler.java | 104 ++++++----- .../internal/memory/GCHeapSummaryHandler.java | 117 ++++++------ .../memory/ParallelHeapSummaryHandler.java | 166 ++++++++++++------ .../{JfrCPUTest.java => JfrCPULockTest.java} | 9 +- 6 files changed, 264 insertions(+), 205 deletions(-) rename jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/{JfrCPUTest.java => JfrCPULockTest.java} (66%) diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java index 00da5621e..14fccccef 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java @@ -12,7 +12,7 @@ private Constants() {} public static final String ONE = "1"; public static final String HERTZ = "Hz"; - public static final String BYTES = "B"; + public static final String BYTES = "By"; public static final String MILLISECONDS = "ms"; public static final String PERCENTAGE = "%age"; public static final String USER = "user"; @@ -27,15 +27,26 @@ private Constants() {} public static final String REGION_COUNT = "region.count"; public static final String COMMITTED = "committed"; public static final String RESERVED = "reserved"; + public static final String USED = "used"; + public static final String COMMITTED_SIZE = "committedSize"; + public static final String DAEMON = "daemon"; + public static final String HEAP = "heap"; public static final String METRIC_NAME_NETWORK_BYTES = "process.runtime.jvm.network.io"; public static final String METRIC_DESCRIPTION_NETWORK_BYTES = "Network read/write bytes"; public static final String METRIC_NAME_NETWORK_DURATION = "process.runtime.jvm.network.time"; public static final String METRIC_DESCRIPTION_NETWORK_DURATION = "Network read/write duration"; + public static final String METRIC_NAME_COMMITTED = "process.runtime.jvm.memory.committed"; + public static final String METRIC_DESCRIPTION_COMMITTED = "Measure of memory committed"; public static final String NETWORK_MODE_READ = "read"; public static final String NETWORK_MODE_WRITE = "write"; - + public static final String METRIC_NAME_MEMORY = "process.runtime.jvm.memory.usage"; + public static final String METRIC_NAME_MEMORY_AFTER = + "process.runtime.jvm.memory.usage_after_last_gc"; + public static final String METRIC_DESCRIPTION_MEMORY = "Measure of memory used"; + public static final String METRIC_DESCRIPTION_MEMORY_AFTER = + "Measure of memory used, as measured after the most recent garbage collection event on this pool"; public static final String METRIC_NAME_MEMORY_ALLOCATION = "process.runtime.jvm.memory.allocation"; public static final String METRIC_DESCRIPTION_MEMORY_ALLOCATION = "Allocation"; @@ -43,8 +54,10 @@ private Constants() {} public static final AttributeKey ATTR_THREAD_NAME = AttributeKey.stringKey("thread.name"); public static final AttributeKey ATTR_ARENA_NAME = AttributeKey.stringKey("arena"); public static final AttributeKey ATTR_NETWORK_MODE = AttributeKey.stringKey("mode"); - public static final AttributeKey ATTR_USAGE = AttributeKey.stringKey("usage.type"); + public static final AttributeKey ATTR_TYPE = AttributeKey.stringKey("type"); + public static final AttributeKey ATTR_POOL = AttributeKey.stringKey("pool"); public static final AttributeKey ATTR_DAEMON = AttributeKey.booleanKey(DAEMON); public static final String UNIT_CLASSES = "{classes}"; public static final String UNIT_THREADS = "{threads}"; + public static final String UNIT_UTILIZATION = "1"; } diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java index 7ed5a2c12..0973283cf 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java @@ -5,15 +5,10 @@ package io.opentelemetry.contrib.jfr.metrics.internal.cpu; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_USAGE; import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.MACHINE; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.PERCENTAGE; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.SYSTEM; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.USER; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.UNIT_UTILIZATION; import static io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler.defaultMeter; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler; import java.time.Duration; @@ -21,18 +16,18 @@ import jdk.jfr.consumer.RecordedEvent; public final class OverallCPULoadHandler implements RecordedEventHandler { - private static final String METRIC_NAME = "process.runtime.jvm.cpu.used"; - private static final String METRIC_DESCRIPTION = "CPU Utilization"; + private static final String METRIC_NAME_PROCESS = "process.runtime.jvm.cpu.utilization"; + private static final String METRIC_NAME_MACHINE = "process.runtime.jvm.system.cpu.utilization"; + private static final String METRIC_DESCRIPTION_PROCESS = "Recent CPU utilization for the process"; + private static final String METRIC_DESCRIPTION_MACHINE = + "Recent CPU utilization for the whole system"; + private static final String EVENT_NAME = "jdk.CPULoad"; private static final String JVM_USER = "jvmUser"; private static final String JVM_SYSTEM = "jvmSystem"; private static final String MACHINE_TOTAL = "machineTotal"; - - private static final Attributes ATTR_USER = Attributes.of(ATTR_USAGE, USER); - private static final Attributes ATTR_SYSTEM = Attributes.of(ATTR_USAGE, SYSTEM); - private static final Attributes ATTR_MACHINE = Attributes.of(ATTR_USAGE, MACHINE); - - private DoubleHistogram histogram; + private volatile double process = 0; + private volatile double machine = 0; public OverallCPULoadHandler() { initializeMeter(defaultMeter()); @@ -40,24 +35,31 @@ public OverallCPULoadHandler() { @Override public void initializeMeter(Meter meter) { - histogram = - meter - .histogramBuilder(METRIC_NAME) - .setDescription(METRIC_DESCRIPTION) - .setUnit(PERCENTAGE) - .build(); + meter + .gaugeBuilder(METRIC_NAME_PROCESS) + .setDescription(METRIC_DESCRIPTION_PROCESS) + .setUnit(UNIT_UTILIZATION) + .buildWithCallback( + measurement -> { + measurement.record(process); + }); + meter + .gaugeBuilder(METRIC_NAME_MACHINE) + .setDescription(METRIC_DESCRIPTION_MACHINE) + .setUnit(UNIT_UTILIZATION) + .buildWithCallback( + measurement -> { + measurement.record(machine); + }); } @Override public void accept(RecordedEvent ev) { - if (ev.hasField(JVM_USER)) { - histogram.record(ev.getDouble(JVM_USER), ATTR_USER); - } - if (ev.hasField(JVM_SYSTEM)) { - histogram.record(ev.getDouble(JVM_SYSTEM), ATTR_SYSTEM); + if (ev.hasField(JVM_USER) && ev.hasField(JVM_SYSTEM)) { + process = ev.getDouble(JVM_USER) + ev.getDouble(JVM_SYSTEM); } if (ev.hasField(MACHINE_TOTAL)) { - histogram.record(ev.getDouble(MACHINE_TOTAL), ATTR_MACHINE); + machine = ev.getDouble(MACHINE); } } diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java index 0c96ad9db..6328239f8 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java @@ -5,21 +5,19 @@ package io.opentelemetry.contrib.jfr.metrics.internal.memory; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_USAGE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_POOL; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_TYPE; import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.BYTES; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.EDEN_SIZE; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.EDEN_SIZE_DELTA; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.EDEN_USED; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.REGION_COUNT; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.SURVIVOR_SIZE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.HEAP; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY_AFTER; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY_AFTER; import static io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler.defaultMeter; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler; -import java.util.HashMap; -import java.util.Map; import java.util.logging.Logger; import jdk.jfr.consumer.RecordedEvent; @@ -30,24 +28,24 @@ public final class G1HeapSummaryHandler implements RecordedEventHandler { private static final Logger logger = Logger.getLogger(G1HeapSummaryHandler.class.getName()); - private static final String METRIC_NAME_MEMORY = "process.runtime.jvm.memory.used"; - private static final String METRIC_DESCRIPTION_MEMORY = "Heap utilization"; private static final String EVENT_NAME = "jdk.G1HeapSummary"; private static final String BEFORE = "Before GC"; private static final String AFTER = "After GC"; private static final String GC_ID = "gcId"; + private static final String EDEN_USED_SIZE = "edenUsedSize"; + private static final String SURVIVOR_USED_SIZE = "survivorUsedSize"; private static final String WHEN = "when"; - private static final Attributes ATTR_MEMORY_EDEN_USED = Attributes.of(ATTR_USAGE, EDEN_USED); - private static final Attributes ATTR_MEMORY_EDEN_SIZE = Attributes.of(ATTR_USAGE, EDEN_SIZE); - private static final Attributes ATTR_MEMORY_EDEN_SIZE_DELTA = - Attributes.of(ATTR_USAGE, EDEN_SIZE_DELTA); - private static final Attributes ATTR_MEMORY_SURVIVOR_SIZE = - Attributes.of(ATTR_USAGE, SURVIVOR_SIZE); - private static final Attributes ATTR_MEMORY_REGIONS = Attributes.of(ATTR_USAGE, REGION_COUNT); + private static final Attributes ATTR_MEMORY_EDEN_USED = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "G1 Eden Space"); + private static final Attributes ATTR_MEMORY_SURVIVOR_USED = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "G1 Survivor Space"); + // private static final Attributes ATTR_MEMORY_OLD_USED = + // Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "G1 Old Gen"); // TODO needs jdk JFR support - private final Map awaitingPairs = new HashMap<>(); - - private LongHistogram memoryHistogram; + private volatile long usageEden = 0; + private volatile long usageEdenAfter = 0; + private volatile long usageSurvivor = 0; + private volatile long usageSurvivorAfter = 0; public G1HeapSummaryHandler() { initializeMeter(defaultMeter()); @@ -55,13 +53,24 @@ public G1HeapSummaryHandler() { @Override public void initializeMeter(Meter meter) { - memoryHistogram = - meter - .histogramBuilder(METRIC_NAME_MEMORY) - .setDescription(METRIC_DESCRIPTION_MEMORY) - .setUnit(BYTES) - .ofLongs() - .build(); + meter + .upDownCounterBuilder(METRIC_NAME_MEMORY) + .setDescription(METRIC_DESCRIPTION_MEMORY) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(usageEden, ATTR_MEMORY_EDEN_USED); + measurement.record(usageSurvivor, ATTR_MEMORY_SURVIVOR_USED); + }); + meter + .upDownCounterBuilder(METRIC_NAME_MEMORY_AFTER) + .setDescription(METRIC_DESCRIPTION_MEMORY_AFTER) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(usageEdenAfter, ATTR_MEMORY_EDEN_USED); + measurement.record(usageSurvivorAfter, ATTR_MEMORY_SURVIVOR_USED); + }); } @Override @@ -87,37 +96,24 @@ public void accept(RecordedEvent ev) { logger.fine(String.format("G1 GC Event seen without GC ID: %s", ev)); return; } - long gcId = ev.getLong(GC_ID); + recordValues(ev, when != null ? when.equals(BEFORE) : false); + } - var pair = awaitingPairs.remove(gcId); - if (pair == null) { - awaitingPairs.put(gcId, ev); - } else { - if (when.equals(BEFORE)) { - recordValues(ev, pair); - } else { // i.e. when.equals(AFTER) - recordValues(pair, ev); + private void recordValues(RecordedEvent event, boolean before) { + if (event.hasField(EDEN_USED_SIZE)) { + if (before) { + usageEden = event.getLong(EDEN_USED_SIZE); + } else { + usageEdenAfter = event.getLong(EDEN_USED_SIZE); } } - } - private void recordValues(RecordedEvent before, RecordedEvent after) { - if (after.hasField("edenUsedSize")) { - memoryHistogram.record(after.getLong("edenUsedSize"), ATTR_MEMORY_EDEN_USED); - if (before.hasField("edenUsedSize")) { - memoryHistogram.record( - after.getLong("edenUsedSize") - before.getLong("edenUsedSize"), - ATTR_MEMORY_EDEN_SIZE_DELTA); + if (event.hasField(SURVIVOR_USED_SIZE)) { + if (before) { + usageSurvivor = event.getLong(SURVIVOR_USED_SIZE); + } else { + usageSurvivorAfter = event.getLong(SURVIVOR_USED_SIZE); } } - if (after.hasField("edenTotalSize")) { - memoryHistogram.record(after.getLong("edenTotalSize"), ATTR_MEMORY_EDEN_SIZE); - } - if (after.hasField("survivorUsedSize")) { - memoryHistogram.record(after.getLong("survivorUsedSize"), ATTR_MEMORY_SURVIVOR_SIZE); - } - if (after.hasField("numberOfRegions")) { - memoryHistogram.record(after.getLong("numberOfRegions"), ATTR_MEMORY_REGIONS); - } } } diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java index fceb06c3d..893d5be1e 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java @@ -5,32 +5,29 @@ package io.opentelemetry.contrib.jfr.metrics.internal.memory; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_USAGE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_POOL; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_TYPE; import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.BYTES; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.COMMITTED; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.MILLISECONDS; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.RESERVED; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.TOTAL_USED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.COMMITTED_SIZE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.HEAP; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_COMMITTED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY_AFTER; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_COMMITTED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY_AFTER; import static io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler.defaultMeter; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.DoubleHistogram; -import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; -import io.opentelemetry.contrib.jfr.metrics.internal.DurationUtil; import io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler; import java.time.Duration; -import java.util.HashMap; -import java.util.Map; +import java.util.Optional; import jdk.jfr.consumer.RecordedEvent; import jdk.jfr.consumer.RecordedObject; /** This class handles GCHeapSummary JFR events. For GC purposes they come in pairs. */ public final class GCHeapSummaryHandler implements RecordedEventHandler { - private static final String METRIC_NAME_DURATION = "process.runtime.jvm.gc.time"; - private static final String METRIC_DESCRIPTION_DURATION = "GC Duration"; - private static final String METRIC_NAME_MEMORY = "process.runtime.jvm.memory.used"; - private static final String METRIC_DESCRIPTION_MEMORY = "Heap utilization"; private static final String EVENT_NAME = "jdk.GCHeapSummary"; private static final String BEFORE = "Before GC"; private static final String AFTER = "After GC"; @@ -38,16 +35,12 @@ public final class GCHeapSummaryHandler implements RecordedEventHandler { private static final String WHEN = "when"; private static final String HEAP_USED = "heapUsed"; private static final String HEAP_SPACE = "heapSpace"; - private static final String COMMITTED_SIZE = "committedSize"; - private static final String RESERVED_SIZE = "reservedSize"; - private static final Attributes ATTR_MEMORY_USED = Attributes.of(ATTR_USAGE, TOTAL_USED); - private static final Attributes ATTR_MEMORY_COMMITTED = Attributes.of(ATTR_USAGE, COMMITTED); - private static final Attributes ATTR_MEMORY_RESERVED = Attributes.of(ATTR_USAGE, RESERVED); + private static final Attributes ATTR = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "Java heap space"); - private final Map awaitingPairs = new HashMap<>(); - - private DoubleHistogram durationHistogram; - private LongHistogram memoryHistogram; + private volatile long usageBefore = 0; + private volatile long usageAfter = 0; + private volatile long committedSize = 0; public GCHeapSummaryHandler() { initializeMeter(defaultMeter()); @@ -55,19 +48,30 @@ public GCHeapSummaryHandler() { @Override public void initializeMeter(Meter meter) { - durationHistogram = - meter - .histogramBuilder(METRIC_NAME_DURATION) - .setDescription(METRIC_DESCRIPTION_DURATION) - .setUnit(MILLISECONDS) - .build(); - memoryHistogram = - meter - .histogramBuilder(METRIC_NAME_MEMORY) - .setDescription(METRIC_DESCRIPTION_MEMORY) - .setUnit(BYTES) - .ofLongs() - .build(); + meter + .upDownCounterBuilder(METRIC_NAME_MEMORY) + .setDescription(METRIC_DESCRIPTION_MEMORY) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(usageBefore, ATTR); + }); + meter + .upDownCounterBuilder(METRIC_NAME_MEMORY_AFTER) + .setDescription(METRIC_DESCRIPTION_MEMORY_AFTER) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(usageAfter, ATTR); + }); + meter + .upDownCounterBuilder(METRIC_NAME_COMMITTED) + .setDescription(METRIC_DESCRIPTION_COMMITTED) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(committedSize, ATTR); + }); } @Override @@ -90,38 +94,31 @@ public void accept(RecordedEvent ev) { if (!ev.hasField(GC_ID)) { return; } - long gcId = ev.getLong(GC_ID); - - var pair = awaitingPairs.get(gcId); - if (pair == null) { - awaitingPairs.put(gcId, ev); - } else { - awaitingPairs.remove(gcId); - if (when != null && when.equals(BEFORE)) { - recordValues(ev, pair); - } else { // i.e. when.equals(AFTER) - recordValues(pair, ev); - } - } + recordValues(ev, when != null ? when.equals(BEFORE) : false); } - private void recordValues(RecordedEvent before, RecordedEvent after) { - durationHistogram.record( - DurationUtil.toMillis(Duration.between(before.getStartTime(), after.getStartTime()))); - if (after.hasField(HEAP_USED)) { - memoryHistogram.record(after.getLong(HEAP_USED), ATTR_MEMORY_USED); + private void recordValues(RecordedEvent event, boolean before) { + + if (event.hasField(HEAP_USED)) { + if (before) { + usageBefore = event.getLong(HEAP_USED); + } else { + usageAfter = event.getLong(HEAP_USED); + } } - if (after.hasField(HEAP_SPACE)) { - Object heapSpaceValue = after.getValue(HEAP_SPACE); + if (event.hasField(HEAP_SPACE)) { + Object heapSpaceValue = event.getValue(HEAP_SPACE); if (heapSpaceValue instanceof RecordedObject) { RecordedObject heapSpace = (RecordedObject) heapSpaceValue; if (heapSpace.hasField(COMMITTED_SIZE)) { - memoryHistogram.record(heapSpace.getLong(COMMITTED_SIZE), ATTR_MEMORY_COMMITTED); - } - if (heapSpace.hasField(RESERVED_SIZE)) { - memoryHistogram.record(heapSpace.getLong(RESERVED_SIZE), ATTR_MEMORY_RESERVED); + committedSize = heapSpace.getLong(COMMITTED_SIZE); } } } } + + @Override + public Optional getPollingDuration() { + return Optional.of(Duration.ofSeconds(1)); + } } diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java index 78b9c7e23..99563d401 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java @@ -5,19 +5,23 @@ package io.opentelemetry.contrib.jfr.metrics.internal.memory; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_USAGE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_POOL; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_TYPE; import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.BYTES; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.EDEN_SIZE; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.EDEN_SIZE_DELTA; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.SURVIVOR_SIZE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.COMMITTED_SIZE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.HEAP; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_COMMITTED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY_AFTER; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_COMMITTED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY_AFTER; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.USED; import static io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler.defaultMeter; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler; -import java.util.HashMap; -import java.util.Map; import java.util.logging.Logger; import jdk.jfr.consumer.RecordedEvent; import jdk.jfr.consumer.RecordedObject; @@ -28,23 +32,30 @@ */ public final class ParallelHeapSummaryHandler implements RecordedEventHandler { private static final Logger logger = Logger.getLogger(ParallelHeapSummaryHandler.class.getName()); - - private static final String METRIC_NAME_MEMORY = "process.runtime.jvm.memory.used"; - private static final String METRIC_DESCRIPTION_MEMORY = "Heap utilization"; private static final String EVENT_NAME = "jdk.PSHeapSummary"; private static final String BEFORE = "Before GC"; private static final String AFTER = "After GC"; private static final String GC_ID = "gcId"; private static final String WHEN = "when"; - private static final Attributes ATTR_MEMORY_EDEN_SIZE = Attributes.of(ATTR_USAGE, EDEN_SIZE); - private static final Attributes ATTR_MEMORY_EDEN_SIZE_DELTA = - Attributes.of(ATTR_USAGE, EDEN_SIZE_DELTA); - private static final Attributes ATTR_MEMORY_SURVIVOR_SIZE = - Attributes.of(ATTR_USAGE, SURVIVOR_SIZE); - - private final Map awaitingPairs = new HashMap<>(); + private static final Attributes ATTR_MEMORY_EDEN_USED = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "PS Eden Space"); + private static final Attributes ATTR_MEMORY_SURVIVOR_USED = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "PS Survivor Space"); + private static final Attributes ATTR_MEMORY_OLD_USED = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "PS Old Gen"); + private static final Attributes ATTR_MEMORY_COMMITTED_OLD = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "PS Old Gen"); + private static final Attributes ATTR_MEMORY_COMMITTED_YOUNG = + Attributes.of(ATTR_TYPE, HEAP, ATTR_POOL, "PS Eden Space"); - private LongHistogram memoryHistogram; + private volatile long usageEden = 0; + private volatile long usageEdenAfter = 0; + private volatile long usageSurvivor = 0; + private volatile long usageSurvivorAfter = 0; + private volatile long usageOld = 0; + private volatile long usageOldAfter = 0; + private volatile long committedOld = 0; + private volatile long committedYoung = 0; public ParallelHeapSummaryHandler() { initializeMeter(defaultMeter()); @@ -52,13 +63,35 @@ public ParallelHeapSummaryHandler() { @Override public void initializeMeter(Meter meter) { - memoryHistogram = - meter - .histogramBuilder(METRIC_NAME_MEMORY) - .setDescription(METRIC_DESCRIPTION_MEMORY) - .setUnit(BYTES) - .ofLongs() - .build(); + meter + .upDownCounterBuilder(METRIC_NAME_MEMORY) + .setDescription(METRIC_DESCRIPTION_MEMORY) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(usageEden, ATTR_MEMORY_EDEN_USED); + measurement.record(usageSurvivor, ATTR_MEMORY_SURVIVOR_USED); + measurement.record(usageOld, ATTR_MEMORY_OLD_USED); + }); + meter + .upDownCounterBuilder(METRIC_NAME_MEMORY_AFTER) + .setDescription(METRIC_DESCRIPTION_MEMORY_AFTER) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(usageEdenAfter, ATTR_MEMORY_EDEN_USED); + measurement.record(usageSurvivorAfter, ATTR_MEMORY_SURVIVOR_USED); + measurement.record(usageOldAfter, ATTR_MEMORY_OLD_USED); + }); + meter + .upDownCounterBuilder(METRIC_NAME_COMMITTED) + .setDescription(METRIC_DESCRIPTION_COMMITTED) + .setUnit(BYTES) + .buildWithCallback( + measurement -> { + measurement.record(committedOld, ATTR_MEMORY_COMMITTED_OLD); + measurement.record(committedYoung, ATTR_MEMORY_COMMITTED_YOUNG); + }); } @Override @@ -86,45 +119,68 @@ public void accept(RecordedEvent ev) { logger.fine(String.format("Parallel GC Event seen without GC ID: %s", ev)); return; } - long gcId = ev.getLong(GC_ID); - - var pair = awaitingPairs.remove(gcId); - if (pair == null) { - awaitingPairs.put(gcId, ev); - } else { - if (when.equals(BEFORE)) { - recordValues(ev, pair); - } else { // i.e. when.equals(AFTER) - recordValues(pair, ev); - } - } + recordValues(ev, when != null ? when.equals(BEFORE) : false); } - private void recordValues(RecordedEvent before, RecordedEvent after) { - if (after.hasField("edenSpace")) { - Object edenSpaceObj = after.getValue("edenSpace"); + private void recordValues(RecordedEvent event, boolean before) { + if (event.hasField("edenSpace")) { + Object edenSpaceObj = event.getValue("edenSpace"); if (edenSpaceObj instanceof RecordedObject) { RecordedObject edenSpace = (RecordedObject) edenSpaceObj; - memoryHistogram.record(edenSpace.getLong("size"), ATTR_MEMORY_EDEN_SIZE); - if (before.hasField("edenSpace")) { - Object beforeSpaceObj = before.getValue("edenSpace"); - if (beforeSpaceObj instanceof RecordedObject) { - RecordedObject beforeSpace = (RecordedObject) beforeSpaceObj; - if (edenSpace.hasField("size") && beforeSpace.hasField("size")) { - memoryHistogram.record( - edenSpace.getLong("size") - beforeSpace.getLong("size"), - ATTR_MEMORY_EDEN_SIZE_DELTA); - } + if (edenSpace.hasField(USED)) { + if (before) { + usageEden = edenSpace.getLong(USED); + } else { + usageEdenAfter = edenSpace.getLong(USED); } } } } - if (after.hasField("fromSpace")) { - Object fromSpaceObj = after.getValue("fromSpace"); - if (after.getValue("fromSpace") instanceof RecordedObject) { + + if (event.hasField("fromSpace")) { + Object fromSpaceObj = event.getValue("fromSpace"); + if (fromSpaceObj instanceof RecordedObject) { RecordedObject fromSpace = (RecordedObject) fromSpaceObj; - if (fromSpace.hasField("size")) { - memoryHistogram.record(fromSpace.getLong("size"), ATTR_MEMORY_SURVIVOR_SIZE); + if (fromSpace.hasField(USED)) { + if (before) { + usageSurvivor = fromSpace.getLong(USED); + } else { + usageSurvivorAfter = fromSpace.getLong(USED); + } + } + } + } + + if (event.hasField("oldObjectSpace")) { + Object oldObjectSpaceObj = event.getValue("oldObjectSpace"); + if (oldObjectSpaceObj instanceof RecordedObject) { + RecordedObject oldObjectSpace = (RecordedObject) oldObjectSpaceObj; + if (oldObjectSpace.hasField(USED)) { + if (before) { + usageSurvivor = oldObjectSpace.getLong(USED); + } else { + usageSurvivorAfter = oldObjectSpace.getLong(USED); + } + } + } + } + + if (event.hasField("oldSpace")) { + Object oldSpaceObj = event.getValue("oldSpace"); + if (oldSpaceObj instanceof RecordedObject) { + RecordedObject oldSpace = (RecordedObject) oldSpaceObj; + if (oldSpace.hasField(COMMITTED_SIZE)) { + committedOld = oldSpace.getLong(COMMITTED_SIZE); + } + } + } + + if (event.hasField("youngSpace")) { + Object youngSpaceObj = event.getValue("youngSpace"); + if (youngSpaceObj instanceof RecordedObject) { + RecordedObject youngSpace = (RecordedObject) youngSpaceObj; + if (youngSpace.hasField(COMMITTED_SIZE)) { + committedYoung = youngSpace.getLong(COMMITTED_SIZE); } } } diff --git a/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrCPUTest.java b/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrCPULockTest.java similarity index 66% rename from jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrCPUTest.java rename to jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrCPULockTest.java index 9b9cfa5f0..f26d9b847 100644 --- a/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrCPUTest.java +++ b/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrCPULockTest.java @@ -9,10 +9,10 @@ import org.junit.jupiter.api.Test; -public class JfrCPUTest extends AbstractMetricsTest { +public class JfrCPULockTest extends AbstractMetricsTest { @Test - public void shouldHaveGcAndLockEvents() throws Exception { + public void shouldHaveLockEvents() throws Exception { // This should generate some events System.gc(); synchronized (this) { @@ -24,11 +24,6 @@ public void shouldHaveGcAndLockEvents() throws Exception { metric .hasName("process.runtime.jvm.cpu.longlock") .hasUnit(MILLISECONDS) - .hasHistogramSatisfying(histogram -> {}), - metric -> - metric - .hasName("process.runtime.jvm.gc.time") - .hasUnit(MILLISECONDS) .hasHistogramSatisfying(histogram -> {})); } } From dd5c43ddf9faaef59187baac7989990170c908d4 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Mon, 21 Nov 2022 12:13:52 -0500 Subject: [PATCH 2/6] tests for CPULoad and GC --- .../jfr/metrics/internal/Constants.java | 1 - .../internal/cpu/OverallCPULoadHandler.java | 3 +- .../metrics/JfrGCHeapSummaryHandlerTest.java | 65 +++++++++++++++++++ .../metrics/JfrOverallCPULoadHandlerTest.java | 30 +++++++++ 4 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrGCHeapSummaryHandlerTest.java create mode 100644 jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrOverallCPULoadHandlerTest.java diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java index 14fccccef..2c0ea6a2c 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/Constants.java @@ -17,7 +17,6 @@ private Constants() {} public static final String PERCENTAGE = "%age"; public static final String USER = "user"; public static final String SYSTEM = "system"; - public static final String MACHINE = "machine.total"; public static final String G1 = "g1"; public static final String TOTAL_USED = "total.used"; public static final String EDEN_USED = "eden.used"; diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java index 0973283cf..1997d8705 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/cpu/OverallCPULoadHandler.java @@ -5,7 +5,6 @@ package io.opentelemetry.contrib.jfr.metrics.internal.cpu; -import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.MACHINE; import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.UNIT_UTILIZATION; import static io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler.defaultMeter; @@ -59,7 +58,7 @@ public void accept(RecordedEvent ev) { process = ev.getDouble(JVM_USER) + ev.getDouble(JVM_SYSTEM); } if (ev.hasField(MACHINE_TOTAL)) { - machine = ev.getDouble(MACHINE); + machine = ev.getDouble(MACHINE_TOTAL); } } diff --git a/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrGCHeapSummaryHandlerTest.java b/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrGCHeapSummaryHandlerTest.java new file mode 100644 index 000000000..85d03c0fd --- /dev/null +++ b/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrGCHeapSummaryHandlerTest.java @@ -0,0 +1,65 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jfr.metrics; + +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_POOL; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.ATTR_TYPE; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.BYTES; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.HEAP; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_COMMITTED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_DESCRIPTION_MEMORY_AFTER; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_COMMITTED; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY; +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.METRIC_NAME_MEMORY_AFTER; +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.sdk.metrics.data.LongPointData; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.data.SumData; +import org.assertj.core.api.ThrowingConsumer; +import org.junit.jupiter.api.Test; + +public class JfrGCHeapSummaryHandlerTest extends AbstractMetricsTest { + + static class AttributeCheck implements ThrowingConsumer { + @Override + public void acceptThrows(MetricData metricData) throws Throwable { + SumData sumData = metricData.getLongSumData(); + assertThat(sumData.getPoints()) + .map(LongPointData.class::cast) + .anyMatch( + p -> + p.getAttributes().get(ATTR_TYPE).equals(HEAP) + && p.getAttributes().get(ATTR_POOL).equals("Java heap space")); + } + } + + @Test + public void shouldHaveGCEvents() throws Exception { + System.gc(); + + waitAndAssertMetrics( + metric -> + metric + .hasName(METRIC_NAME_MEMORY) + .hasUnit(BYTES) + .hasDescription(METRIC_DESCRIPTION_MEMORY) + .satisfies(new AttributeCheck()), + metric -> + metric + .hasName(METRIC_NAME_MEMORY_AFTER) + .hasUnit(BYTES) + .hasDescription(METRIC_DESCRIPTION_MEMORY_AFTER) + .satisfies(new AttributeCheck()), + metric -> + metric + .hasName(METRIC_NAME_COMMITTED) + .hasUnit(BYTES) + .hasDescription(METRIC_DESCRIPTION_COMMITTED) + .satisfies(new AttributeCheck())); + } +} diff --git a/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrOverallCPULoadHandlerTest.java b/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrOverallCPULoadHandlerTest.java new file mode 100644 index 000000000..afdd3eead --- /dev/null +++ b/jfr-streaming/src/test/java/io/opentelemetry/contrib/jfr/metrics/JfrOverallCPULoadHandlerTest.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jfr.metrics; + +import static io.opentelemetry.contrib.jfr.metrics.internal.Constants.UNIT_UTILIZATION; + +import org.junit.jupiter.api.Test; + +public class JfrOverallCPULoadHandlerTest extends AbstractMetricsTest { + + @Test + public void shouldHaveCPULoadEvents() throws Exception { + waitAndAssertMetrics( + metric -> + metric + .hasName("process.runtime.jvm.cpu.utilization") + .hasUnit(UNIT_UTILIZATION) + .hasDescription("Recent CPU utilization for the process") + .hasDoubleGaugeSatisfying(gauge -> {}), + metric -> + metric + .hasName("process.runtime.jvm.system.cpu.utilization") + .hasUnit(UNIT_UTILIZATION) + .hasDescription("Recent CPU utilization for the whole system") + .hasDoubleGaugeSatisfying(gauge -> {})); + } +} From 86609d470011f2a96b415f7a4327ecd839f0fc35 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Tue, 29 Nov 2022 10:03:11 -0500 Subject: [PATCH 3/6] Update jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../jfr/metrics/internal/memory/G1HeapSummaryHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java index 6328239f8..6b53bb53f 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/G1HeapSummaryHandler.java @@ -96,7 +96,7 @@ public void accept(RecordedEvent ev) { logger.fine(String.format("G1 GC Event seen without GC ID: %s", ev)); return; } - recordValues(ev, when != null ? when.equals(BEFORE) : false); + recordValues(ev, BEFORE.equals(when)); } private void recordValues(RecordedEvent event, boolean before) { From 9210884fcba2f6af4985c7ac1c5cd35682cf7c01 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Tue, 29 Nov 2022 10:03:17 -0500 Subject: [PATCH 4/6] Update jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../jfr/metrics/internal/memory/GCHeapSummaryHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java index 893d5be1e..1ea7ae418 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/GCHeapSummaryHandler.java @@ -94,7 +94,7 @@ public void accept(RecordedEvent ev) { if (!ev.hasField(GC_ID)) { return; } - recordValues(ev, when != null ? when.equals(BEFORE) : false); + recordValues(ev, BEFORE.equals(when)); } private void recordValues(RecordedEvent event, boolean before) { From 19be1843ca3b76cb641560faaa8cfcdc6dc7a1c8 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Tue, 29 Nov 2022 10:03:26 -0500 Subject: [PATCH 5/6] Update jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java index 99563d401..541558958 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java @@ -119,7 +119,7 @@ public void accept(RecordedEvent ev) { logger.fine(String.format("Parallel GC Event seen without GC ID: %s", ev)); return; } - recordValues(ev, when != null ? when.equals(BEFORE) : false); + recordValues(ev, BEFORE.equals(when)); } private void recordValues(RecordedEvent event, boolean before) { From e7fc7691fef34f428468dada776b5268f738a231 Mon Sep 17 00:00:00 2001 From: Robert Toyonaga Date: Tue, 29 Nov 2022 10:46:20 -0500 Subject: [PATCH 6/6] refactor --- .../memory/ParallelHeapSummaryHandler.java | 124 ++++++++++-------- 1 file changed, 66 insertions(+), 58 deletions(-) diff --git a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java index 541558958..7abf1779b 100644 --- a/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java +++ b/jfr-streaming/src/main/java/io/opentelemetry/contrib/jfr/metrics/internal/memory/ParallelHeapSummaryHandler.java @@ -22,6 +22,7 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.contrib.jfr.metrics.internal.RecordedEventHandler; +import java.util.function.Consumer; import java.util.logging.Logger; import jdk.jfr.consumer.RecordedEvent; import jdk.jfr.consumer.RecordedObject; @@ -122,67 +123,74 @@ public void accept(RecordedEvent ev) { recordValues(ev, BEFORE.equals(when)); } - private void recordValues(RecordedEvent event, boolean before) { - if (event.hasField("edenSpace")) { - Object edenSpaceObj = event.getValue("edenSpace"); - if (edenSpaceObj instanceof RecordedObject) { - RecordedObject edenSpace = (RecordedObject) edenSpaceObj; - if (edenSpace.hasField(USED)) { - if (before) { - usageEden = edenSpace.getLong(USED); - } else { - usageEdenAfter = edenSpace.getLong(USED); - } - } - } - } - - if (event.hasField("fromSpace")) { - Object fromSpaceObj = event.getValue("fromSpace"); - if (fromSpaceObj instanceof RecordedObject) { - RecordedObject fromSpace = (RecordedObject) fromSpaceObj; - if (fromSpace.hasField(USED)) { - if (before) { - usageSurvivor = fromSpace.getLong(USED); - } else { - usageSurvivorAfter = fromSpace.getLong(USED); - } - } - } + private static void doIfAvailable( + RecordedEvent event, String field, Consumer closure) { + if (!event.hasField(field)) { + return; } - - if (event.hasField("oldObjectSpace")) { - Object oldObjectSpaceObj = event.getValue("oldObjectSpace"); - if (oldObjectSpaceObj instanceof RecordedObject) { - RecordedObject oldObjectSpace = (RecordedObject) oldObjectSpaceObj; - if (oldObjectSpace.hasField(USED)) { - if (before) { - usageSurvivor = oldObjectSpace.getLong(USED); - } else { - usageSurvivorAfter = oldObjectSpace.getLong(USED); - } - } - } + Object value = event.getValue(field); + if (value instanceof RecordedObject) { + closure.accept((RecordedObject) value); } + } - if (event.hasField("oldSpace")) { - Object oldSpaceObj = event.getValue("oldSpace"); - if (oldSpaceObj instanceof RecordedObject) { - RecordedObject oldSpace = (RecordedObject) oldSpaceObj; - if (oldSpace.hasField(COMMITTED_SIZE)) { - committedOld = oldSpace.getLong(COMMITTED_SIZE); - } - } - } + private void recordValues(RecordedEvent event, boolean before) { - if (event.hasField("youngSpace")) { - Object youngSpaceObj = event.getValue("youngSpace"); - if (youngSpaceObj instanceof RecordedObject) { - RecordedObject youngSpace = (RecordedObject) youngSpaceObj; - if (youngSpace.hasField(COMMITTED_SIZE)) { - committedYoung = youngSpace.getLong(COMMITTED_SIZE); - } - } - } + doIfAvailable( + event, + "edenSpace", + edenSpace -> { + if (edenSpace.hasField(USED)) { + if (before) { + usageEden = edenSpace.getLong(USED); + } else { + usageEdenAfter = edenSpace.getLong(USED); + } + } + }); + + doIfAvailable( + event, + "fromSpace", + fromSpace -> { + if (fromSpace.hasField(USED)) { + if (before) { + usageSurvivor = fromSpace.getLong(USED); + } else { + usageSurvivorAfter = fromSpace.getLong(USED); + } + } + }); + + doIfAvailable( + event, + "oldObjectSpace", + oldObjectSpace -> { + if (oldObjectSpace.hasField(USED)) { + if (before) { + usageSurvivor = oldObjectSpace.getLong(USED); + } else { + usageSurvivorAfter = oldObjectSpace.getLong(USED); + } + } + }); + + doIfAvailable( + event, + "oldSpace", + oldSpace -> { + if (oldSpace.hasField(COMMITTED_SIZE)) { + committedOld = oldSpace.getLong(COMMITTED_SIZE); + } + }); + + doIfAvailable( + event, + "youngSpace", + youngSpace -> { + if (youngSpace.hasField(COMMITTED_SIZE)) { + committedYoung = youngSpace.getLong(COMMITTED_SIZE); + } + }); } }