Micrometer instrumentation, part 2 (#5001)

* Micrometer instrumentation, part 2

* use double counter instead of gauge in FunctionTimer

* code review suggestions

* fix errorprone

* rebase and fix compilation failure

instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/AsyncInstrumentRegistry.java

@@ -11,20 +11,28 @@
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
+import io.opentelemetry.api.metrics.ObservableLongMeasurement;
 import io.opentelemetry.instrumentation.api.internal.GuardedBy;
 import java.lang.ref.WeakReference;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Consumer;
 import java.util.function.ToDoubleFunction;
+import java.util.function.ToLongFunction;
 import javax.annotation.Nullable;
 
 final class AsyncInstrumentRegistry {
 
   private final Meter meter;
 
   @GuardedBy("gauges")
-  private final Map<String, GaugeMeasurementsRecorder> gauges = new HashMap<>();
+  private final Map<String, DoubleMeasurementsRecorder> gauges = new HashMap<>();
+
+  @GuardedBy("doubleCounters")
+  private final Map<String, DoubleMeasurementsRecorder> doubleCounters = new HashMap<>();
+
+  @GuardedBy("longCounters")
+  private final Map<String, LongMeasurementsRecorder> longCounters = new HashMap<>();
 
   AsyncInstrumentRegistry(Meter meter) {
     this.meter = meter;
@@ -36,83 +44,230 @@ <T> void buildGauge(
       @Nullable T obj,
       ToDoubleFunction<T> objMetric) {
 
+    String name = meterId.getName();
     synchronized (gauges) {
-      GaugeMeasurementsRecorder recorder =
+      // use the gauges map as lock for the recorder state - this way all gauge-related mutable
+      // state will always be accessed in synchronized(gauges)
+      Object recorderLock = gauges;
+
+      DoubleMeasurementsRecorder recorder =
           gauges.computeIfAbsent(
-              meterId.getName(),
+              name,
               n -> {
-                GaugeMeasurementsRecorder recorderCallback = new GaugeMeasurementsRecorder();
+                DoubleMeasurementsRecorder recorderCallback =
+                    new DoubleMeasurementsRecorder(recorderLock);
                 meter
-                    .gaugeBuilder(meterId.getName())
+                    .gaugeBuilder(name)
                     .setDescription(description(meterId))
                     .setUnit(baseUnit(meterId))
                     .buildWithCallback(recorderCallback);
                 return recorderCallback;
               });
-      recorder.addGaugeMeasurement(attributes, obj, objMetric);
+      recorder.addMeasurement(
+          attributes, new DoubleMetricInfo(obj, (ToDoubleFunction<Object>) objMetric));
     }
   }
 
   void removeGauge(String name, Attributes attributes) {
     synchronized (gauges) {
-      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);
-        }
-      }
+      removeMeasurement(gauges, name, attributes);
     }
   }
 
-  private final class GaugeMeasurementsRecorder implements Consumer<ObservableDoubleMeasurement> {
+  <T> void buildDoubleCounter(
+      io.micrometer.core.instrument.Meter.Id meterId,
+      Attributes attributes,
+      T obj,
+      ToDoubleFunction<T> objMetric) {
+    buildDoubleCounter(
+        meterId.getName(), description(meterId), baseUnit(meterId), attributes, obj, objMetric);
+  }
 
-    @GuardedBy("gauges")
-    private final Map<Attributes, GaugeInfo> measurements = new HashMap<>();
+  <T> void buildDoubleCounter(
+      String name,
+      String description,
+      String baseUnit,
+      Attributes attributes,
+      @Nullable T obj,
+      ToDoubleFunction<T> objMetric) {
 
-    @Override
-    public void accept(ObservableDoubleMeasurement measurement) {
-      Map<Attributes, GaugeInfo> measurementsCopy;
-      synchronized (gauges) {
-        measurementsCopy = new HashMap<>(measurements);
+    synchronized (doubleCounters) {
+      // use the counters map as lock for the recorder state - this way all double counter-related
+      // mutable state will always be accessed in synchronized(doubleCounters)
+      Object recorderLock = doubleCounters;
+
+      DoubleMeasurementsRecorder recorder =
+          doubleCounters.computeIfAbsent(
+              name,
+              n -> {
+                DoubleMeasurementsRecorder recorderCallback =
+                    new DoubleMeasurementsRecorder(recorderLock);
+                meter
+                    .counterBuilder(name)
+                    .setDescription(description)
+                    .setUnit(baseUnit)
+                    .ofDoubles()
+                    .buildWithCallback(recorderCallback);
+                return recorderCallback;
+              });
+      recorder.addMeasurement(
+          attributes, new DoubleMetricInfo(obj, (ToDoubleFunction<Object>) objMetric));
+    }
+  }
+
+  void removeDoubleCounter(String name, Attributes attributes) {
+    synchronized (doubleCounters) {
+      removeMeasurement(doubleCounters, name, attributes);
+    }
+  }
+
+  <T> void buildLongCounter(
+      String name,
+      String description,
+      String baseUnit,
+      Attributes attributes,
+      @Nullable T obj,
+      ToLongFunction<T> objMetric) {
+
+    synchronized (longCounters) {
+      // use the counters map as lock for the recorder state - this way all gauge-related mutable
+      // state will always be accessed in synchronized(longCounters)
+      Object recorderLock = longCounters;
+
+      LongMeasurementsRecorder recorder =
+          longCounters.computeIfAbsent(
+              name,
+              n -> {
+                LongMeasurementsRecorder recorderCallback =
+                    new LongMeasurementsRecorder(recorderLock);
+                meter
+                    .counterBuilder(name)
+                    .setDescription(description)
+                    .setUnit(baseUnit)
+                    .buildWithCallback(recorderCallback);
+                return recorderCallback;
+              });
+      recorder.addMeasurement(
+          attributes, new LongMetricInfo(obj, (ToLongFunction<Object>) objMetric));
+    }
+  }
+
+  void removeLongCounter(String name, Attributes attributes) {
+    synchronized (longCounters) {
+      removeMeasurement(longCounters, name, attributes);
+    }
+  }
+
+  private static void removeMeasurement(
+      Map<String, ? extends MutableMeasurementsRecorder<?>> registry,
+      String name,
+      Attributes attributes) {
+
+    MutableMeasurementsRecorder<?> recorder = registry.get(name);
+    if (recorder != null) {
+      recorder.removeMeasurement(attributes);
+      // if this was the last measurement then let's remove the whole recorder
+      if (recorder.isEmpty()) {
+        registry.remove(name);
       }
+    }
+  }
+
+  private abstract static class MutableMeasurementsRecorder<I> {
+
+    private final Object lock;
 
-      measurementsCopy.forEach(
-          (attributes, gauge) -> {
-            Object obj = gauge.objWeakRef.get();
-            if (obj != null) {
-              measurement.record(gauge.metricFunction.applyAsDouble(obj), attributes);
-            }
-          });
+    @GuardedBy("lock")
+    private final Map<Attributes, I> measurements = new HashMap<>();
+
+    protected MutableMeasurementsRecorder(Object lock) {
+      this.lock = lock;
     }
 
-    <T> void addGaugeMeasurement(
-        Attributes attributes, @Nullable T obj, ToDoubleFunction<T> objMetric) {
-      synchronized (gauges) {
-        measurements.put(attributes, new GaugeInfo(obj, (ToDoubleFunction<Object>) objMetric));
+    Map<Attributes, I> copyForRead() {
+      synchronized (lock) {
+        return new HashMap<>(measurements);
       }
     }
 
-    void removeGaugeMeasurement(Attributes attributes) {
-      synchronized (gauges) {
+    void addMeasurement(Attributes attributes, I info) {
+      synchronized (lock) {
+        measurements.put(attributes, info);
+      }
+    }
+
+    void removeMeasurement(Attributes attributes) {
+      synchronized (lock) {
         measurements.remove(attributes);
       }
     }
 
     boolean isEmpty() {
-      synchronized (gauges) {
+      synchronized (lock) {
         return measurements.isEmpty();
       }
     }
   }
 
-  private static final class GaugeInfo {
+  private static final class DoubleMeasurementsRecorder
+      extends MutableMeasurementsRecorder<DoubleMetricInfo>
+      implements Consumer<ObservableDoubleMeasurement> {
+
+    private DoubleMeasurementsRecorder(Object lock) {
+      super(lock);
+    }
+
+    @Override
+    public void accept(ObservableDoubleMeasurement measurement) {
+      copyForRead()
+          .forEach(
+              (attributes, gauge) -> {
+                Object obj = gauge.objWeakRef.get();
+                if (obj != null) {
+                  measurement.record(gauge.metricFunction.applyAsDouble(obj), attributes);
+                }
+              });
+    }
+  }
+
+  private static final class LongMeasurementsRecorder
+      extends MutableMeasurementsRecorder<LongMetricInfo>
+      implements Consumer<ObservableLongMeasurement> {
+
+    private LongMeasurementsRecorder(Object lock) {
+      super(lock);
+    }
+
+    @Override
+    public void accept(ObservableLongMeasurement measurement) {
+      copyForRead()
+          .forEach(
+              (attributes, gauge) -> {
+                Object obj = gauge.objWeakRef.get();
+                if (obj != null) {
+                  measurement.record(gauge.metricFunction.applyAsLong(obj), attributes);
+                }
+              });
+    }
+  }
+
+  private static final class DoubleMetricInfo {
 
     private final WeakReference<Object> objWeakRef;
     private final ToDoubleFunction<Object> metricFunction;
 
-    private GaugeInfo(@Nullable Object obj, ToDoubleFunction<Object> metricFunction) {
+    private DoubleMetricInfo(@Nullable Object obj, ToDoubleFunction<Object> metricFunction) {
+      this.objWeakRef = new WeakReference<>(obj);
+      this.metricFunction = metricFunction;
+    }
+  }
+
+  private static final class LongMetricInfo {
+
+    private final WeakReference<Object> objWeakRef;
+    private final ToLongFunction<Object> metricFunction;
+
+    private LongMetricInfo(@Nullable Object obj, ToLongFunction<Object> metricFunction) {
       this.objWeakRef = new WeakReference<>(obj);
       this.metricFunction = metricFunction;
     }

instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/Bridging.java

@@ -16,7 +16,8 @@ final class Bridging {
 
   private static final Cache<String, AttributeKey<String>> tagsCache = Cache.bounded(1024);
 
-  static Attributes toAttributes(Iterable<Tag> tags) {
+  static Attributes tagsAsAttributes(Meter.Id id) {
+    Iterable<Tag> tags = id.getTagsAsIterable();
     if (!tags.iterator().hasNext()) {
       return Attributes.empty();
     }

instrumentation/micrometer/micrometer-1.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/micrometer/v1_5/OpenTelemetryCounter.java

@@ -7,15 +7,18 @@
 
 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 static io.opentelemetry.javaagent.instrumentation.micrometer.v1_5.Bridging.tagsAsAttributes;
 
 import io.micrometer.core.instrument.Counter;
 import io.micrometer.core.instrument.Measurement;
+import io.micrometer.core.instrument.util.MeterEquivalence;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.metrics.DoubleCounter;
 import io.opentelemetry.api.metrics.Meter;
 import java.util.Collections;
+import javax.annotation.Nullable;
 
+@SuppressWarnings("HashCodeToString")
 final class OpenTelemetryCounter implements Counter, RemovableMeter {
 
   private final Id id;
@@ -34,7 +37,7 @@ final class OpenTelemetryCounter implements Counter, RemovableMeter {
             .setUnit(baseUnit(id))
             .ofDoubles()
             .build();
-    this.attributes = toAttributes(id.getTags());
+    this.attributes = tagsAsAttributes(id);
   }
 
   @Override
@@ -66,4 +69,15 @@ public Id getId() {
   public void onRemove() {
     removed = true;
   }
+
+  @SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
+  @Override
+  public boolean equals(@Nullable Object o) {
+    return MeterEquivalence.equals(this, o);
+  }
+
+  @Override
+  public int hashCode() {
+    return MeterEquivalence.hashCode(this);
+  }
 }