From 83ac09e745a11e98fe4982f195e6de005c1f3463 Mon Sep 17 00:00:00 2001 From: Ryan Fitzpatrick Date: Tue, 2 Aug 2022 21:51:10 +0000 Subject: [PATCH] jmx-metrics: register all GroovyMetricEnvironment instruments --- .../jmxmetrics/GroovyMetricEnvironment.java | 304 +++++++++++++----- .../OtelHelperSynchronousMetricTest.java | 39 +++ 2 files changed, 267 insertions(+), 76 deletions(-) diff --git a/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/GroovyMetricEnvironment.java b/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/GroovyMetricEnvironment.java index 0e868e9d3..9389a6cd2 100644 --- a/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/GroovyMetricEnvironment.java +++ b/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/GroovyMetricEnvironment.java @@ -14,8 +14,14 @@ import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.LongUpDownCounter; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.metrics.ObservableDoubleCounter; +import io.opentelemetry.api.metrics.ObservableDoubleGauge; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import io.opentelemetry.api.metrics.ObservableDoubleUpDownCounter; +import io.opentelemetry.api.metrics.ObservableLongCounter; +import io.opentelemetry.api.metrics.ObservableLongGauge; import io.opentelemetry.api.metrics.ObservableLongMeasurement; +import io.opentelemetry.api.metrics.ObservableLongUpDownCounter; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.InstrumentValueType; @@ -26,6 +32,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Function; import javax.annotation.Nullable; public class GroovyMetricEnvironment { @@ -33,15 +40,68 @@ public class GroovyMetricEnvironment { private final SdkMeterProvider meterProvider; private final Meter meter; - // Observer updaters can only be specified in the builder as of v0.13.0, so to work with our model + private class Registry { + private final Map> registry = new ConcurrentHashMap<>(); + + private T getOrSet( + String name, + String description, + String unit, + InstrumentType instrumentType, + InstrumentValueType instrumentValueType, + Function createFunction) { + InstrumentDescriptor descriptor = + InstrumentDescriptor.create(name, description, unit, instrumentType, instrumentValueType); + return registry + .computeIfAbsent( + descriptor.hashCode(), + k -> { + AtomicReference at = new AtomicReference(); + at.set(createFunction.apply(k)); + return at; + }) + .get(); + } + + private AtomicReference getExisting( + String name, + String description, + String unit, + InstrumentType instrumentType, + InstrumentValueType instrumentValueType) { + InstrumentDescriptor descriptor = + InstrumentDescriptor.create(name, description, unit, instrumentType, instrumentValueType); + registry.putIfAbsent(descriptor.hashCode(), new AtomicReference<>()); + return registry.get(descriptor.hashCode()); + } + } + + // cache all instruments by their descriptors to prevent attempted duplicate creation + private final Registry doubleCounterRegistry = new Registry<>(); + private final Registry longCounterRegistry = new Registry<>(); + private final Registry doubleUpDownCounterRegistry = new Registry<>(); + private final Registry longUpDownCounterRegistry = new Registry<>(); + private final Registry doubleHistogramRegistry = new Registry<>(); + private final Registry longHistogramRegistry = new Registry<>(); + private final Registry observableDoubleGaugeRegistry = new Registry<>(); + private final Registry observableLongGaugeRegistry = new Registry<>(); + private final Registry observableDoubleCounterRegistry = + new Registry<>(); + private final Registry observableLongCounterRegistry = new Registry<>(); + private final Registry observableDoubleUpDownCounterRegistry = + new Registry<>(); + private final Registry observableLongUpDownCounterRegistry = + new Registry<>(); + + // Observable consumers can only be specified in the builder as of v0.13.0, so to work with our + // model // of running groovy scripts on an interval a reference to the desired updater should be held and // updated w/ each instrument creation call. Otherwise no observed changes in MBean availability - // would be possible. These registry stores are maps of instrument descriptor hashes to updater - // consumer references. - private final Map>> - longUpdaterRegistry = new ConcurrentHashMap<>(); - private final Map>> - doubleUpdaterRegistry = new ConcurrentHashMap<>(); + // would be possible. + private final Registry> longUpdaterRegistry = + new Registry<>(); + private final Registry> doubleUpdaterRegistry = + new Registry<>(); /** * A central context for creating and exporting metrics, to be used by groovy scripts via {@link @@ -139,7 +199,19 @@ protected static Attributes mapToAttributes(@Nullable final Map */ public DoubleCounter getDoubleCounter( final String name, final String description, final String unit) { - return meter.counterBuilder(name).setDescription(description).setUnit(unit).ofDoubles().build(); + return doubleCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.COUNTER, + InstrumentValueType.DOUBLE, + k -> + meter + .counterBuilder(name) + .setDescription(description) + .setUnit(unit) + .ofDoubles() + .build()); } /** @@ -152,7 +224,13 @@ public DoubleCounter getDoubleCounter( */ public LongCounter getLongCounter( final String name, final String description, final String unit) { - return meter.counterBuilder(name).setDescription(description).setUnit(unit).build(); + return longCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.COUNTER, + InstrumentValueType.LONG, + k -> meter.counterBuilder(name).setDescription(description).setUnit(unit).build()); } /** @@ -165,12 +243,19 @@ public LongCounter getLongCounter( */ public DoubleUpDownCounter getDoubleUpDownCounter( final String name, final String description, final String unit) { - return meter - .upDownCounterBuilder(name) - .setDescription(description) - .setUnit(unit) - .ofDoubles() - .build(); + return doubleUpDownCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.UP_DOWN_COUNTER, + InstrumentValueType.DOUBLE, + k -> + meter + .upDownCounterBuilder(name) + .setDescription(description) + .setUnit(unit) + .ofDoubles() + .build()); } /** @@ -183,7 +268,13 @@ public DoubleUpDownCounter getDoubleUpDownCounter( */ public LongUpDownCounter getLongUpDownCounter( final String name, final String description, final String unit) { - return meter.upDownCounterBuilder(name).setDescription(description).setUnit(unit).build(); + return longUpDownCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.UP_DOWN_COUNTER, + InstrumentValueType.LONG, + k -> meter.upDownCounterBuilder(name).setDescription(description).setUnit(unit).build()); } /** @@ -196,7 +287,13 @@ public LongUpDownCounter getLongUpDownCounter( */ public DoubleHistogram getDoubleHistogram( final String name, final String description, final String unit) { - return meter.histogramBuilder(name).setDescription(description).setUnit(unit).build(); + return doubleHistogramRegistry.getOrSet( + name, + description, + unit, + InstrumentType.HISTOGRAM, + InstrumentValueType.DOUBLE, + k -> meter.histogramBuilder(name).setDescription(description).setUnit(unit).build()); } /** @@ -209,7 +306,19 @@ public DoubleHistogram getDoubleHistogram( */ public LongHistogram getLongHistogram( final String name, final String description, final String unit) { - return meter.histogramBuilder(name).setDescription(description).setUnit(unit).ofLongs().build(); + return longHistogramRegistry.getOrSet( + name, + description, + unit, + InstrumentType.HISTOGRAM, + InstrumentValueType.LONG, + k -> + meter + .histogramBuilder(name) + .setDescription(description) + .setUnit(unit) + .ofLongs() + .build()); } /** @@ -225,13 +334,21 @@ public void registerDoubleValueCallback( final String description, final String unit, final Consumer updater) { - meter - .gaugeBuilder(name) - .setDescription(description) - .setUnit(unit) - .buildWithCallback( - proxiedDoubleObserver( - name, description, unit, InstrumentType.OBSERVABLE_GAUGE, updater)); + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedDoubleObserver(name, description, unit, InstrumentType.OBSERVABLE_GAUGE, updater); + observableDoubleGaugeRegistry.getOrSet( + name, + description, + unit, + InstrumentType.OBSERVABLE_GAUGE, + InstrumentValueType.DOUBLE, + k -> + meter + .gaugeBuilder(name) + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo)); } /** @@ -247,13 +364,22 @@ public void registerLongValueCallback( final String description, final String unit, final Consumer updater) { - meter - .gaugeBuilder(name) - .ofLongs() - .setDescription(description) - .setUnit(unit) - .buildWithCallback( - proxiedLongObserver(name, description, unit, InstrumentType.OBSERVABLE_GAUGE, updater)); + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedLongObserver(name, description, unit, InstrumentType.OBSERVABLE_GAUGE, updater); + observableLongGaugeRegistry.getOrSet( + name, + description, + unit, + InstrumentType.OBSERVABLE_GAUGE, + InstrumentValueType.LONG, + k -> + meter + .gaugeBuilder(name) + .ofLongs() + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo)); } /** @@ -269,14 +395,22 @@ public void registerDoubleCounterCallback( final String description, final String unit, final Consumer updater) { - meter - .counterBuilder(name) - .ofDoubles() - .setDescription(description) - .setUnit(unit) - .buildWithCallback( - proxiedDoubleObserver( - name, description, unit, InstrumentType.OBSERVABLE_COUNTER, updater)); + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedDoubleObserver(name, description, unit, InstrumentType.OBSERVABLE_COUNTER, updater); + observableDoubleCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.OBSERVABLE_COUNTER, + InstrumentValueType.DOUBLE, + k -> + meter + .counterBuilder(name) + .ofDoubles() + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo)); } /** @@ -292,13 +426,21 @@ public void registerLongCounterCallback( final String description, final String unit, final Consumer updater) { - meter - .counterBuilder(name) - .setDescription(description) - .setUnit(unit) - .buildWithCallback( - proxiedLongObserver( - name, description, unit, InstrumentType.OBSERVABLE_COUNTER, updater)); + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedLongObserver(name, description, unit, InstrumentType.OBSERVABLE_COUNTER, updater); + observableLongCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.OBSERVABLE_COUNTER, + InstrumentValueType.LONG, + k -> + meter + .counterBuilder(name) + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo)); } /** @@ -314,14 +456,23 @@ public void registerDoubleUpDownCounterCallback( final String description, final String unit, final Consumer updater) { - meter - .upDownCounterBuilder(name) - .ofDoubles() - .setDescription(description) - .setUnit(unit) - .buildWithCallback( - proxiedDoubleObserver( - name, description, unit, InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, updater)); + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedDoubleObserver( + name, description, unit, InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, updater); + observableDoubleUpDownCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, + InstrumentValueType.DOUBLE, + k -> + meter + .upDownCounterBuilder(name) + .ofDoubles() + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo)); } /** @@ -337,13 +488,22 @@ public void registerLongUpDownCounterCallback( final String description, final String unit, final Consumer updater) { - meter - .upDownCounterBuilder(name) - .setDescription(description) - .setUnit(unit) - .buildWithCallback( - proxiedLongObserver( - name, description, unit, InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, updater)); + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedLongObserver( + name, description, unit, InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, updater); + observableLongUpDownCounterRegistry.getOrSet( + name, + description, + unit, + InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, + InstrumentValueType.LONG, + k -> + meter + .upDownCounterBuilder(name) + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo)); } private Consumer proxiedDoubleObserver( @@ -352,16 +512,12 @@ private Consumer proxiedDoubleObserver( final String unit, final InstrumentType instrumentType, final Consumer updater) { - InstrumentDescriptor descriptor = - InstrumentDescriptor.create( - name, description, unit, instrumentType, InstrumentValueType.DOUBLE); - doubleUpdaterRegistry.putIfAbsent(descriptor.hashCode(), new AtomicReference<>()); AtomicReference> existingUpdater = - doubleUpdaterRegistry.get(descriptor.hashCode()); + doubleUpdaterRegistry.getExisting( + name, description, unit, instrumentType, InstrumentValueType.DOUBLE); existingUpdater.set(updater); return doubleResult -> { - Consumer existing = existingUpdater.get(); - existing.accept(doubleResult); + existingUpdater.get().accept(doubleResult); }; } @@ -371,16 +527,12 @@ private Consumer proxiedLongObserver( final String unit, final InstrumentType instrumentType, final Consumer updater) { - InstrumentDescriptor descriptor = - InstrumentDescriptor.create( - name, description, unit, instrumentType, InstrumentValueType.LONG); - longUpdaterRegistry.putIfAbsent(descriptor.hashCode(), new AtomicReference<>()); AtomicReference> existingUpdater = - longUpdaterRegistry.get(descriptor.hashCode()); + longUpdaterRegistry.getExisting( + name, description, unit, instrumentType, InstrumentValueType.LONG); existingUpdater.set(updater); return longResult -> { - Consumer existing = existingUpdater.get(); - existing.accept(longResult); + existingUpdater.get().accept(longResult); }; } } diff --git a/jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperSynchronousMetricTest.java b/jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperSynchronousMetricTest.java index 91df7f0d3..b84a31fda 100644 --- a/jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperSynchronousMetricTest.java +++ b/jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/OtelHelperSynchronousMetricTest.java @@ -39,15 +39,20 @@ void setUp() { @Test void doubleCounter() { DoubleCounter dc = otel.doubleCounter("double-counter", "a double counter", "ms"); + assertThat(dc).isSameAs(otel.doubleCounter("double-counter", "a double counter", "ms")); dc.add(123.456, Attributes.builder().put("key", "value").build()); dc = otel.doubleCounter("my-double-counter", "another double counter", "us"); + assertThat(dc) + .isSameAs(otel.doubleCounter("my-double-counter", "another double counter", "us")); dc.add(234.567, Attributes.builder().put("myKey", "myValue").build()); dc = otel.doubleCounter("another-double-counter", "double counter"); + assertThat(dc).isSameAs(otel.doubleCounter("another-double-counter", "double counter")); dc.add(345.678, Attributes.builder().put("anotherKey", "anotherValue").build()); dc = otel.doubleCounter("yet-another-double-counter"); + assertThat(dc).isSameAs(otel.doubleCounter("yet-another-double-counter")); dc.add(456.789, Attributes.builder().put("yetAnotherKey", "yetAnotherValue").build()); assertThat(metricReader.collectAllMetrics()) @@ -113,15 +118,19 @@ void doubleCounter() { @Test void longCounter() { LongCounter lc = otel.longCounter("long-counter", "a long counter", "ms"); + assertThat(lc).isSameAs(otel.longCounter("long-counter", "a long counter", "ms")); lc.add(123, Attributes.builder().put("key", "value").build()); lc = otel.longCounter("my-long-counter", "another long counter", "us"); + assertThat(lc).isSameAs(otel.longCounter("my-long-counter", "another long counter", "us")); lc.add(234, Attributes.builder().put("myKey", "myValue").build()); lc = otel.longCounter("another-long-counter", "long counter"); + assertThat(lc).isSameAs(otel.longCounter("another-long-counter", "long counter")); lc.add(345, Attributes.builder().put("anotherKey", "anotherValue").build()); lc = otel.longCounter("yet-another-long-counter"); + assertThat(lc).isSameAs(otel.longCounter("yet-another-long-counter")); lc.add(456, Attributes.builder().put("yetAnotherKey", "yetAnotherValue").build()); assertThat(metricReader.collectAllMetrics()) @@ -188,17 +197,28 @@ void longCounter() { void doubleUpDownCounter() { DoubleUpDownCounter dudc = otel.doubleUpDownCounter("double-up-down-counter", "a double up-down-counter", "ms"); + assertThat(dudc) + .isSameAs( + otel.doubleUpDownCounter("double-up-down-counter", "a double up-down-counter", "ms")); dudc.add(-234.567, Attributes.builder().put("key", "value").build()); dudc = otel.doubleUpDownCounter( "my-double-up-down-counter", "another double up-down-counter", "us"); + assertThat(dudc) + .isSameAs( + otel.doubleUpDownCounter( + "my-double-up-down-counter", "another double up-down-counter", "us")); dudc.add(-123.456, Attributes.builder().put("myKey", "myValue").build()); dudc = otel.doubleUpDownCounter("another-double-up-down-counter", "double up-down-counter"); + assertThat(dudc) + .isSameAs( + otel.doubleUpDownCounter("another-double-up-down-counter", "double up-down-counter")); dudc.add(345.678, Attributes.builder().put("anotherKey", "anotherValue").build()); dudc = otel.doubleUpDownCounter("yet-another-double-up-down-counter"); + assertThat(dudc).isSameAs(otel.doubleUpDownCounter("yet-another-double-up-down-counter")); dudc.add(456.789, Attributes.builder().put("yetAnotherKey", "yetAnotherValue").build()); assertThat(metricReader.collectAllMetrics()) @@ -265,15 +285,24 @@ void doubleUpDownCounter() { void longUpDownCounter() { LongUpDownCounter ludc = otel.longUpDownCounter("long-up-down-counter", "a long up-down-counter", "ms"); + assertThat(ludc) + .isSameAs(otel.longUpDownCounter("long-up-down-counter", "a long up-down-counter", "ms")); ludc.add(-234, Attributes.builder().put("key", "value").build()); ludc = otel.longUpDownCounter("my-long-up-down-counter", "another long up-down-counter", "us"); + assertThat(ludc) + .isSameAs( + otel.longUpDownCounter( + "my-long-up-down-counter", "another long up-down-counter", "us")); ludc.add(-123, Attributes.builder().put("myKey", "myValue").build()); ludc = otel.longUpDownCounter("another-long-up-down-counter", "long up-down-counter"); + assertThat(ludc) + .isSameAs(otel.longUpDownCounter("another-long-up-down-counter", "long up-down-counter")); ludc.add(345, Attributes.builder().put("anotherKey", "anotherValue").build()); ludc = otel.longUpDownCounter("yet-another-long-up-down-counter"); + assertThat(ludc).isSameAs(otel.longUpDownCounter("yet-another-long-up-down-counter")); ludc.add(456, Attributes.builder().put("yetAnotherKey", "yetAnotherValue").build()); assertThat(metricReader.collectAllMetrics()) @@ -339,15 +368,20 @@ void longUpDownCounter() { @Test void doubleHistogram() { DoubleHistogram dh = otel.doubleHistogram("double-histogram", "a double histogram", "ms"); + assertThat(dh).isSameAs(otel.doubleHistogram("double-histogram", "a double histogram", "ms")); dh.record(234.567, Attributes.builder().put("key", "value").build()); dh = otel.doubleHistogram("my-double-histogram", "another double histogram", "us"); + assertThat(dh) + .isSameAs(otel.doubleHistogram("my-double-histogram", "another double histogram", "us")); dh.record(123.456, Attributes.builder().put("myKey", "myValue").build()); dh = otel.doubleHistogram("another-double-histogram", "double histogram"); + assertThat(dh).isSameAs(otel.doubleHistogram("another-double-histogram", "double histogram")); dh.record(345.678, Attributes.builder().put("anotherKey", "anotherValue").build()); dh = otel.doubleHistogram("yet-another-double-histogram"); + assertThat(dh).isSameAs(otel.doubleHistogram("yet-another-double-histogram")); dh.record(456.789, Attributes.builder().put("yetAnotherKey", "yetAnotherValue").build()); assertThat(metricReader.collectAllMetrics()) @@ -417,15 +451,20 @@ void doubleHistogram() { @Test void longHistogram() { LongHistogram lh = otel.longHistogram("long-histogram", "a long histogram", "ms"); + assertThat(lh).isSameAs(otel.longHistogram("long-histogram", "a long histogram", "ms")); lh.record(234, Attributes.builder().put("key", "value").build()); lh = otel.longHistogram("my-long-histogram", "another long histogram", "us"); + assertThat(lh) + .isSameAs(otel.longHistogram("my-long-histogram", "another long histogram", "us")); lh.record(123, Attributes.builder().put("myKey", "myValue").build()); lh = otel.longHistogram("another-long-histogram", "long histogram"); + assertThat(lh).isSameAs(otel.longHistogram("another-long-histogram", "long histogram")); lh.record(345, Attributes.builder().put("anotherKey", "anotherValue").build()); lh = otel.longHistogram("yet-another-long-histogram"); + assertThat(lh).isSameAs(otel.longHistogram("yet-another-long-histogram")); lh.record(456, Attributes.builder().put("yetAnotherKey", "yetAnotherValue").build()); assertThat(metricReader.collectAllMetrics())