Disable by default gauge-based histograms in the Micrometer bridge

open-telemetry · Apr 25, 2023 · 2e02a1e · 2e02a1e
1 parent 1a68f4d
commit 2e02a1e
Show file tree

Hide file tree

Showing 17 changed files with 636 additions and 343 deletions.
diff --git a/instrumentation/micrometer/micrometer-1.5/README.md b/instrumentation/micrometer/micrometer-1.5/README.md
@@ -1,6 +1,7 @@
 # Settings for the Micrometer bridge instrumentation
 
-| System property | Type | Default | Description |
-|---|---|---|---|
-| `otel.instrumentation.micrometer.base-time-unit`          | String  | `ms`  | Set the base time unit for the OpenTelemetry `MeterRegistry` implementation. <details><summary>Valid values</summary>`ns`, `nanoseconds`, `us`, `microseconds`, `ms`, `microseconds`, `s`, `seconds`, `min`, `minutes`, `h`, `hours`, `d`, `days`</details> |
-| `otel.instrumentation.micrometer.prometheus-mode.enabled` | boolean | false | Enable the "Prometheus mode" this will simulate the behavior of Micrometer's PrometheusMeterRegistry. The instruments will be renamed to match Micrometer instrument naming, and the base time unit will be set to seconds. |
+| System property                                            | Type    | Default | Description                                                                                                                                                                                                                                                 |
+|------------------------------------------------------------|---------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `otel.instrumentation.micrometer.base-time-unit`           | String  | `ms`    | Set the base time unit for the OpenTelemetry `MeterRegistry` implementation. <details><summary>Valid values</summary>`ns`, `nanoseconds`, `us`, `microseconds`, `ms`, `microseconds`, `s`, `seconds`, `min`, `minutes`, `h`, `hours`, `d`, `days`</details> |
+| `otel.instrumentation.micrometer.prometheus-mode.enabled`  | boolean | false   | Enable the "Prometheus mode" this will simulate the behavior of Micrometer's PrometheusMeterRegistry. The instruments will be renamed to match Micrometer instrument naming, and the base time unit will be set to seconds.                                 |
+| `otel.instrumentation.micrometer.histogram-gauges.enabled` | boolean | false   | Enables the generation of gauge-based Micrometer histograms for `DistributionSummary` and `Timer` instruments.                                                                                                                                              |
diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts b/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts
@@ -36,15 +36,25 @@ tasks {
     jvmArgs("-Dotel.instrumentation.micrometer.base-time-unit=seconds")
   }
 
+  val testHistogramGauges by registering(Test::class) {
+    filter {
+      includeTestsMatching("*HistogramGaugesTest")
+    }
+    include("**/*HistogramGaugesTest.*")
+    jvmArgs("-Dotel.instrumentation.micrometer.histogram-gauges.enabled=true")
+  }
+
   test {
     filter {
       excludeTestsMatching("*TimerSecondsTest")
       excludeTestsMatching("*PrometheusModeTest")
+      excludeTestsMatching("*HistogramGaugesTest")
     }
   }
 
   check {
     dependsOn(testBaseTimeUnit)
     dependsOn(testPrometheusMode)
+    dependsOn(testHistogramGauges)
   }
 }
diff --git a/...java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerSingletons.java b/...java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/MicrometerSingletons.java
@@ -23,6 +23,9 @@ public final class MicrometerSingletons {
             .setBaseTimeUnit(
                 TimeUnitParser.parseConfigValue(
                     config.getString("otel.instrumentation.micrometer.base-time-unit")))
+            .setMicrometerHistogramGaugesEnabled(
+                config.getBoolean(
+                    "otel.instrumentation.micrometer.histogram-gauges.enabled", false))
             .build();
   }
 

diff --git a/...try/javaagent/instrumentation/micrometer/v1_5/DistributionSummaryHistogramGaugesTest.java b/...try/javaagent/instrumentation/micrometer/v1_5/DistributionSummaryHistogramGaugesTest.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5;
+
+import io.micrometer.core.instrument.Metrics;
+import io.opentelemetry.instrumentation.micrometer.v1_5.AbstractDistributionSummaryHistogramGaugesTest;
+import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class DistributionSummaryHistogramGaugesTest
+    extends AbstractDistributionSummaryHistogramGaugesTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+
+  @AfterEach
+  public void cleanup() {
+    Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove);
+  }
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}
diff --git a/.../io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerHistogramGaugesTest.java b/.../io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/TimerHistogramGaugesTest.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5;
+
+import io.micrometer.core.instrument.Metrics;
+import io.opentelemetry.instrumentation.micrometer.v1_5.AbstractTimerHistogramGaugesTest;
+import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class TimerHistogramGaugesTest extends AbstractTimerHistogramGaugesTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+
+  @AfterEach
+  public void cleanup() {
+    Metrics.globalRegistry.forEachMeter(Metrics.globalRegistry::remove);
+  }
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}
diff --git a/...io/opentelemetry/instrumentation/micrometer/v1_5/DistributionStatisticConfigModifier.java b/...io/opentelemetry/instrumentation/micrometer/v1_5/DistributionStatisticConfigModifier.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.micrometer.v1_5;
+
+import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
+
+enum DistributionStatisticConfigModifier {
+  DISABLE_HISTOGRAM_GAUGES {
+    @Override
+    DistributionStatisticConfig modify(DistributionStatisticConfig config) {
+      return DistributionStatisticConfig.builder()
+          // disable all percentile and slo related options
+          .percentilesHistogram(null)
+          .percentiles((double[]) null)
+          .serviceLevelObjectives((double[]) null)
+          .percentilePrecision(null)
+          // and keep the rest
+          .minimumExpectedValue(config.getMinimumExpectedValueAsDouble())
+          .maximumExpectedValue(config.getMaximumExpectedValueAsDouble())
+          .expiry(config.getExpiry())
+          .bufferLength(config.getBufferLength())
+          .build();
+    }
+  },
+  IDENTITY {
+    @Override
+    DistributionStatisticConfig modify(DistributionStatisticConfig config) {
+      return config;
+    }
+  };
+
+  abstract DistributionStatisticConfig modify(DistributionStatisticConfig config);
+}
diff --git a/...va/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryDistributionSummary.java b/...va/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryDistributionSummary.java
@@ -41,9 +41,10 @@ final class OpenTelemetryDistributionSummary extends AbstractDistributionSummary
       NamingConvention namingConvention,
       Clock clock,
       DistributionStatisticConfig distributionStatisticConfig,
+      DistributionStatisticConfigModifier modifier,
       double scale,
       Meter otelMeter) {
-    super(id, clock, distributionStatisticConfig, scale, false);
+    super(id, clock, modifier.modify(distributionStatisticConfig), scale, false);
 
     if (isUsingMicrometerHistograms()) {
       measurements = new MicrometerHistogramMeasurements();

diff --git a/...ain/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java b/...ain/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java
@@ -50,15 +50,18 @@ public static OpenTelemetryMeterRegistryBuilder builder(OpenTelemetry openTeleme
   }
 
   private final TimeUnit baseTimeUnit;
+  private final DistributionStatisticConfigModifier distributionStatisticConfigModifier;
   private final io.opentelemetry.api.metrics.Meter otelMeter;
 
   OpenTelemetryMeterRegistry(
       Clock clock,
       TimeUnit baseTimeUnit,
       NamingConvention namingConvention,
+      DistributionStatisticConfigModifier distributionStatisticConfigModifier,
       io.opentelemetry.api.metrics.Meter otelMeter) {
     super(clock);
     this.baseTimeUnit = baseTimeUnit;
+    this.distributionStatisticConfigModifier = distributionStatisticConfigModifier;
     this.otelMeter = otelMeter;
 
     this.config()
@@ -104,6 +107,7 @@ protected Timer newTimer(
             config().namingConvention(),
             clock,
             distributionStatisticConfig,
+            distributionStatisticConfigModifier,
             pauseDetector,
             getBaseTimeUnit(),
             otelMeter);
@@ -118,7 +122,13 @@ protected DistributionSummary newDistributionSummary(
       Meter.Id id, DistributionStatisticConfig distributionStatisticConfig, double scale) {
     OpenTelemetryDistributionSummary distributionSummary =
         new OpenTelemetryDistributionSummary(
-            id, config().namingConvention(), clock, distributionStatisticConfig, scale, otelMeter);
+            id,
+            config().namingConvention(),
+            clock,
+            distributionStatisticConfig,
+            distributionStatisticConfigModifier,
+            scale,
+            otelMeter);
     if (distributionSummary.isUsingMicrometerHistograms()) {
       HistogramGauges.registerWithCommonFormat(distributionSummary, this);
     }

diff --git a/...a/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistryBuilder.java b/...a/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistryBuilder.java
@@ -7,7 +7,10 @@
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import io.micrometer.core.instrument.Clock;
+import io.micrometer.core.instrument.DistributionSummary;
+import io.micrometer.core.instrument.LongTaskTimer;
 import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Timer;
 import io.micrometer.core.instrument.config.NamingConvention;
 import io.opentelemetry.api.OpenTelemetry;
 import java.util.concurrent.TimeUnit;
@@ -22,6 +25,7 @@ public final class OpenTelemetryMeterRegistryBuilder {
   private Clock clock = Clock.SYSTEM;
   private TimeUnit baseTimeUnit = TimeUnit.MILLISECONDS;
   private boolean prometheusMode = false;
+  private boolean histogramGaugesEnabled = false;
 
   OpenTelemetryMeterRegistryBuilder(OpenTelemetry openTelemetry) {
     this.openTelemetry = openTelemetry;
@@ -54,6 +58,27 @@ public OpenTelemetryMeterRegistryBuilder setPrometheusMode(boolean prometheusMod
     return this;
   }
 
+  /**
+   * Enables the generation of gauge-based Micrometer histograms. While the Micrometer bridge is
+   * able to map Micrometer's {@link DistributionSummary} and {@link Timer} service level objectives
+   * to OpenTelemetry histogram buckets, it might not cover all cases that are normally supported by
+   * Micrometer (e.g. the bridge is not able to translate percentiles). With this setting enabled,
+   * the Micrometer bridge will additionally emit Micrometer service level objectives and
+   * percentiles as separate gauges.
+   *
+   * <p>Note that this setting does not concern the {@link LongTaskTimer}, as it is not bridged to
+   * an OpenTelemetry histogram.
+   *
+   * <p>This is disabled by default, set this to {@code true} to enable gauge-based Micrometer
+   * histograms.
+   */
+  @CanIgnoreReturnValue
+  public OpenTelemetryMeterRegistryBuilder setMicrometerHistogramGaugesEnabled(
+      boolean histogramGaugesEnabled) {
+    this.histogramGaugesEnabled = histogramGaugesEnabled;
+    return this;
+  }
+
   /**
    * Returns a new {@link OpenTelemetryMeterRegistry} with the settings of this {@link
    * OpenTelemetryMeterRegistryBuilder}.
@@ -63,11 +88,16 @@ public MeterRegistry build() {
     TimeUnit baseTimeUnit = prometheusMode ? TimeUnit.SECONDS : this.baseTimeUnit;
     NamingConvention namingConvention =
         prometheusMode ? PrometheusModeNamingConvention.INSTANCE : NamingConvention.identity;
+    DistributionStatisticConfigModifier modifier =
+        histogramGaugesEnabled
+            ? DistributionStatisticConfigModifier.IDENTITY
+            : DistributionStatisticConfigModifier.DISABLE_HISTOGRAM_GAUGES;
 
     return new OpenTelemetryMeterRegistry(
         clock,
         baseTimeUnit,
         namingConvention,
+        modifier,
         openTelemetry.getMeterProvider().get(INSTRUMENTATION_NAME));
   }
 }
diff --git a/...ry/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java b/...ry/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryTimer.java
@@ -43,10 +43,17 @@ final class OpenTelemetryTimer extends AbstractTimer implements RemovableMeter {
       NamingConvention namingConvention,
       Clock clock,
       DistributionStatisticConfig distributionStatisticConfig,
+      DistributionStatisticConfigModifier modifier,
       PauseDetector pauseDetector,
       TimeUnit baseTimeUnit,
       Meter otelMeter) {
-    super(id, clock, distributionStatisticConfig, pauseDetector, TimeUnit.MILLISECONDS, false);
+    super(
+        id,
+        clock,
+        modifier.modify(distributionStatisticConfig),
+        pauseDetector,
+        baseTimeUnit,
+        false);
 
     if (isUsingMicrometerHistograms()) {
       measurements = new MicrometerHistogramMeasurements();

diff --git a/...opentelemetry/instrumentation/micrometer/v1_5/DistributionSummaryHistogramGaugesTest.java b/...opentelemetry/instrumentation/micrometer/v1_5/DistributionSummaryHistogramGaugesTest.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.micrometer.v1_5;
+
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class DistributionSummaryHistogramGaugesTest
+    extends AbstractDistributionSummaryHistogramGaugesTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
+
+  @RegisterExtension
+  static final MicrometerTestingExtension micrometerExtension =
+      new MicrometerTestingExtension(testing) {
+        @Override
+        OpenTelemetryMeterRegistryBuilder configureOtelRegistry(
+            OpenTelemetryMeterRegistryBuilder registry) {
+          return registry.setMicrometerHistogramGaugesEnabled(true);
+        }
+      };
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}
diff --git a/.../src/test/java/io/opentelemetry/instrumentation/micrometer/v1_5/NamingConventionTest.java b/.../src/test/java/io/opentelemetry/instrumentation/micrometer/v1_5/NamingConventionTest.java
@@ -10,7 +10,7 @@
 import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-public class NamingConventionTest extends AbstractNamingConventionTest {
+class NamingConventionTest extends AbstractNamingConventionTest {
 
   @RegisterExtension
   static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();

diff --git a/.../test/java/io/opentelemetry/instrumentation/micrometer/v1_5/TimerHistogramGaugesTest.java b/.../test/java/io/opentelemetry/instrumentation/micrometer/v1_5/TimerHistogramGaugesTest.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.micrometer.v1_5;
+
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class TimerHistogramGaugesTest extends AbstractTimerHistogramGaugesTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
+
+  @RegisterExtension
+  static final MicrometerTestingExtension micrometerExtension =
+      new MicrometerTestingExtension(testing) {
+        @Override
+        OpenTelemetryMeterRegistryBuilder configureOtelRegistry(
+            OpenTelemetryMeterRegistryBuilder registry) {
+          return registry.setMicrometerHistogramGaugesEnabled(true);
+        }
+      };
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}