From 0210e70c23aef8e4b33254dfed6a1f2bbceb7dcf Mon Sep 17 00:00:00 2001 From: Ryan Fitzpatrick Date: Fri, 11 Mar 2022 18:33:58 +0000 Subject: [PATCH] jmx-metrics: register all GroovyMetricEnvironment instruments --- .../jmxmetrics/GroovyMetricEnvironment.java | 305 +++++++++++++----- .../OtelHelperSynchronousMetricTest.java | 39 +++ 2 files changed, 272 insertions(+), 72 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 a903b4d03..f6ee39c62 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.SdkMeterProvider; import io.opentelemetry.sdk.metrics.common.InstrumentType; @@ -33,15 +39,48 @@ 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 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 +178,16 @@ 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(); + AtomicReference existingCounter = + doubleCounterRegistry.getExisting( + name, description, unit, InstrumentType.COUNTER, InstrumentValueType.DOUBLE); + + DoubleCounter dc = existingCounter.get(); + if (dc == null) { + dc = meter.counterBuilder(name).setDescription(description).setUnit(unit).ofDoubles().build(); + existingCounter.set(dc); + } + return dc; } /** @@ -152,7 +200,16 @@ public DoubleCounter getDoubleCounter( */ public LongCounter getLongCounter( final String name, final String description, final String unit) { - return meter.counterBuilder(name).setDescription(description).setUnit(unit).build(); + AtomicReference existingCounter = + longCounterRegistry.getExisting( + name, description, unit, InstrumentType.COUNTER, InstrumentValueType.LONG); + + LongCounter lc = existingCounter.get(); + if (lc == null) { + lc = meter.counterBuilder(name).setDescription(description).setUnit(unit).build(); + existingCounter.set(lc); + } + return lc; } /** @@ -165,12 +222,22 @@ 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(); + AtomicReference existingCounter = + doubleUpDownCounterRegistry.getExisting( + name, description, unit, InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.DOUBLE); + + DoubleUpDownCounter dc = existingCounter.get(); + if (dc == null) { + dc = + meter + .upDownCounterBuilder(name) + .setDescription(description) + .setUnit(unit) + .ofDoubles() + .build(); + existingCounter.set(dc); + } + return dc; } /** @@ -183,7 +250,16 @@ public DoubleUpDownCounter getDoubleUpDownCounter( */ public LongUpDownCounter getLongUpDownCounter( final String name, final String description, final String unit) { - return meter.upDownCounterBuilder(name).setDescription(description).setUnit(unit).build(); + AtomicReference existingCounter = + longUpDownCounterRegistry.getExisting( + name, description, unit, InstrumentType.UP_DOWN_COUNTER, InstrumentValueType.LONG); + LongUpDownCounter lc = existingCounter.get(); + + if (lc == null) { + lc = meter.upDownCounterBuilder(name).setDescription(description).setUnit(unit).build(); + existingCounter.set(lc); + } + return lc; } /** @@ -196,7 +272,16 @@ public LongUpDownCounter getLongUpDownCounter( */ public DoubleHistogram getDoubleHistogram( final String name, final String description, final String unit) { - return meter.histogramBuilder(name).setDescription(description).setUnit(unit).build(); + AtomicReference existingCounter = + doubleHistogramRegistry.getExisting( + name, description, unit, InstrumentType.HISTOGRAM, InstrumentValueType.DOUBLE); + + DoubleHistogram dh = existingCounter.get(); + if (dh == null) { + dh = meter.histogramBuilder(name).setDescription(description).setUnit(unit).build(); + existingCounter.set(dh); + } + return dh; } /** @@ -209,7 +294,16 @@ 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(); + AtomicReference existingCounter = + longHistogramRegistry.getExisting( + name, description, unit, InstrumentType.HISTOGRAM, InstrumentValueType.LONG); + + LongHistogram lh = existingCounter.get(); + if (lh == null) { + lh = meter.histogramBuilder(name).setDescription(description).setUnit(unit).ofLongs().build(); + existingCounter.set(lh); + } + return lh; } /** @@ -225,13 +319,20 @@ 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)); + AtomicReference existingCounter = + observableDoubleGaugeRegistry.getExisting( + name, description, unit, InstrumentType.OBSERVABLE_GAUGE, InstrumentValueType.DOUBLE); + + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedDoubleObserver(name, description, unit, InstrumentType.OBSERVABLE_GAUGE, updater); + + ObservableDoubleGauge dg = existingCounter.get(); + if (dg == null) { + dg = + meter.gaugeBuilder(name).setDescription(description).setUnit(unit).buildWithCallback(pdo); + existingCounter.set(dg); + } } /** @@ -247,13 +348,25 @@ 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)); + AtomicReference existingCounter = + observableLongGaugeRegistry.getExisting( + name, description, unit, InstrumentType.OBSERVABLE_GAUGE, InstrumentValueType.LONG); + + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedLongObserver(name, description, unit, InstrumentType.OBSERVABLE_GAUGE, updater); + + ObservableLongGauge dg = existingCounter.get(); + if (dg == null) { + dg = + meter + .gaugeBuilder(name) + .ofLongs() + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo); + existingCounter.set(dg); + } } /** @@ -269,14 +382,25 @@ 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)); + AtomicReference existingCounter = + observableDoubleCounterRegistry.getExisting( + name, description, unit, InstrumentType.OBSERVABLE_COUNTER, InstrumentValueType.DOUBLE); + + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedDoubleObserver(name, description, unit, InstrumentType.OBSERVABLE_COUNTER, updater); + + ObservableDoubleCounter dg = existingCounter.get(); + if (dg == null) { + dg = + meter + .counterBuilder(name) + .ofDoubles() + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo); + existingCounter.set(dg); + } } /** @@ -292,13 +416,24 @@ 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)); + AtomicReference existingCounter = + observableLongCounterRegistry.getExisting( + name, description, unit, InstrumentType.OBSERVABLE_COUNTER, InstrumentValueType.LONG); + + // we must invoke this every call to update the consumer's proxied value + Consumer pdo = + proxiedLongObserver(name, description, unit, InstrumentType.OBSERVABLE_COUNTER, updater); + + ObservableLongCounter dg = existingCounter.get(); + if (dg == null) { + dg = + meter + .counterBuilder(name) + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo); + existingCounter.set(dg); + } } /** @@ -314,14 +449,30 @@ 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)); + AtomicReference existingCounter = + observableDoubleUpDownCounterRegistry.getExisting( + name, + description, + unit, + InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, + InstrumentValueType.DOUBLE); + + // 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); + + ObservableDoubleUpDownCounter dg = existingCounter.get(); + if (dg == null) { + dg = + meter + .upDownCounterBuilder(name) + .ofDoubles() + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo); + existingCounter.set(dg); + } } /** @@ -337,13 +488,29 @@ 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)); + AtomicReference existingCounter = + observableLongUpDownCounterRegistry.getExisting( + name, + description, + unit, + InstrumentType.OBSERVABLE_UP_DOWN_COUNTER, + InstrumentValueType.LONG); + + // 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); + + ObservableLongUpDownCounter dg = existingCounter.get(); + if (dg == null) { + dg = + meter + .upDownCounterBuilder(name) + .setDescription(description) + .setUnit(unit) + .buildWithCallback(pdo); + existingCounter.set(dg); + } } private Consumer proxiedDoubleObserver( @@ -352,12 +519,9 @@ 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(); @@ -371,12 +535,9 @@ 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(); 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 a9fa2ef13..31506348c 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 @@ -38,15 +38,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", "µs"); + assertThat(dc) + .isSameAs(otel.doubleCounter("my-double-counter", "another double counter", "µs")); 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()) @@ -108,15 +113,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", "µs"); + assertThat(lc).isSameAs(otel.longCounter("my-long-counter", "another long counter", "µs")); 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()) @@ -179,17 +188,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", "µs"); + assertThat(dudc) + .isSameAs( + otel.doubleUpDownCounter( + "my-double-up-down-counter", "another double up-down-counter", "µs")); 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()) @@ -252,15 +272,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", "µs"); + assertThat(ludc) + .isSameAs( + otel.longUpDownCounter( + "my-long-up-down-counter", "another long up-down-counter", "µs")); 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()) @@ -322,15 +351,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", "µs"); + assertThat(dh) + .isSameAs(otel.doubleHistogram("my-double-histogram", "another double histogram", "µs")); 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()) @@ -396,15 +430,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", "µs"); + assertThat(lh) + .isSameAs(otel.longHistogram("my-long-histogram", "another long histogram", "µs")); 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())