[Metric Framework] Adds support for Histogram metric (#12062)

* [Metric Framework] Adds support for Histogram metric Signed-off-by: Gagan Juneja <[email protected]> * Adds test Signed-off-by: Gagan Juneja <[email protected]> * Addresses review comments Signed-off-by: Gagan Juneja <[email protected]> * Adds change log Signed-off-by: Gagan Juneja <[email protected]> * Fixed spotless Signed-off-by: Gagan Juneja <[email protected]> * Fixes javadoc Signed-off-by: Gagan Juneja <[email protected]> * Fixes javadoc Signed-off-by: Gagan Juneja <[email protected]> * Fixes test Signed-off-by: Gagan Juneja <[email protected]> * Removes explicit approach Signed-off-by: Gagan Juneja <[email protected]> * Removes explicit approach Signed-off-by: Gagan Juneja <[email protected]> * Addresses review comments Signed-off-by: Gagan Juneja <[email protected]> --------- Signed-off-by: Gagan Juneja <[email protected]> Co-authored-by: Gagan Juneja <[email protected]> (cherry picked from commit a4bc4af) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Feb 5, 2024 · 45e0c61 · 45e0c61
1 parent 4082fe8
commit 45e0c61
Show file tree

Hide file tree

Showing 14 changed files with 242 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - New DateTime format for RFC3339 compatible date fields ([#11465](https://github.com/opensearch-project/OpenSearch/pull/11465))
 - Remove concurrent segment search feature flag for GA launch ([#12074](https://github.com/opensearch-project/OpenSearch/pull/12074))
 - Enable Fuzzy codec for doc id fields using a bloom filter ([#11022](https://github.com/opensearch-project/OpenSearch/pull/11022))
+- [Metrics Framework] Adds support for Histogram metric ([#12062](https://github.com/opensearch-project/OpenSearch/pull/12062))
 
 ### Dependencies
 - Bumps jetty version to 9.4.52.v20230823 to fix GMS-2023-1857 ([#9822](https://github.com/opensearch-project/OpenSearch/pull/9822))

diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java
@@ -34,6 +34,11 @@ public Counter createUpDownCounter(String name, String description, String unit)
         return metricsTelemetry.createUpDownCounter(name, description, unit);
     }
 
+    @Override
+    public Histogram createHistogram(String name, String description, String unit) {
+        return metricsTelemetry.createHistogram(name, description, unit);
+    }
+
     @Override
     public void close() throws IOException {
         metricsTelemetry.close();

diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Histogram.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Histogram.java
@@ -0,0 +1,35 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.telemetry.metrics;
+
+import org.opensearch.common.annotation.ExperimentalApi;
+import org.opensearch.telemetry.metrics.tags.Tags;
+
+/**
+ * Histogram records the value for an existing metric.
+ * {@opensearch.experimental}
+ */
+@ExperimentalApi
+public interface Histogram {
+
+    /**
+     * record value.
+     * @param value value to be added.
+     */
+    void record(double value);
+
+    /**
+     * record value along with the attributes.
+     *
+     * @param value value to be added.
+     * @param tags  attributes/dimensions of the metric.
+     */
+    void record(double value, Tags tags);
+
+}
diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java
@@ -36,4 +36,15 @@ public interface MetricsRegistry extends Closeable {
      * @return counter.
      */
     Counter createUpDownCounter(String name, String description, String unit);
+
+    /**
+     * Creates the histogram type of Metric. Implementation framework will take care
+     * of the bucketing strategy.
+     *
+     * @param name        name of the histogram.
+     * @param description any description about the metric.
+     * @param unit        unit of the metric.
+     * @return histogram.
+     */
+    Histogram createHistogram(String name, String description, String unit);
 }
diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopHistogram.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopHistogram.java
@@ -0,0 +1,38 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.telemetry.metrics.noop;
+
+import org.opensearch.common.annotation.InternalApi;
+import org.opensearch.telemetry.metrics.Histogram;
+import org.opensearch.telemetry.metrics.tags.Tags;
+
+/**
+ * No-op {@link Histogram}
+ * {@opensearch.internal}
+ */
+@InternalApi
+public class NoopHistogram implements Histogram {
+
+    /**
+     * No-op Histogram instance
+     */
+    public final static NoopHistogram INSTANCE = new NoopHistogram();
+
+    private NoopHistogram() {}
+
+    @Override
+    public void record(double value) {
+
+    }
+
+    @Override
+    public void record(double value, Tags tags) {
+
+    }
+}
diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java
@@ -10,6 +10,7 @@
 
 import org.opensearch.common.annotation.InternalApi;
 import org.opensearch.telemetry.metrics.Counter;
+import org.opensearch.telemetry.metrics.Histogram;
 import org.opensearch.telemetry.metrics.MetricsRegistry;
 
 import java.io.IOException;
@@ -38,6 +39,11 @@ public Counter createUpDownCounter(String name, String description, String unit)
         return NoopCounter.INSTANCE;
     }
 
+    @Override
+    public Histogram createHistogram(String name, String description, String unit) {
+        return NoopHistogram.INSTANCE;
+    }
+
     @Override
     public void close() throws IOException {
 

diff --git a/...telemetry/src/test/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistryTests.java b/...telemetry/src/test/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistryTests.java
@@ -48,4 +48,15 @@ public void testUpDownCounter() {
         assertSame(mockCounter, counter);
     }
 
+    public void testHistogram() {
+        Histogram mockHistogram = mock(Histogram.class);
+        when(defaultMeterRegistry.createHistogram(any(String.class), any(String.class), any(String.class))).thenReturn(mockHistogram);
+        Histogram histogram = defaultMeterRegistry.createHistogram(
+            "org.opensearch.telemetry.metrics.DefaultMeterRegistryTests.testHistogram",
+            "test up-down counter",
+            "ms"
+        );
+        assertSame(mockHistogram, histogram);
+    }
+
 }
diff --git a/...alClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsDisabledSanityIT.java b/...alClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsDisabledSanityIT.java
@@ -15,6 +15,7 @@
 import org.opensearch.telemetry.OTelTelemetrySettings;
 import org.opensearch.telemetry.TelemetrySettings;
 import org.opensearch.telemetry.metrics.noop.NoopCounter;
+import org.opensearch.telemetry.metrics.noop.NoopHistogram;
 import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
@@ -53,10 +54,13 @@ public void testSanityChecksWhenMetricsDisabled() throws Exception {
         Counter counter = metricsRegistry.createCounter("test-counter", "test", "1");
         counter.add(1.0);
 
+        Histogram histogram = metricsRegistry.createHistogram("test-histogram", "test", "1");
+
         Thread.sleep(2000);
 
         assertTrue(metricsRegistry instanceof NoopMetricsRegistry);
         assertTrue(counter instanceof NoopCounter);
+        assertTrue(histogram instanceof NoopHistogram);
     }
 
 }
diff --git a/...nalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledSanityIT.java b/...nalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledSanityIT.java
@@ -23,6 +23,7 @@
 import java.util.stream.Collectors;
 
 import io.opentelemetry.sdk.metrics.data.DoublePointData;
+import io.opentelemetry.sdk.metrics.internal.data.ImmutableExponentialHistogramPointData;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, minNumDataNodes = 1)
 public class TelemetryMetricsEnabledSanityIT extends OpenSearchIntegTestCase {
@@ -92,6 +93,31 @@ public void testUpDownCounter() throws Exception {
         assertEquals(-1.0, value, 0.0);
     }
 
+    public void testHistogram() throws Exception {
+        MetricsRegistry metricsRegistry = internalCluster().getInstance(MetricsRegistry.class);
+        InMemorySingletonMetricsExporter.INSTANCE.reset();
+
+        Histogram histogram = metricsRegistry.createHistogram("test-histogram", "test", "ms");
+        histogram.record(2.0);
+        histogram.record(1.0);
+        histogram.record(3.0);
+        // Sleep for about 2s to wait for metrics to be published.
+        Thread.sleep(2000);
+
+        InMemorySingletonMetricsExporter exporter = InMemorySingletonMetricsExporter.INSTANCE;
+        ImmutableExponentialHistogramPointData histogramPointData = ((ImmutableExponentialHistogramPointData) ((ArrayList) exporter
+            .getFinishedMetricItems()
+            .stream()
+            .filter(a -> a.getName().contains("test-histogram"))
+            .collect(Collectors.toList())
+            .get(0)
+            .getExponentialHistogramData()
+            .getPoints()).get(0));
+        assertEquals(1.0, histogramPointData.getSum(), 6.0);
+        assertEquals(1.0, histogramPointData.getMax(), 3.0);
+        assertEquals(1.0, histogramPointData.getMin(), 1.0);
+    }
+
     @After
     public void reset() {
         InMemorySingletonMetricsExporter.INSTANCE.reset();

diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelHistogram.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelHistogram.java
@@ -0,0 +1,40 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.telemetry.metrics;
+
+import org.opensearch.telemetry.OTelAttributesConverter;
+import org.opensearch.telemetry.metrics.tags.Tags;
+
+import io.opentelemetry.api.metrics.DoubleHistogram;
+
+/**
+ * OTel aware implementation {@link Histogram}
+ */
+class OTelHistogram implements Histogram {
+
+    private final DoubleHistogram otelDoubleHistogram;
+
+    /**
+     * Constructor
+     * @param otelDoubleCounter delegate counter.
+     */
+    public OTelHistogram(DoubleHistogram otelDoubleCounter) {
+        this.otelDoubleHistogram = otelDoubleCounter;
+    }
+
+    @Override
+    public void record(double value) {
+        otelDoubleHistogram.record(value);
+    }
+
+    @Override
+    public void record(double value, Tags tags) {
+        otelDoubleHistogram.record(value, OTelAttributesConverter.convert(tags));
+    }
+}
diff --git a/...s/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/...s/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java
@@ -17,6 +17,7 @@
 import java.security.PrivilegedAction;
 
 import io.opentelemetry.api.metrics.DoubleCounter;
+import io.opentelemetry.api.metrics.DoubleHistogram;
 import io.opentelemetry.api.metrics.DoubleUpDownCounter;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.api.metrics.MeterProvider;
@@ -68,6 +69,23 @@ public Counter createUpDownCounter(String name, String description, String unit)
         return new OTelUpDownCounter(doubleUpDownCounter);
     }
 
+    /**
+     * Creates the Otel Histogram. In {@link org.opensearch.telemetry.tracing.OTelResourceProvider}
+     * we can configure the bucketing/aggregation strategy through view. Default startegy configured
+     * is the {@link io.opentelemetry.sdk.metrics.internal.view.Base2ExponentialHistogramAggregation}.
+     * @param name        name of the histogram.
+     * @param description any description about the metric.
+     * @param unit        unit of the metric.
+     * @return histogram
+     */
+    @Override
+    public Histogram createHistogram(String name, String description, String unit) {
+        DoubleHistogram doubleHistogram = AccessController.doPrivileged(
+            (PrivilegedAction<DoubleHistogram>) () -> otelMeter.histogramBuilder(name).setUnit(unit).setDescription(description).build()
+        );
+        return new OTelHistogram(doubleHistogram);
+    }
+
     @Override
     public void close() throws IOException {
         meterProvider.close();

diff --git a/...s/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java b/...s/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java
@@ -23,8 +23,12 @@
 import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.metrics.InstrumentSelector;
+import io.opentelemetry.sdk.metrics.InstrumentType;
 import io.opentelemetry.sdk.metrics.SdkMeterProvider;
+import io.opentelemetry.sdk.metrics.View;
 import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader;
+import io.opentelemetry.sdk.metrics.internal.view.Base2ExponentialHistogramAggregation;
 import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
 import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
@@ -40,6 +44,7 @@
  * This class encapsulates all OpenTelemetry related resources
  */
 public final class OTelResourceProvider {
+
     private OTelResourceProvider() {}
 
     /**
@@ -92,6 +97,10 @@ private static SdkMeterProvider createSdkMetricProvider(Settings settings, Resou
                     .setInterval(TelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING.get(settings).getSeconds(), TimeUnit.SECONDS)
                     .build()
             )
+            .registerView(
+                InstrumentSelector.builder().setType(InstrumentType.HISTOGRAM).build(),
+                View.builder().setAggregation(Base2ExponentialHistogramAggregation.getDefault()).build()
+            )
             .build();
     }
 

diff --git a/...emetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/...emetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java
@@ -17,12 +17,15 @@
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.metrics.DoubleCounter;
 import io.opentelemetry.api.metrics.DoubleCounterBuilder;
+import io.opentelemetry.api.metrics.DoubleHistogram;
+import io.opentelemetry.api.metrics.DoubleHistogramBuilder;
 import io.opentelemetry.api.metrics.DoubleUpDownCounter;
 import io.opentelemetry.api.metrics.DoubleUpDownCounterBuilder;
 import io.opentelemetry.api.metrics.LongCounterBuilder;
 import io.opentelemetry.api.metrics.LongUpDownCounterBuilder;
 import io.opentelemetry.api.metrics.Meter;
 import io.opentelemetry.api.metrics.MeterProvider;
+import org.mockito.Mockito;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -118,4 +121,32 @@ public void testUpDownCounter() {
         counter.add(-2.0, tags);
         verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(tags));
     }
+
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    public void testHistogram() {
+        String histogramName = "test-histogram";
+        String description = "test";
+        String unit = "1";
+        Meter mockMeter = mock(Meter.class);
+        OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class);
+        DoubleHistogram mockOTelDoubleHistogram = mock(DoubleHistogram.class);
+        DoubleHistogramBuilder mockOTelDoubleHistogramBuilder = mock(DoubleHistogramBuilder.class);
+        MeterProvider meterProvider = mock(MeterProvider.class);
+        when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter);
+        MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(
+            new RefCountedReleasable("telemetry", mockOpenTelemetry, () -> {}),
+            meterProvider
+        );
+        when(mockMeter.histogramBuilder(Mockito.contains(histogramName))).thenReturn(mockOTelDoubleHistogramBuilder);
+        when(mockOTelDoubleHistogramBuilder.setDescription(description)).thenReturn(mockOTelDoubleHistogramBuilder);
+        when(mockOTelDoubleHistogramBuilder.setUnit(unit)).thenReturn(mockOTelDoubleHistogramBuilder);
+        when(mockOTelDoubleHistogramBuilder.build()).thenReturn(mockOTelDoubleHistogram);
+
+        Histogram histogram = metricsTelemetry.createHistogram(histogramName, description, unit);
+        histogram.record(1.0);
+        verify(mockOTelDoubleHistogram).record(1.0);
+        Tags tags = Tags.create().addTag("test", "test");
+        histogram.record(2.0, tags);
+        verify(mockOTelDoubleHistogram).record(2.0, OTelAttributesConverter.convert(tags));
+    }
 }
diff --git a/test/framework/src/main/java/org/opensearch/test/telemetry/MockTelemetry.java b/test/framework/src/main/java/org/opensearch/test/telemetry/MockTelemetry.java
@@ -11,8 +11,10 @@
 import org.opensearch.telemetry.Telemetry;
 import org.opensearch.telemetry.TelemetrySettings;
 import org.opensearch.telemetry.metrics.Counter;
+import org.opensearch.telemetry.metrics.Histogram;
 import org.opensearch.telemetry.metrics.MetricsTelemetry;
 import org.opensearch.telemetry.metrics.noop.NoopCounter;
+import org.opensearch.telemetry.metrics.noop.NoopHistogram;
 import org.opensearch.telemetry.tracing.TracingTelemetry;
 import org.opensearch.test.telemetry.tracing.MockTracingTelemetry;
 
@@ -46,6 +48,11 @@ public Counter createUpDownCounter(String name, String description, String unit)
                 return NoopCounter.INSTANCE;
             }
 
+            @Override
+            public Histogram createHistogram(String name, String description, String unit) {
+                return NoopHistogram.INSTANCE;
+            }
+
             @Override
             public void close() {