From 579ee6f0e9e32038139e57d17bb4f2e7f0cca95d Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 16 Dec 2021 16:58:59 +0100 Subject: [PATCH 01/10] Micrometer bridge instrumentation --- .../micrometer-1.5/javaagent/build.gradle.kts | 7 + .../micrometer/v1_5/Bridging.java | 41 +++++ .../v1_5/MetricsInstrumentation.java | 38 ++++ .../v1_5/MicrometerInstrumentationModule.java | 34 ++++ .../micrometer/v1_5/OpenTelemetryCounter.java | 69 ++++++++ .../micrometer/v1_5/OpenTelemetryGauge.java | 70 ++++++++ .../v1_5/OpenTelemetryMeterRegistry.java | 117 ++++++++++++ .../micrometer/v1_5/OpenTelemetryTimer.java | 167 ++++++++++++++++++ .../micrometer/v1_5/RemovableMeter.java | 11 ++ .../micrometer/v1_5/CounterTest.java | 77 ++++++++ .../micrometer/v1_5/GaugeTest.java | 63 +++++++ .../micrometer/v1_5/TimerTest.java | 138 +++++++++++++++ settings.gradle.kts | 1 + 13 files changed, 833 insertions(+) create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/RemovableMeter.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts b/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts new file mode 100644 index 000000000000..0bd3875f4897 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts @@ -0,0 +1,7 @@ +plugins { + id("otel.javaagent-instrumentation") +} + +dependencies { + library("io.micrometer:micrometer-core:1.5.0") +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java new file mode 100644 index 000000000000..d198691bdc14 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.Tag; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.instrumentation.api.cache.Cache; + +final class Bridging { + + private static final Cache> tagsCache = Cache.bounded(64); + + static Attributes toAttributes(Iterable tags) { + if (!tags.iterator().hasNext()) { + return Attributes.empty(); + } + AttributesBuilder builder = Attributes.builder(); + for (Tag tag : tags) { + builder.put(tagsCache.computeIfAbsent(tag.getKey(), AttributeKey::stringKey), tag.getValue()); + } + return builder.build(); + } + + static String description(Meter.Id id) { + String description = id.getDescription(); + return description == null ? "" : description; + } + + static String baseUnit(Meter.Id id) { + String baseUnit = id.getBaseUnit(); + return baseUnit == null ? "1" : baseUnit; + } + + private Bridging() {} +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java new file mode 100644 index 000000000000..326ddc9c6955 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java @@ -0,0 +1,38 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static net.bytebuddy.matcher.ElementMatchers.isTypeInitializer; +import static net.bytebuddy.matcher.ElementMatchers.named; + +import io.micrometer.core.instrument.Metrics; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class MetricsInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("io.micrometer.core.instrument.Metrics"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + isTypeInitializer(), this.getClass().getName() + "$StaticInitializerAdvice"); + } + + @SuppressWarnings("unused") + public static class StaticInitializerAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit() { + Metrics.addRegistry(OpenTelemetryMeterRegistry.create()); + } + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java new file mode 100644 index 000000000000..22e6e0096711 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java @@ -0,0 +1,34 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.Collections; +import java.util.List; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(InstrumentationModule.class) +public class MicrometerInstrumentationModule extends InstrumentationModule { + + public MicrometerInstrumentationModule() { + super("micrometer", "micrometer-1.5"); + } + + @Override + public ElementMatcher.Junction classLoaderMatcher() { + // added in 1.5 + return hasClassesNamed("io.micrometer.core.instrument.config.validate.Validated"); + } + + @Override + public List typeInstrumentations() { + return Collections.singletonList(new MetricsInstrumentation()); + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java new file mode 100644 index 000000000000..feda877c0601 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java @@ -0,0 +1,69 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.baseUnit; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.description; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.toAttributes; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Measurement; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleCounter; +import io.opentelemetry.api.metrics.Meter; +import java.util.Collections; + +final class OpenTelemetryCounter implements Counter, RemovableMeter { + + private final Id id; + // TODO: use bound instruments when they're available + private final DoubleCounter otelCounter; + private final Attributes attributes; + + volatile boolean removed = false; + + OpenTelemetryCounter(Id id, Meter otelMeter) { + this.id = id; + this.otelCounter = + otelMeter + .counterBuilder(id.getName()) + .setDescription(description(id)) + .setUnit(baseUnit(id)) + .ofDoubles() + .build(); + this.attributes = toAttributes(id.getTags()); + } + + @Override + public void increment(double v) { + if (removed) { + return; + } + otelCounter.add(v, attributes); + } + + @Override + public double count() { + // OpenTelemetry metrics bridge does not support reading measurements + return Double.NaN; + } + + @Override + public Iterable measure() { + // OpenTelemetry metrics bridge does not support reading measurements + return Collections.emptyList(); + } + + @Override + public Id getId() { + return id; + } + + @Override + public void onRemove() { + removed = true; + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java new file mode 100644 index 000000000000..54ddb74495db --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java @@ -0,0 +1,70 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.baseUnit; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.description; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.toAttributes; + +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.Measurement; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import java.util.Collections; +import java.util.function.ToDoubleFunction; + +final class OpenTelemetryGauge implements Gauge, RemovableMeter { + + private final Id id; + private final T obj; + private final ToDoubleFunction objMetric; + private final Attributes attributes; + + volatile boolean removed = false; + + OpenTelemetryGauge(Id id, T obj, ToDoubleFunction objMetric, Meter otelMeter) { + this.id = id; + this.obj = obj; + this.objMetric = objMetric; + this.attributes = toAttributes(id.getTags()); + + otelMeter + .gaugeBuilder(id.getName()) + .setDescription(description(id)) + .setUnit(baseUnit(id)) + .buildWithCallback(this::recordMeasurements); + } + + private void recordMeasurements(ObservableDoubleMeasurement measurement) { + if (removed) { + return; + } + measurement.record(objMetric.applyAsDouble(obj), attributes); + } + + @Override + public double value() { + // OpenTelemetry metrics bridge does not support reading measurements + return Double.NaN; + } + + @Override + public Iterable measure() { + // OpenTelemetry metrics bridge does not support reading measurements + return Collections.emptyList(); + } + + @Override + public Id getId() { + return id; + } + + @Override + public void onRemove() { + removed = true; + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java new file mode 100644 index 000000000000..38a5501f23ed --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java @@ -0,0 +1,117 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import io.micrometer.core.instrument.Clock; +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.DistributionSummary; +import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.FunctionTimer; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.LongTaskTimer; +import io.micrometer.core.instrument.Measurement; +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; +import io.micrometer.core.instrument.distribution.HistogramGauges; +import io.micrometer.core.instrument.distribution.pause.PauseDetector; +import io.opentelemetry.api.GlobalOpenTelemetry; +import java.util.concurrent.TimeUnit; +import java.util.function.ToDoubleFunction; +import java.util.function.ToLongFunction; + +public final class OpenTelemetryMeterRegistry extends MeterRegistry { + + private static final String INSTRUMENTATION_NAME = "io.opentelemetry.micrometer-1.5"; + + public static MeterRegistry create() { + OpenTelemetryMeterRegistry openTelemetryMeterRegistry = + new OpenTelemetryMeterRegistry( + Clock.SYSTEM, GlobalOpenTelemetry.get().getMeterProvider().get(INSTRUMENTATION_NAME)); + openTelemetryMeterRegistry.config().onMeterRemoved(OpenTelemetryMeterRegistry::onMeterRemoved); + return openTelemetryMeterRegistry; + } + + private final io.opentelemetry.api.metrics.Meter otelMeter; + + private OpenTelemetryMeterRegistry(Clock clock, io.opentelemetry.api.metrics.Meter otelMeter) { + super(clock); + this.otelMeter = otelMeter; + } + + @Override + protected Gauge newGauge(Meter.Id id, T t, ToDoubleFunction toDoubleFunction) { + return new OpenTelemetryGauge<>(id, t, toDoubleFunction, otelMeter); + } + + @Override + protected Counter newCounter(Meter.Id id) { + return new OpenTelemetryCounter(id, otelMeter); + } + + @Override + protected LongTaskTimer newLongTaskTimer( + Meter.Id id, DistributionStatisticConfig distributionStatisticConfig) { + throw new UnsupportedOperationException("Not implemented yet"); + } + + @Override + protected Timer newTimer( + Meter.Id id, + DistributionStatisticConfig distributionStatisticConfig, + PauseDetector pauseDetector) { + OpenTelemetryTimer timer = + new OpenTelemetryTimer(id, clock, distributionStatisticConfig, pauseDetector, otelMeter); + if (timer.isUsingMicrometerHistograms()) { + HistogramGauges.registerWithCommonFormat(timer, this); + } + return timer; + } + + @Override + protected DistributionSummary newDistributionSummary( + Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, double v) { + throw new UnsupportedOperationException("Not implemented yet"); + } + + @Override + protected Meter newMeter(Meter.Id id, Meter.Type type, Iterable iterable) { + throw new UnsupportedOperationException("Not implemented yet"); + } + + @Override + protected FunctionTimer newFunctionTimer( + Meter.Id id, + T t, + ToLongFunction toLongFunction, + ToDoubleFunction toDoubleFunction, + TimeUnit timeUnit) { + throw new UnsupportedOperationException("Not implemented yet"); + } + + @Override + protected FunctionCounter newFunctionCounter( + Meter.Id id, T t, ToDoubleFunction toDoubleFunction) { + throw new UnsupportedOperationException("Not implemented yet"); + } + + @Override + protected TimeUnit getBaseTimeUnit() { + return TimeUnit.MILLISECONDS; + } + + @Override + protected DistributionStatisticConfig defaultHistogramConfig() { + return DistributionStatisticConfig.DEFAULT; + } + + private static void onMeterRemoved(Meter meter) { + if (meter instanceof RemovableMeter) { + ((RemovableMeter) meter).onRemove(); + } + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java new file mode 100644 index 000000000000..2f132dbcecbf --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java @@ -0,0 +1,167 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.description; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.toAttributes; + +import io.micrometer.core.instrument.AbstractTimer; +import io.micrometer.core.instrument.Clock; +import io.micrometer.core.instrument.Measurement; +import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; +import io.micrometer.core.instrument.distribution.NoopHistogram; +import io.micrometer.core.instrument.distribution.TimeWindowMax; +import io.micrometer.core.instrument.distribution.pause.PauseDetector; +import io.micrometer.core.instrument.util.TimeUtils; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.LongHistogram; +import io.opentelemetry.api.metrics.Meter; +import java.util.Collections; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { + + // TODO: use bound instruments when they're available + private final LongHistogram otelHistogram; + private final Attributes attributes; + private final Measurements measurements; + + volatile boolean removed = false; + + OpenTelemetryTimer( + Id id, + Clock clock, + DistributionStatisticConfig distributionStatisticConfig, + PauseDetector pauseDetector, + Meter otelMeter) { + super(id, clock, distributionStatisticConfig, pauseDetector, TimeUnit.MILLISECONDS, false); + + this.otelHistogram = + otelMeter + .histogramBuilder(id.getName()) + .setDescription(description(id)) + .setUnit("milliseconds") + .ofLongs() + .build(); + this.attributes = toAttributes(id.getTags()); + + if (isUsingMicrometerHistograms()) { + measurements = new MicrometerHistogramMeasurements(clock, distributionStatisticConfig); + } else { + measurements = NoopMeasurements.INSTANCE; + } + } + + boolean isUsingMicrometerHistograms() { + return histogram != NoopHistogram.INSTANCE; + } + + @Override + protected void recordNonNegative(long amount, TimeUnit unit) { + if (amount >= 0 && !removed) { + otelHistogram.record(unit.toMillis(amount), attributes); + measurements.record(amount, unit); + } + } + + @Override + public long count() { + return measurements.count(); + } + + @Override + public double totalTime(TimeUnit unit) { + return measurements.totalTime(unit); + } + + @Override + public double max(TimeUnit unit) { + return measurements.max(unit); + } + + @Override + public Iterable measure() { + // OpenTelemetry metrics bridge does not support reading measurements + return Collections.emptyList(); + } + + @Override + public void onRemove() { + removed = true; + } + + private interface Measurements { + void record(long amount, TimeUnit unit); + + long count(); + + double totalTime(TimeUnit unit); + + double max(TimeUnit unit); + } + + // if micrometer histograms are not being used then there's no need to keep any local state + // OpenTelemetry metrics bridge does not support reading measurements + enum NoopMeasurements implements Measurements { + INSTANCE; + + @Override + public void record(long amount, TimeUnit unit) {} + + @Override + public long count() { + return 0; + } + + @Override + public double totalTime(TimeUnit unit) { + return Double.NaN; + } + + @Override + public double max(TimeUnit unit) { + return Double.NaN; + } + } + + // calculate count, totalTime and max value for the use of micrometer histograms + // kinda similar to how DropwizardTimer does that + private static final class MicrometerHistogramMeasurements implements Measurements { + + private final AtomicLong count = new AtomicLong(0); + private final AtomicLong totalTime = new AtomicLong(0); + private final TimeWindowMax max; + + MicrometerHistogramMeasurements( + Clock clock, DistributionStatisticConfig distributionStatisticConfig) { + this.max = new TimeWindowMax(clock, distributionStatisticConfig); + } + + @Override + public void record(long amount, TimeUnit unit) { + long nanos = unit.toNanos(amount); + count.incrementAndGet(); + totalTime.addAndGet(nanos); + max.record(nanos, TimeUnit.NANOSECONDS); + } + + @Override + public long count() { + return count.get(); + } + + @Override + public double totalTime(TimeUnit unit) { + return TimeUtils.nanosToUnit(totalTime.get(), unit); + } + + @Override + public double max(TimeUnit unit) { + return max.poll(unit); + } + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/RemovableMeter.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/RemovableMeter.java new file mode 100644 index 000000000000..c60703b8c7a6 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/RemovableMeter.java @@ -0,0 +1,11 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +interface RemovableMeter { + + void onRemove(); +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java new file mode 100644 index 000000000000..c3685a502915 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java @@ -0,0 +1,77 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry; +import static io.opentelemetry.sdk.testing.assertj.metrics.MetricAssertions.assertThat; + +import io.micrometer.core.instrument.Counter; +import io.micrometer.core.instrument.Metrics; +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +class CounterTest { + + static final String INSTRUMENTATION_NAME = "io.opentelemetry.micrometer-1.5"; + + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + @Test + void testCounter() { + // given + Counter counter = + Counter.builder("testCounter") + .description("This is a test counter") + .tags("tag", "value") + .baseUnit("items") + .register(Metrics.globalRegistry); + + // when + counter.increment(); + counter.increment(2); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testCounter", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDescription("This is a test counter") + .hasUnit("items") + .hasDoubleSum() + .isMonotonic() + .isCumulative() + .points() + .satisfiesExactly( + point -> + assertThat(point) + .hasValue(3) + .attributes() + .containsOnly(attributeEntry("tag", "value"))))); + testing.clearData(); + + // when + Metrics.globalRegistry.remove(counter); + counter.increment(); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testCounter", + metrics -> + metrics.allSatisfy( + metric -> + assertThat(metric) + .hasDoubleSum() + .points() + .noneSatisfy(point -> assertThat(point).hasValue(4)))); + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java new file mode 100644 index 000000000000..894d169ae321 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java @@ -0,0 +1,63 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry; +import static io.opentelemetry.sdk.testing.assertj.metrics.MetricAssertions.assertThat; + +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.Metrics; +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import org.assertj.core.api.AbstractIterableAssert; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +class GaugeTest { + + static final String INSTRUMENTATION_NAME = "io.opentelemetry.micrometer-1.5"; + + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + @Test + void testGauge() { + // when + Gauge gauge = + Gauge.builder("testGauge", () -> 42) + .description("This is a test gauge") + .tags("tag", "value") + .baseUnit("items") + .register(Metrics.globalRegistry); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testGauge", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDescription("This is a test gauge") + .hasUnit("items") + .hasDoubleGauge() + .points() + .satisfiesExactly( + point -> + assertThat(point) + .hasValue(42) + .attributes() + .containsOnly(attributeEntry("tag", "value"))))); + testing.clearData(); + + // when + Metrics.globalRegistry.remove(gauge); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, "testGauge", AbstractIterableAssert::isEmpty); + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java new file mode 100644 index 000000000000..76e7c399b702 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java @@ -0,0 +1,138 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry; +import static io.opentelemetry.sdk.testing.assertj.metrics.MetricAssertions.assertThat; + +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.Timer; +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import java.time.Duration; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +@SuppressWarnings("PreferJavaTimeOverload") +class TimerTest { + + static final String INSTRUMENTATION_NAME = "io.opentelemetry.micrometer-1.5"; + + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + @Test + void testTimer() { + // given + Timer timer = + Timer.builder("testTimer") + .description("This is a test timer") + .tags("tag", "value") + .register(Metrics.globalRegistry); + + // when + timer.record(42, TimeUnit.SECONDS); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testTimer", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDescription("This is a test timer") + .hasUnit("milliseconds") + .hasDoubleHistogram() + .points() + .satisfiesExactly( + point -> + assertThat(point) + .hasSum(42_000) + .hasCount(1) + .attributes() + .containsOnly(attributeEntry("tag", "value"))))); + testing.clearData(); + + // when + Metrics.globalRegistry.remove(timer); + timer.record(12, TimeUnit.SECONDS); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testTimer", + metrics -> + metrics.allSatisfy( + metric -> + assertThat(metric) + .hasDoubleHistogram() + .points() + .noneSatisfy(point -> assertThat(point).hasSum(54_000).hasCount(2)))); + } + + @Test + void testMicrometerHistogram() { + // given + Timer timer = + Timer.builder("testHistogram") + .description("This is a test timer") + .tags("tag", "value") + .serviceLevelObjectives( + Duration.ofSeconds(1), + Duration.ofSeconds(10), + Duration.ofSeconds(100), + Duration.ofSeconds(1000)) + .distributionStatisticBufferLength(10) + .register(Metrics.globalRegistry); + + // when + timer.record(500, TimeUnit.MILLISECONDS); + timer.record(5, TimeUnit.SECONDS); + timer.record(50, TimeUnit.SECONDS); + timer.record(500, TimeUnit.SECONDS); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testHistogram.histogram", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDoubleGauge() + .points() + .anySatisfy( + point -> + assertThat(point) + .hasValue(1) + .attributes() + .containsEntry("le", "1000")) + // TODO: I have absolutely no idea why otel drops these points below + /* + .anySatisfy( + point -> + assertThat(point) + .hasValue(2) + .attributes() + .containsEntry("le", "10000")) + .anySatisfy( + point -> + assertThat(point) + .hasValue(3) + .attributes() + .containsEntry("le", "100000")) + .anySatisfy( + point -> + assertThat(point) + .hasValue(4) + .attributes() + .containsEntry("le", "1000000")) + */ + )); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index fdc0204f3b71..3ad8e5744fe4 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -273,6 +273,7 @@ include(":instrumentation:logback-1.0:javaagent") include(":instrumentation:logback-1.0:library") include(":instrumentation:logback-1.0:testing") include(":instrumentation:methods:javaagent") +include(":instrumentation:micrometer:micrometer-1.5:javaagent") include(":instrumentation:mongo:mongo-3.1:javaagent") include(":instrumentation:mongo:mongo-3.1:library") include(":instrumentation:mongo:mongo-3.1:testing") From 944b071f5915c809d2874fc25d0b12f93b3d6474 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 17 Dec 2021 12:15:10 +0100 Subject: [PATCH 02/10] gauges with the same name and different attributes --- .../v1_5/AsyncInstrumentRegistry.java | 75 +++++++++++++++++++ .../micrometer/v1_5/OpenTelemetryGauge.java | 32 +++----- .../v1_5/OpenTelemetryMeterRegistry.java | 4 +- .../micrometer/v1_5/CounterTest.java | 6 ++ .../micrometer/v1_5/GaugeTest.java | 46 ++++++++++++ .../micrometer/v1_5/TimerTest.java | 46 ++++++------ 6 files changed, 164 insertions(+), 45 deletions(-) create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java new file mode 100644 index 000000000000..e8f56dc30836 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java @@ -0,0 +1,75 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Consumer; +import java.util.function.DoubleSupplier; +import java.util.function.ToDoubleFunction; + +final class AsyncInstrumentRegistry { + + private final Meter meter; + private final ConcurrentMap gauges = new ConcurrentHashMap<>(); + + AsyncInstrumentRegistry(Meter meter) { + this.meter = meter; + } + + void buildGauge( + String name, + String description, + String baseUnit, + Attributes attributes, + T obj, + ToDoubleFunction objMetric) { + gauges + .computeIfAbsent( + name, + n -> { + GaugeMeasurementsRecorder recorder = new GaugeMeasurementsRecorder(); + meter + .gaugeBuilder(name) + .setDescription(description) + .setUnit(baseUnit) + .buildWithCallback(recorder); + return recorder; + }) + .addGaugeMeasurement(attributes, obj, objMetric); + } + + void removeGauge(String name, Attributes attributes) { + GaugeMeasurementsRecorder recorder = gauges.get(name); + if (recorder != null) { + recorder.removeGaugeMeasurement(attributes); + } + } + + private static final class GaugeMeasurementsRecorder + implements Consumer { + + private final ConcurrentMap measurements = + new ConcurrentHashMap<>(); + + @Override + public void accept(ObservableDoubleMeasurement measurement) { + measurements.forEach( + (attributes, supplier) -> measurement.record(supplier.getAsDouble(), attributes)); + } + + void addGaugeMeasurement(Attributes attributes, T obj, ToDoubleFunction objMetric) { + measurements.put(attributes, () -> objMetric.applyAsDouble(obj)); + } + + void removeGaugeMeasurement(Attributes attributes) { + measurements.remove(attributes); + } + } +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java index 54ddb74495db..e573f9aed76a 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java @@ -12,38 +12,26 @@ import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.Measurement; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.Meter; -import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; import java.util.Collections; import java.util.function.ToDoubleFunction; final class OpenTelemetryGauge implements Gauge, RemovableMeter { private final Id id; - private final T obj; - private final ToDoubleFunction objMetric; private final Attributes attributes; + private final AsyncInstrumentRegistry asyncInstrumentRegistry; - volatile boolean removed = false; - - OpenTelemetryGauge(Id id, T obj, ToDoubleFunction objMetric, Meter otelMeter) { + OpenTelemetryGauge( + Id id, + T obj, + ToDoubleFunction objMetric, + AsyncInstrumentRegistry asyncInstrumentRegistry) { this.id = id; - this.obj = obj; - this.objMetric = objMetric; this.attributes = toAttributes(id.getTags()); + this.asyncInstrumentRegistry = asyncInstrumentRegistry; - otelMeter - .gaugeBuilder(id.getName()) - .setDescription(description(id)) - .setUnit(baseUnit(id)) - .buildWithCallback(this::recordMeasurements); - } - - private void recordMeasurements(ObservableDoubleMeasurement measurement) { - if (removed) { - return; - } - measurement.record(objMetric.applyAsDouble(obj), attributes); + asyncInstrumentRegistry.buildGauge( + id.getName(), description(id), baseUnit(id), attributes, obj, objMetric); } @Override @@ -65,6 +53,6 @@ public Id getId() { @Override public void onRemove() { - removed = true; + asyncInstrumentRegistry.removeGauge(id.getName(), attributes); } } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java index 38a5501f23ed..1e7e0faea927 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java @@ -37,15 +37,17 @@ public static MeterRegistry create() { } private final io.opentelemetry.api.metrics.Meter otelMeter; + private final AsyncInstrumentRegistry asyncInstrumentRegistry; private OpenTelemetryMeterRegistry(Clock clock, io.opentelemetry.api.metrics.Meter otelMeter) { super(clock); this.otelMeter = otelMeter; + this.asyncInstrumentRegistry = new AsyncInstrumentRegistry(otelMeter); } @Override protected Gauge newGauge(Meter.Id id, T t, ToDoubleFunction toDoubleFunction) { - return new OpenTelemetryGauge<>(id, t, toDoubleFunction, otelMeter); + return new OpenTelemetryGauge<>(id, t, toDoubleFunction, asyncInstrumentRegistry); } @Override diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java index c3685a502915..36f12655b4b2 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/CounterTest.java @@ -12,6 +12,7 @@ import io.micrometer.core.instrument.Metrics; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -22,6 +23,11 @@ class CounterTest { @RegisterExtension static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + @BeforeEach + void cleanupMeters() { + Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove); + } + @Test void testCounter() { // given diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java index 894d169ae321..7fee7f80c602 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java @@ -13,6 +13,7 @@ import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import org.assertj.core.api.AbstractIterableAssert; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -23,6 +24,11 @@ class GaugeTest { @RegisterExtension static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + @BeforeEach + void cleanupMeters() { + Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove); + } + @Test void testGauge() { // when @@ -60,4 +66,44 @@ void testGauge() { testing.waitAndAssertMetrics( INSTRUMENTATION_NAME, "testGauge", AbstractIterableAssert::isEmpty); } + + @Test + void gaugesWithSameNameAndDifferentTags() { + // when + Gauge.builder("testGaugeWithTags", () -> 12) + .description("First description wins") + .baseUnit("items") + .tags("tag", "1") + .register(Metrics.globalRegistry); + Gauge.builder("testGaugeWithTags", () -> 42) + .description("ignored") + .baseUnit("ignored") + .tags("tag", "2") + .register(Metrics.globalRegistry); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testGaugeWithTags", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDescription("First description wins") + .hasUnit("items") + .hasDoubleGauge() + .points() + .anySatisfy( + point -> + assertThat(point) + .hasValue(12) + .attributes() + .containsOnly(attributeEntry("tag", "1"))) + .anySatisfy( + point -> + assertThat(point) + .hasValue(42) + .attributes() + .containsOnly(attributeEntry("tag", "2"))))); + } } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java index 76e7c399b702..d4ff16b3e201 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java @@ -14,6 +14,7 @@ import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import java.time.Duration; import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -25,6 +26,11 @@ class TimerTest { @RegisterExtension static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + @BeforeEach + void cleanupMeters() { + Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove); + } + @Test void testTimer() { // given @@ -112,27 +118,23 @@ void testMicrometerHistogram() { .hasValue(1) .attributes() .containsEntry("le", "1000")) - // TODO: I have absolutely no idea why otel drops these points below - /* - .anySatisfy( - point -> - assertThat(point) - .hasValue(2) - .attributes() - .containsEntry("le", "10000")) - .anySatisfy( - point -> - assertThat(point) - .hasValue(3) - .attributes() - .containsEntry("le", "100000")) - .anySatisfy( - point -> - assertThat(point) - .hasValue(4) - .attributes() - .containsEntry("le", "1000000")) - */ - )); + .anySatisfy( + point -> + assertThat(point) + .hasValue(2) + .attributes() + .containsEntry("le", "10000")) + .anySatisfy( + point -> + assertThat(point) + .hasValue(3) + .attributes() + .containsEntry("le", "100000")) + .anySatisfy( + point -> + assertThat(point) + .hasValue(4) + .attributes() + .containsEntry("le", "1000000")))); } } From 1f958f98503c2c22ff571ce045d4121f48653f70 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 17 Dec 2021 13:02:58 +0100 Subject: [PATCH 03/10] weak ref gauge --- .../v1_5/AsyncInstrumentRegistry.java | 25 +++++++++++--- .../micrometer/v1_5/GaugeTest.java | 34 +++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java index e8f56dc30836..78aa0922ecdc 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java @@ -8,10 +8,10 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import java.lang.ref.WeakReference; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Consumer; -import java.util.function.DoubleSupplier; import java.util.function.ToDoubleFunction; final class AsyncInstrumentRegistry { @@ -55,21 +55,36 @@ void removeGauge(String name, Attributes attributes) { private static final class GaugeMeasurementsRecorder implements Consumer { - private final ConcurrentMap measurements = - new ConcurrentHashMap<>(); + private final ConcurrentMap measurements = new ConcurrentHashMap<>(); @Override public void accept(ObservableDoubleMeasurement measurement) { measurements.forEach( - (attributes, supplier) -> measurement.record(supplier.getAsDouble(), attributes)); + (attributes, gauge) -> { + Object obj = gauge.objWeakRef.get(); + if (obj != null) { + measurement.record(gauge.metricFunction.applyAsDouble(obj), attributes); + } + }); } void addGaugeMeasurement(Attributes attributes, T obj, ToDoubleFunction objMetric) { - measurements.put(attributes, () -> objMetric.applyAsDouble(obj)); + measurements.put(attributes, new GaugeInfo(obj, (ToDoubleFunction) objMetric)); } void removeGaugeMeasurement(Attributes attributes) { measurements.remove(attributes); } } + + private static final class GaugeInfo { + + private final WeakReference objWeakRef; + private final ToDoubleFunction metricFunction; + + private GaugeInfo(Object obj, ToDoubleFunction metricFunction) { + this.objWeakRef = new WeakReference<>(obj); + this.metricFunction = metricFunction; + } + } } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java index 7fee7f80c602..f1c815fe72c2 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java @@ -10,8 +10,11 @@ import io.micrometer.core.instrument.Gauge; import io.micrometer.core.instrument.Metrics; +import io.opentelemetry.instrumentation.test.utils.GcUtils; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import java.lang.ref.WeakReference; +import java.util.concurrent.atomic.AtomicLong; import org.assertj.core.api.AbstractIterableAssert; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -106,4 +109,35 @@ void gaugesWithSameNameAndDifferentTags() { .attributes() .containsOnly(attributeEntry("tag", "2"))))); } + + @Test + void testWeakRefGauge() throws InterruptedException { + // when + AtomicLong num = new AtomicLong(42); + Gauge.builder("testWeakRefGauge", num, AtomicLong::get) + .strongReference(false) + .register(Metrics.globalRegistry); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testWeakRefGauge", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDoubleGauge() + .points() + .satisfiesExactly(point -> assertThat(point).hasValue(42)))); + testing.clearData(); + + // when + WeakReference numWeakRef = new WeakReference<>(num); + num = null; + GcUtils.awaitGc(numWeakRef); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, "testWeakRefGauge", AbstractIterableAssert::isEmpty); + } } From bb6278cedf9e7d4102155c61a556cf4d14c8bd7b Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 17 Dec 2021 17:16:30 +0100 Subject: [PATCH 04/10] one more test --- .../micrometer/v1_5/TimerTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java index d4ff16b3e201..4853b32c3328 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java @@ -137,4 +137,36 @@ void testMicrometerHistogram() { .attributes() .containsEntry("le", "1000000")))); } + + @Test + void testMicrometerPercentiles() { + // given + Timer timer = + Timer.builder("testPercentiles") + .description("This is a test timer") + .tags("tag", "value") + .publishPercentiles(0.5, 0.95, 0.99) + .register(Metrics.globalRegistry); + + // when + timer.record(50, TimeUnit.MILLISECONDS); + timer.record(100, TimeUnit.MILLISECONDS); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testPercentiles.percentile", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasDoubleGauge() + .points() + .anySatisfy( + point -> assertThat(point).attributes().containsEntry("phi", "0.5")) + .anySatisfy( + point -> assertThat(point).attributes().containsEntry("phi", "0.95")) + .anySatisfy( + point -> assertThat(point).attributes().containsEntry("phi", "0.99")))); + } } From 8419ea540dd2cd4cad918a9447f85fdba7127f49 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 17 Dec 2021 17:23:22 +0100 Subject: [PATCH 05/10] disable by default + muzzle --- .../micrometer-1.5/javaagent/build.gradle.kts | 14 ++++++++++++++ .../v1_5/MicrometerInstrumentationModule.java | 6 ++++++ 2 files changed, 20 insertions(+) diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts b/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts index 0bd3875f4897..f5a1ce9f7b75 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts @@ -2,6 +2,20 @@ plugins { id("otel.javaagent-instrumentation") } +muzzle { + pass { + group.set("io.micrometer") + module.set("micrometer-core") + versions.set("[1.5.0,)") + assertInverse.set(true) + } +} + dependencies { library("io.micrometer:micrometer-core:1.5.0") } + +// TODO: disabled by default, since not all instruments are implemented +tasks.withType().configureEach { + jvmArgs("-Dotel.instrumentation.micrometer.enabled=true") +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java index 22e6e0096711..49dedf81c600 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerInstrumentationModule.java @@ -27,6 +27,12 @@ public ElementMatcher.Junction classLoaderMatcher() { return hasClassesNamed("io.micrometer.core.instrument.config.validate.Validated"); } + @Override + protected boolean defaultEnabled() { + // TODO: disabled by default, since not all instruments are implemented + return false; + } + @Override public List typeInstrumentations() { return Collections.singletonList(new MetricsInstrumentation()); From 4b386198d35c66355ae0e4fb6ffe50b6bba1b631 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 20 Dec 2021 10:04:16 +0100 Subject: [PATCH 06/10] code review comments --- .../v1_5/AsyncInstrumentRegistry.java | 72 +++++++++++++------ .../v1_5/MetricsInstrumentation.java | 2 +- .../micrometer/v1_5/MicrometerSingletons.java | 21 ++++++ .../micrometer/v1_5/OpenTelemetryCounter.java | 2 +- .../micrometer/v1_5/OpenTelemetryGauge.java | 8 +-- .../v1_5/OpenTelemetryMeterRegistry.java | 9 +-- .../micrometer/v1_5/OpenTelemetryTimer.java | 18 ++--- .../micrometer/v1_5/GaugeTest.java | 7 +- .../micrometer/v1_5/TimerTest.java | 2 +- 9 files changed, 97 insertions(+), 44 deletions(-) create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerSingletons.java diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java index 78aa0922ecdc..d24fb61603e1 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java @@ -5,53 +5,78 @@ package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.baseUnit; +import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.description; + import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import io.opentelemetry.instrumentation.api.internal.GuardedBy; import java.lang.ref.WeakReference; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Consumer; import java.util.function.ToDoubleFunction; +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; +@ThreadSafe final class AsyncInstrumentRegistry { private final Meter meter; - private final ConcurrentMap gauges = new ConcurrentHashMap<>(); + + @GuardedBy("gauges") + private final Map gauges = new HashMap<>(); AsyncInstrumentRegistry(Meter meter) { this.meter = meter; } void buildGauge( - String name, - String description, - String baseUnit, + io.micrometer.core.instrument.Meter.Id meterId, Attributes attributes, - T obj, + @Nullable T obj, ToDoubleFunction objMetric) { - gauges - .computeIfAbsent( - name, - n -> { - GaugeMeasurementsRecorder recorder = new GaugeMeasurementsRecorder(); - meter - .gaugeBuilder(name) - .setDescription(description) - .setUnit(baseUnit) - .buildWithCallback(recorder); - return recorder; - }) - .addGaugeMeasurement(attributes, obj, objMetric); + + GaugeMeasurementsRecorder recorder; + synchronized (gauges) { + recorder = + gauges.computeIfAbsent( + meterId.getName(), + n -> { + GaugeMeasurementsRecorder recorderCallback = new GaugeMeasurementsRecorder(); + meter + .gaugeBuilder(meterId.getName()) + .setDescription(description(meterId)) + .setUnit(baseUnit(meterId)) + .buildWithCallback(recorderCallback); + return recorderCallback; + }); + } + + // kept outside the synchronized block to avoid holding two locks at once + recorder.addGaugeMeasurement(attributes, obj, objMetric); } void removeGauge(String name, Attributes attributes) { - GaugeMeasurementsRecorder recorder = gauges.get(name); + GaugeMeasurementsRecorder recorder; + synchronized (gauges) { + recorder = gauges.get(name); + // if this is the last measurement then let's remove the whole recorder + if (recorder != null && recorder.size() == 1) { + gauges.remove(name); + } + } + + // kept outside the synchronized block to avoid holding two locks at once if (recorder != null) { recorder.removeGaugeMeasurement(attributes); } } + @ThreadSafe private static final class GaugeMeasurementsRecorder implements Consumer { @@ -68,13 +93,18 @@ public void accept(ObservableDoubleMeasurement measurement) { }); } - void addGaugeMeasurement(Attributes attributes, T obj, ToDoubleFunction objMetric) { + void addGaugeMeasurement( + Attributes attributes, @Nullable T obj, ToDoubleFunction objMetric) { measurements.put(attributes, new GaugeInfo(obj, (ToDoubleFunction) objMetric)); } void removeGaugeMeasurement(Attributes attributes) { measurements.remove(attributes); } + + int size() { + return measurements.size(); + } } private static final class GaugeInfo { @@ -82,7 +112,7 @@ private static final class GaugeInfo { private final WeakReference objWeakRef; private final ToDoubleFunction metricFunction; - private GaugeInfo(Object obj, ToDoubleFunction metricFunction) { + private GaugeInfo(@Nullable Object obj, ToDoubleFunction metricFunction) { this.objWeakRef = new WeakReference<>(obj); this.metricFunction = metricFunction; } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java index 326ddc9c6955..6460abf0721a 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MetricsInstrumentation.java @@ -32,7 +32,7 @@ public void transform(TypeTransformer transformer) { public static class StaticInitializerAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void onExit() { - Metrics.addRegistry(OpenTelemetryMeterRegistry.create()); + Metrics.addRegistry(MicrometerSingletons.meterRegistry()); } } } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerSingletons.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerSingletons.java new file mode 100644 index 000000000000..45bdec402858 --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerSingletons.java @@ -0,0 +1,21 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import io.micrometer.core.instrument.MeterRegistry; +import io.opentelemetry.api.GlobalOpenTelemetry; + +public final class MicrometerSingletons { + + private static final MeterRegistry METER_REGISTRY = + OpenTelemetryMeterRegistry.create(GlobalOpenTelemetry.get()); + + public static MeterRegistry meterRegistry() { + return METER_REGISTRY; + } + + private MicrometerSingletons() {} +} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java index feda877c0601..aa38bc1c9607 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java @@ -23,7 +23,7 @@ final class OpenTelemetryCounter implements Counter, RemovableMeter { private final DoubleCounter otelCounter; private final Attributes attributes; - volatile boolean removed = false; + private volatile boolean removed = false; OpenTelemetryCounter(Id id, Meter otelMeter) { this.id = id; diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java index e573f9aed76a..df6cee43eea8 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java @@ -5,8 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; -import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.baseUnit; -import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.description; import static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.toAttributes; import io.micrometer.core.instrument.Gauge; @@ -14,6 +12,7 @@ import io.opentelemetry.api.common.Attributes; import java.util.Collections; import java.util.function.ToDoubleFunction; +import javax.annotation.Nullable; final class OpenTelemetryGauge implements Gauge, RemovableMeter { @@ -23,15 +22,14 @@ final class OpenTelemetryGauge implements Gauge, RemovableMeter { OpenTelemetryGauge( Id id, - T obj, + @Nullable T obj, ToDoubleFunction objMetric, AsyncInstrumentRegistry asyncInstrumentRegistry) { this.id = id; this.attributes = toAttributes(id.getTags()); this.asyncInstrumentRegistry = asyncInstrumentRegistry; - asyncInstrumentRegistry.buildGauge( - id.getName(), description(id), baseUnit(id), attributes, obj, objMetric); + asyncInstrumentRegistry.buildGauge(id, attributes, obj, objMetric); } @Override diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java index 1e7e0faea927..c92deb4552d7 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java @@ -19,19 +19,20 @@ import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; import io.micrometer.core.instrument.distribution.HistogramGauges; import io.micrometer.core.instrument.distribution.pause.PauseDetector; -import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.OpenTelemetry; import java.util.concurrent.TimeUnit; import java.util.function.ToDoubleFunction; import java.util.function.ToLongFunction; +import javax.annotation.Nullable; public final class OpenTelemetryMeterRegistry extends MeterRegistry { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.micrometer-1.5"; - public static MeterRegistry create() { + public static MeterRegistry create(OpenTelemetry openTelemetry) { OpenTelemetryMeterRegistry openTelemetryMeterRegistry = new OpenTelemetryMeterRegistry( - Clock.SYSTEM, GlobalOpenTelemetry.get().getMeterProvider().get(INSTRUMENTATION_NAME)); + Clock.SYSTEM, openTelemetry.getMeterProvider().get(INSTRUMENTATION_NAME)); openTelemetryMeterRegistry.config().onMeterRemoved(OpenTelemetryMeterRegistry::onMeterRemoved); return openTelemetryMeterRegistry; } @@ -46,7 +47,7 @@ private OpenTelemetryMeterRegistry(Clock clock, io.opentelemetry.api.metrics.Met } @Override - protected Gauge newGauge(Meter.Id id, T t, ToDoubleFunction toDoubleFunction) { + protected Gauge newGauge(Meter.Id id, @Nullable T t, ToDoubleFunction toDoubleFunction) { return new OpenTelemetryGauge<>(id, t, toDoubleFunction, asyncInstrumentRegistry); } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java index 2f132dbcecbf..eb59d29927b2 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java @@ -21,7 +21,7 @@ import io.opentelemetry.api.metrics.Meter; import java.util.Collections; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.LongAdder; final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { @@ -30,7 +30,7 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { private final Attributes attributes; private final Measurements measurements; - volatile boolean removed = false; + private volatile boolean removed = false; OpenTelemetryTimer( Id id, @@ -44,7 +44,7 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { otelMeter .histogramBuilder(id.getName()) .setDescription(description(id)) - .setUnit("milliseconds") + .setUnit("ms") .ofLongs() .build(); this.attributes = toAttributes(id.getTags()); @@ -132,8 +132,8 @@ public double max(TimeUnit unit) { // kinda similar to how DropwizardTimer does that private static final class MicrometerHistogramMeasurements implements Measurements { - private final AtomicLong count = new AtomicLong(0); - private final AtomicLong totalTime = new AtomicLong(0); + private final LongAdder count = new LongAdder(); + private final LongAdder totalTime = new LongAdder(); private final TimeWindowMax max; MicrometerHistogramMeasurements( @@ -144,19 +144,19 @@ private static final class MicrometerHistogramMeasurements implements Measuremen @Override public void record(long amount, TimeUnit unit) { long nanos = unit.toNanos(amount); - count.incrementAndGet(); - totalTime.addAndGet(nanos); + count.increment(); + totalTime.add(nanos); max.record(nanos, TimeUnit.NANOSECONDS); } @Override public long count() { - return count.get(); + return count.sum(); } @Override public double totalTime(TimeUnit unit) { - return TimeUtils.nanosToUnit(totalTime.get(), unit); + return TimeUtils.nanosToUnit(totalTime.sum(), unit); } @Override diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java index f1c815fe72c2..221f2bdd0787 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/GaugeTest.java @@ -33,7 +33,7 @@ void cleanupMeters() { } @Test - void testGauge() { + void testGauge() throws Exception { // when Gauge gauge = Gauge.builder("testGauge", () -> 42) @@ -60,12 +60,14 @@ void testGauge() { .hasValue(42) .attributes() .containsOnly(attributeEntry("tag", "value"))))); - testing.clearData(); // when Metrics.globalRegistry.remove(gauge); + Thread.sleep(10); // give time for any inflight metric export to be received + testing.clearData(); // then + Thread.sleep(100); // interval of the test metrics exporter testing.waitAndAssertMetrics( INSTRUMENTATION_NAME, "testGauge", AbstractIterableAssert::isEmpty); } @@ -137,6 +139,7 @@ void testWeakRefGauge() throws InterruptedException { GcUtils.awaitGc(numWeakRef); // then + Thread.sleep(100); // interval of the test metrics exporter testing.waitAndAssertMetrics( INSTRUMENTATION_NAME, "testWeakRefGauge", AbstractIterableAssert::isEmpty); } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java index 4853b32c3328..ac6b737254ba 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java @@ -52,7 +52,7 @@ void testTimer() { metric -> assertThat(metric) .hasDescription("This is a test timer") - .hasUnit("milliseconds") + .hasUnit("ms") .hasDoubleHistogram() .points() .satisfiesExactly( From 7f8bbd3bda92e430731efeb7b8ab733f904e2411 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 20 Dec 2021 10:12:25 +0100 Subject: [PATCH 07/10] log one-time warning --- .../micrometer/v1_5/OpenTelemetryCounter.java | 4 ++-- .../micrometer/v1_5/OpenTelemetryGauge.java | 4 ++-- .../micrometer/v1_5/OpenTelemetryTimer.java | 5 +++- .../v1_5/UnsupportedReadLogger.java | 24 +++++++++++++++++++ 4 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java index aa38bc1c9607..5b86ba815711 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java @@ -47,13 +47,13 @@ public void increment(double v) { @Override public double count() { - // OpenTelemetry metrics bridge does not support reading measurements + UnsupportedReadLogger.logWarning(); return Double.NaN; } @Override public Iterable measure() { - // OpenTelemetry metrics bridge does not support reading measurements + UnsupportedReadLogger.logWarning(); return Collections.emptyList(); } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java index df6cee43eea8..0e227e0af264 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryGauge.java @@ -34,13 +34,13 @@ final class OpenTelemetryGauge implements Gauge, RemovableMeter { @Override public double value() { - // OpenTelemetry metrics bridge does not support reading measurements + UnsupportedReadLogger.logWarning(); return Double.NaN; } @Override public Iterable measure() { - // OpenTelemetry metrics bridge does not support reading measurements + UnsupportedReadLogger.logWarning(); return Collections.emptyList(); } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java index eb59d29927b2..d5a1e6449043 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java @@ -85,7 +85,7 @@ public double max(TimeUnit unit) { @Override public Iterable measure() { - // OpenTelemetry metrics bridge does not support reading measurements + UnsupportedReadLogger.logWarning(); return Collections.emptyList(); } @@ -114,16 +114,19 @@ public void record(long amount, TimeUnit unit) {} @Override public long count() { + UnsupportedReadLogger.logWarning(); return 0; } @Override public double totalTime(TimeUnit unit) { + UnsupportedReadLogger.logWarning(); return Double.NaN; } @Override public double max(TimeUnit unit) { + UnsupportedReadLogger.logWarning(); return Double.NaN; } } diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java new file mode 100644 index 000000000000..3e24ed221e3a --- /dev/null +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; + +import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +final class UnsupportedReadLogger { + + private static final Logger logger = LoggerFactory.getLogger(OpenTelemetryMeterRegistry.class); + private static final AtomicBoolean done = new AtomicBoolean(false); + + static void logWarning() { + if (done.compareAndSet(false, true)) { + logger.warn("OpenTelemetry metrics bridge does not support reading measurements"); + } + } + + private UnsupportedReadLogger() {} +} From 1a9fe562a21c00d1364ca232a2818d77907ec69b Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 20 Dec 2021 15:25:36 +0100 Subject: [PATCH 08/10] make AsyncInstrumentRegistry actually thread safe --- .../v1_5/AsyncInstrumentRegistry.java | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java index d24fb61603e1..75d871a183b3 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java @@ -15,14 +15,10 @@ import java.lang.ref.WeakReference; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.function.Consumer; import java.util.function.ToDoubleFunction; import javax.annotation.Nullable; -import javax.annotation.concurrent.ThreadSafe; -@ThreadSafe final class AsyncInstrumentRegistry { private final Meter meter; @@ -40,9 +36,8 @@ void buildGauge( @Nullable T obj, ToDoubleFunction objMetric) { - GaugeMeasurementsRecorder recorder; synchronized (gauges) { - recorder = + GaugeMeasurementsRecorder recorder = gauges.computeIfAbsent( meterId.getName(), n -> { @@ -54,37 +49,36 @@ void buildGauge( .buildWithCallback(recorderCallback); return recorderCallback; }); + recorder.addGaugeMeasurement(attributes, obj, objMetric); } - - // kept outside the synchronized block to avoid holding two locks at once - recorder.addGaugeMeasurement(attributes, obj, objMetric); } void removeGauge(String name, Attributes attributes) { - GaugeMeasurementsRecorder recorder; synchronized (gauges) { - recorder = gauges.get(name); - // if this is the last measurement then let's remove the whole recorder - if (recorder != null && recorder.size() == 1) { - gauges.remove(name); + GaugeMeasurementsRecorder recorder = gauges.get(name); + if (recorder != null) { + recorder.removeGaugeMeasurement(attributes); + // if this was the last measurement then let's remove the whole recorder + if (recorder.isEmpty()) { + gauges.remove(name); + } } } - - // kept outside the synchronized block to avoid holding two locks at once - if (recorder != null) { - recorder.removeGaugeMeasurement(attributes); - } } - @ThreadSafe - private static final class GaugeMeasurementsRecorder - implements Consumer { + private final class GaugeMeasurementsRecorder implements Consumer { - private final ConcurrentMap measurements = new ConcurrentHashMap<>(); + @GuardedBy("gauges") + private final Map measurements = new HashMap<>(); @Override public void accept(ObservableDoubleMeasurement measurement) { - measurements.forEach( + Map measurementsCopy; + synchronized (gauges) { + measurementsCopy = new HashMap<>(measurements); + } + + measurementsCopy.forEach( (attributes, gauge) -> { Object obj = gauge.objWeakRef.get(); if (obj != null) { @@ -95,15 +89,21 @@ public void accept(ObservableDoubleMeasurement measurement) { void addGaugeMeasurement( Attributes attributes, @Nullable T obj, ToDoubleFunction objMetric) { - measurements.put(attributes, new GaugeInfo(obj, (ToDoubleFunction) objMetric)); + synchronized (gauges) { + measurements.put(attributes, new GaugeInfo(obj, (ToDoubleFunction) objMetric)); + } } void removeGaugeMeasurement(Attributes attributes) { - measurements.remove(attributes); + synchronized (gauges) { + measurements.remove(attributes); + } } - int size() { - return measurements.size(); + boolean isEmpty() { + synchronized (gauges) { + return measurements.isEmpty(); + } } } From 88457a01dbde34430e41c6042241eca524bccefe Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 21 Dec 2021 09:06:44 +0100 Subject: [PATCH 09/10] code review comments --- .../micrometer/v1_5/Bridging.java | 2 +- .../micrometer/v1_5/OpenTelemetryTimer.java | 24 ++++++++++++------- .../v1_5/UnsupportedReadLogger.java | 11 ++++----- .../micrometer/v1_5/TimerTest.java | 23 ++++++++++++++++++ 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java index d198691bdc14..7a3be0b7ef56 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java @@ -14,7 +14,7 @@ final class Bridging { - private static final Cache> tagsCache = Cache.bounded(64); + private static final Cache> tagsCache = Cache.bounded(1024); static Attributes toAttributes(Iterable tags) { if (!tags.iterator().hasNext()) { diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java index d5a1e6449043..e3b89fc7f43f 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java @@ -17,7 +17,7 @@ import io.micrometer.core.instrument.distribution.pause.PauseDetector; import io.micrometer.core.instrument.util.TimeUtils; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.metrics.LongHistogram; +import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.Meter; import java.util.Collections; import java.util.concurrent.TimeUnit; @@ -25,8 +25,10 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { + private static final long NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1); + // TODO: use bound instruments when they're available - private final LongHistogram otelHistogram; + private final DoubleHistogram otelHistogram; private final Attributes attributes; private final Measurements measurements; @@ -45,7 +47,6 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { .histogramBuilder(id.getName()) .setDescription(description(id)) .setUnit("ms") - .ofLongs() .build(); this.attributes = toAttributes(id.getTags()); @@ -63,8 +64,14 @@ boolean isUsingMicrometerHistograms() { @Override protected void recordNonNegative(long amount, TimeUnit unit) { if (amount >= 0 && !removed) { - otelHistogram.record(unit.toMillis(amount), attributes); - measurements.record(amount, unit); + long nanos = unit.toNanos(amount); + + long millisPart = nanos / NANOS_PER_MS; + long nanosPart = nanos % NANOS_PER_MS; + double time = millisPart + nanosPart / (double) NANOS_PER_MS; + + otelHistogram.record(time, attributes); + measurements.record(nanos); } } @@ -95,7 +102,7 @@ public void onRemove() { } private interface Measurements { - void record(long amount, TimeUnit unit); + void record(long nanos); long count(); @@ -110,7 +117,7 @@ enum NoopMeasurements implements Measurements { INSTANCE; @Override - public void record(long amount, TimeUnit unit) {} + public void record(long nanos) {} @Override public long count() { @@ -145,8 +152,7 @@ private static final class MicrometerHistogramMeasurements implements Measuremen } @Override - public void record(long amount, TimeUnit unit) { - long nanos = unit.toNanos(amount); + public void record(long nanos) { count.increment(); totalTime.add(nanos); max.record(nanos, TimeUnit.NANOSECONDS); diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java index 3e24ed221e3a..de9ae1d5178f 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/UnsupportedReadLogger.java @@ -5,19 +5,18 @@ package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5; -import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; import org.slf4j.LoggerFactory; final class UnsupportedReadLogger { - private static final Logger logger = LoggerFactory.getLogger(OpenTelemetryMeterRegistry.class); - private static final AtomicBoolean done = new AtomicBoolean(false); + static { + Logger logger = LoggerFactory.getLogger(OpenTelemetryMeterRegistry.class); + logger.warn("OpenTelemetry metrics bridge does not support reading measurements"); + } static void logWarning() { - if (done.compareAndSet(false, true)) { - logger.warn("OpenTelemetry metrics bridge does not support reading measurements"); - } + // do nothing; the warning will be logged exactly once when this class is loaded } private UnsupportedReadLogger() {} diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java index ac6b737254ba..545e807ca193 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerTest.java @@ -81,6 +81,29 @@ void testTimer() { .noneSatisfy(point -> assertThat(point).hasSum(54_000).hasCount(2)))); } + @Test + void testNanoPrecision() { + // given + Timer timer = Timer.builder("testNanoTimer").register(Metrics.globalRegistry); + + // when + timer.record(1_234_000, TimeUnit.NANOSECONDS); + + // then + testing.waitAndAssertMetrics( + INSTRUMENTATION_NAME, + "testNanoTimer", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasUnit("ms") + .hasDoubleHistogram() + .points() + .satisfiesExactly( + point -> assertThat(point).hasSum(1.234).hasCount(1).attributes()))); + } + @Test void testMicrometerHistogram() { // given From aee177948d463e67a47450804e82880c3f85d1c9 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 3 Jan 2022 11:01:53 +0100 Subject: [PATCH 10/10] one more minor fix --- .../micrometer/v1_5/OpenTelemetryTimer.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java index e3b89fc7f43f..a0b9044d9fc2 100644 --- a/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java +++ b/instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java @@ -25,7 +25,7 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter { - private static final long NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1); + private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1); // TODO: use bound instruments when they're available private final DoubleHistogram otelHistogram; @@ -65,11 +65,7 @@ boolean isUsingMicrometerHistograms() { protected void recordNonNegative(long amount, TimeUnit unit) { if (amount >= 0 && !removed) { long nanos = unit.toNanos(amount); - - long millisPart = nanos / NANOS_PER_MS; - long nanosPart = nanos % NANOS_PER_MS; - double time = millisPart + nanosPart / (double) NANOS_PER_MS; - + double time = nanos / NANOS_PER_MS; otelHistogram.record(time, attributes); measurements.record(nanos); }