Implement "Prometheus mode" for better micrometer->OTel->Prometheus s…

…upport (#5537) * Implement "Prometheus mode" for better micrometer->OTel->Prometheus support * code review comments * forgot about README
open-telemetry · Mar 18, 2022 · ab587a9 · ab587a9
1 parent e20f295
commit ab587a9
Show file tree

Hide file tree

Showing 11 changed files with 516 additions and 16 deletions.
diff --git a/instrumentation/micrometer/micrometer-1.5/README.md b/instrumentation/micrometer/micrometer-1.5/README.md
@@ -2,4 +2,5 @@
 
 | 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.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. |
diff --git a/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts b/instrumentation/micrometer/micrometer-1.5/javaagent/build.gradle.kts
@@ -20,6 +20,14 @@ dependencies {
 }
 
 tasks {
+  val testPrometheusMode by registering(Test::class) {
+    filter {
+      includeTestsMatching("*PrometheusModeTest")
+    }
+    include("**/*PrometheusModeTest.*")
+    jvmArgs("-Dotel.instrumentation.micrometer.prometheus-mode.enabled=true")
+  }
+
   val testBaseTimeUnit by registering(Test::class) {
     filter {
       includeTestsMatching("*TimerSecondsTest")
@@ -31,10 +39,12 @@ tasks {
   test {
     filter {
       excludeTestsMatching("*TimerSecondsTest")
+      excludeTestsMatching("*PrometheusModeTest")
     }
   }
 
   check {
     dependsOn(testBaseTimeUnit)
+    dependsOn(testPrometheusMode)
   }
 }
diff --git a/...t/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/PrometheusModeTest.java b/...t/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/PrometheusModeTest.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.micrometer.v1_5;
+
+import io.opentelemetry.instrumentation.micrometer.v1_5.AbstractPrometheusModeTest;
+import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class PrometheusModeTest extends AbstractPrometheusModeTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}
diff --git a/...va/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryDistributionSummary.java b/...va/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryDistributionSummary.java
@@ -12,7 +12,6 @@
 
 import io.micrometer.core.instrument.AbstractDistributionSummary;
 import io.micrometer.core.instrument.Clock;
-import io.micrometer.core.instrument.DistributionSummary;
 import io.micrometer.core.instrument.Measurement;
 import io.micrometer.core.instrument.config.NamingConvention;
 import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
@@ -27,7 +26,7 @@
 import java.util.concurrent.atomic.LongAdder;
 
 final class OpenTelemetryDistributionSummary extends AbstractDistributionSummary
-    implements DistributionSummary, RemovableMeter {
+    implements RemovableMeter {
 
   private final Measurements measurements;
   private final TimeWindowMax max;
@@ -78,7 +77,7 @@ boolean isUsingMicrometerHistograms() {
 
   @Override
   protected void recordNonNegative(double amount) {
-    if (amount >= 0 && !removed) {
+    if (!removed) {
       otelHistogram.record(amount, attributes);
       measurements.record(amount);
       max.record(amount);

diff --git a/...ain/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java b/...ain/java/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistry.java
@@ -53,13 +53,18 @@ public static OpenTelemetryMeterRegistryBuilder builder(OpenTelemetry openTeleme
   private final io.opentelemetry.api.metrics.Meter otelMeter;
 
   OpenTelemetryMeterRegistry(
-      Clock clock, TimeUnit baseTimeUnit, io.opentelemetry.api.metrics.Meter otelMeter) {
+      Clock clock,
+      TimeUnit baseTimeUnit,
+      boolean prometheusMode,
+      io.opentelemetry.api.metrics.Meter otelMeter) {
     super(clock);
     this.baseTimeUnit = baseTimeUnit;
     this.otelMeter = otelMeter;
 
+    NamingConvention namingConvention =
+        prometheusMode ? PrometheusModeNamingConvention.INSTANCE : NamingConvention.identity;
     this.config()
-        .namingConvention(NamingConvention.identity)
+        .namingConvention(namingConvention)
         .onMeterRemoved(OpenTelemetryMeterRegistry::onMeterRemoved);
   }
 

diff --git a/...a/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistryBuilder.java b/...a/io/opentelemetry/instrumentation/micrometer/v1_5/OpenTelemetryMeterRegistryBuilder.java
@@ -10,6 +10,7 @@
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.api.config.Config;
 import java.util.concurrent.TimeUnit;
+import javax.annotation.Nullable;
 
 /** A builder of {@link OpenTelemetryMeterRegistry}. */
 public final class OpenTelemetryMeterRegistryBuilder {
@@ -18,9 +19,9 @@ public final class OpenTelemetryMeterRegistryBuilder {
 
   private final OpenTelemetry openTelemetry;
   private Clock clock = Clock.SYSTEM;
-  private TimeUnit baseTimeUnit =
-      TimeUnitHelper.parseConfigValue(
-          Config.get().getString("otel.instrumentation.micrometer.base-time-unit"));
+  @Nullable private TimeUnit baseTimeUnit = null;
+  private boolean prometheusMode =
+      Config.get().getBoolean("otel.instrumentation.micrometer.prometheus-mode.enabled", false);
 
   OpenTelemetryMeterRegistryBuilder(OpenTelemetry openTelemetry) {
     this.openTelemetry = openTelemetry;
@@ -38,12 +39,38 @@ public OpenTelemetryMeterRegistryBuilder setBaseTimeUnit(TimeUnit baseTimeUnit)
     return this;
   }
 
+  /**
+   * Enables the "Prometheus mode" - this will simulate the behavior of Micrometer's {@code
+   * PrometheusMeterRegistry}. The instruments will be renamed to match Micrometer instrument
+   * naming, and the base time unit will be set to seconds.
+   *
+   * <p>Set this to {@code true} if you are using the Prometheus metrics exporter.
+   */
+  public OpenTelemetryMeterRegistryBuilder setPrometheusMode(boolean prometheusMode) {
+    this.prometheusMode = prometheusMode;
+    return this;
+  }
+
   /**
    * Returns a new {@link OpenTelemetryMeterRegistry} with the settings of this {@link
    * OpenTelemetryMeterRegistryBuilder}.
    */
   public MeterRegistry build() {
+    if (prometheusMode) {
+      // prometheus mode overrides any unit settings with SECONDS
+      setBaseTimeUnit(TimeUnit.SECONDS);
+    } else if (baseTimeUnit == null) {
+      // if the unit was not manually set, try to initialize it using config
+      setBaseTimeUnit(
+          TimeUnitHelper.parseConfigValue(
+              Config.get().getString("otel.instrumentation.micrometer.base-time-unit"),
+              TimeUnit.MILLISECONDS));
+    }
+
     return new OpenTelemetryMeterRegistry(
-        clock, baseTimeUnit, openTelemetry.getMeterProvider().get(INSTRUMENTATION_NAME));
+        clock,
+        baseTimeUnit,
+        prometheusMode,
+        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
@@ -81,7 +81,7 @@ boolean isUsingMicrometerHistograms() {
 
   @Override
   protected void recordNonNegative(long amount, TimeUnit unit) {
-    if (amount >= 0 && !removed) {
+    if (!removed) {
       long nanos = unit.toNanos(amount);
       double time = TimeUtils.nanosToUnit(nanos, baseTimeUnit);
       otelHistogram.record(time, attributes);

diff --git a/...java/io/opentelemetry/instrumentation/micrometer/v1_5/PrometheusModeNamingConvention.java b/...java/io/opentelemetry/instrumentation/micrometer/v1_5/PrometheusModeNamingConvention.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.Meter;
+import io.micrometer.core.instrument.config.NamingConvention;
+import javax.annotation.Nullable;
+
+// This naming strategy does not replace '.' with '_', and it does not append '_total' to counter
+// names - the reason behind it is that this is already done by the Prometheus exporter; see the
+// io.opentelemetry.exporter.prometheus.MetricAdapter class
+enum PrometheusModeNamingConvention implements NamingConvention {
+  INSTANCE;
+
+  @Override
+  public String name(String name, Meter.Type type, @Nullable String baseUnit) {
+    if (type == Meter.Type.COUNTER
+        || type == Meter.Type.DISTRIBUTION_SUMMARY
+        || type == Meter.Type.GAUGE) {
+      if (baseUnit != null && !name.endsWith("." + baseUnit)) {
+        name = name + "." + baseUnit;
+      }
+    }
+
+    if (type == Meter.Type.LONG_TASK_TIMER || type == Meter.Type.TIMER) {
+      if (!name.endsWith(".seconds")) {
+        name = name + ".seconds";
+      }
+    }
+
+    return name;
+  }
+}
diff --git a/...ibrary/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/TimeUnitHelper.java b/...ibrary/src/main/java/io/opentelemetry/instrumentation/micrometer/v1_5/TimeUnitHelper.java
@@ -15,9 +15,9 @@ final class TimeUnitHelper {
 
   private static final Logger logger = LoggerFactory.getLogger(OpenTelemetryMeterRegistry.class);
 
-  static TimeUnit parseConfigValue(@Nullable String value) {
+  static TimeUnit parseConfigValue(@Nullable String value, TimeUnit defaultUnit) {
     if (value == null) {
-      return TimeUnit.MILLISECONDS;
+      return defaultUnit;
     }
     // short names are UCUM names
     // long names are just TimeUnit values lowercased
@@ -45,9 +45,10 @@ static TimeUnit parseConfigValue(@Nullable String value) {
         return TimeUnit.DAYS;
       default:
         logger.warn(
-            "Invalid base time unit: '{}'; using microseconds as the base time unit instead",
-            value);
-        return TimeUnit.MILLISECONDS;
+            "Invalid base time unit: '{}'; using '{}' as the base time unit instead",
+            value,
+            getUnitString(defaultUnit));
+        return defaultUnit;
     }
   }
 

diff --git a/...ry/src/test/java/io/opentelemetry/instrumentation/micrometer/v1_5/PrometheusModeTest.java b/...ry/src/test/java/io/opentelemetry/instrumentation/micrometer/v1_5/PrometheusModeTest.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.micrometer.v1_5;
+
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Metrics;
+import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
+import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+class PrometheusModeTest extends AbstractPrometheusModeTest {
+
+  @RegisterExtension
+  static final InstrumentationExtension testing = LibraryInstrumentationExtension.create();
+
+  static MeterRegistry otelMeterRegistry;
+
+  @BeforeAll
+  public static void setUpRegistry() {
+    otelMeterRegistry =
+        OpenTelemetryMeterRegistry.builder(testing.getOpenTelemetry())
+            .setPrometheusMode(true)
+            .build();
+    Metrics.addRegistry(otelMeterRegistry);
+  }
+
+  @AfterAll
+  public static void tearDownRegistry() {
+    Metrics.removeRegistry(otelMeterRegistry);
+  }
+
+  @Override
+  protected InstrumentationExtension testing() {
+    return testing;
+  }
+}