Removes explicit approach

Signed-off-by: Gagan Juneja <gjjuneja@amazon.com>
Gaganjuneja · Feb 5, 2024 · 38eb4f3 · 38eb4f3
1 parent 5eed5cf
commit 38eb4f3
Show file tree

Hide file tree

Showing 9 changed files with 0 additions and 132 deletions.
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
@@ -9,7 +9,6 @@
 package org.opensearch.telemetry.metrics;
 
 import java.io.IOException;
-import java.util.List;
 
 /**
  * Default implementation for {@link MetricsRegistry}
@@ -40,11 +39,6 @@ public Histogram createHistogram(String name, String description, String unit) {
         return metricsTelemetry.createHistogram(name, description, unit);
     }
 
-    @Override
-    public Histogram createHistogram(String name, String description, String unit, List<Double> buckets) {
-        return metricsTelemetry.createHistogram(name, description, unit, buckets);
-    }
-
     @Override
     public void close() throws IOException {
         metricsTelemetry.close();

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
@@ -11,7 +11,6 @@
 import org.opensearch.common.annotation.ExperimentalApi;
 
 import java.io.Closeable;
-import java.util.List;
 
 /**
  * MetricsRegistry helps in creating the metric instruments.
@@ -48,16 +47,4 @@ public interface MetricsRegistry extends Closeable {
      * @return histogram.
      */
     Histogram createHistogram(String name, String description, String unit);
-
-    /**
-     * Creates the histogram type of Metric with the explicit buckets defined. This should be used only
-     * in the scenario where user is very much aware of the buckets otherwise it's advised to use the
-     * default api.
-     * @param name        name of the histogram.
-     * @param description any description about the metric.
-     * @param unit        unit of the metric.
-     * @param buckets     list of explicit histogram buckets.
-     * @return histogram.
-     */
-    Histogram createHistogram(String name, String description, String unit, List<Double> buckets);
 }
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
@@ -14,7 +14,6 @@
 import org.opensearch.telemetry.metrics.MetricsRegistry;
 
 import java.io.IOException;
-import java.util.List;
 
 /**
  *No-op {@link MetricsRegistry}
@@ -45,11 +44,6 @@ public Histogram createHistogram(String name, String description, String unit) {
         return NoopHistogram.INSTANCE;
     }
 
-    @Override
-    public Histogram createHistogram(String name, String description, String unit, List<Double> buckets) {
-        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
@@ -10,10 +10,6 @@
 
 import org.opensearch.test.OpenSearchTestCase;
 
-import java.util.Arrays;
-
-import org.mockito.ArgumentMatchers;
-
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -63,23 +59,4 @@ public void testHistogram() {
         assertSame(mockHistogram, histogram);
     }
 
-    public void testHistogramWithExplicitBuckets() {
-        Histogram mockHistogram = mock(Histogram.class);
-        when(
-            defaultMeterRegistry.createHistogram(
-                any(String.class),
-                any(String.class),
-                any(String.class),
-                ArgumentMatchers.<Double>anyList()
-            )
-        ).thenReturn(mockHistogram);
-        Histogram histogram = defaultMeterRegistry.createHistogram(
-            "org.opensearch.telemetry.metrics.DefaultMeterRegistryTests.testHistogram",
-            "test up-down counter",
-            "ms",
-            Arrays.asList(1.0, 2.0)
-        );
-        assertSame(mockHistogram, histogram);
-    }
-
 }
diff --git a/...alClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsDisabledSanityIT.java b/...alClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsDisabledSanityIT.java
@@ -56,14 +56,11 @@ public void testSanityChecksWhenMetricsDisabled() throws Exception {
 
         Histogram histogram = metricsRegistry.createHistogram("test-histogram", "test", "1");
 
-        Histogram histogramWithBuckets = metricsRegistry.createHistogram("test-histogram", "test", "1", Arrays.asList(1.0));
-
         Thread.sleep(2000);
 
         assertTrue(metricsRegistry instanceof NoopMetricsRegistry);
         assertTrue(counter instanceof NoopCounter);
         assertTrue(histogram instanceof NoopHistogram);
-        assertTrue(histogramWithBuckets instanceof NoopHistogram);
     }
 
 }
diff --git a/...nalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledSanityIT.java b/...nalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledSanityIT.java
@@ -20,12 +20,10 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.List;
 import java.util.stream.Collectors;
 
 import io.opentelemetry.sdk.metrics.data.DoublePointData;
 import io.opentelemetry.sdk.metrics.internal.data.ImmutableExponentialHistogramPointData;
-import io.opentelemetry.sdk.metrics.internal.data.ImmutableHistogramPointData;
 
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, minNumDataNodes = 1)
 public class TelemetryMetricsEnabledSanityIT extends OpenSearchIntegTestCase {
@@ -120,32 +118,6 @@ public void testHistogram() throws Exception {
         assertEquals(1.0, histogramPointData.getMin(), 1.0);
     }
 
-    public void testHistogramWithExplicitBuckets() throws Exception {
-        MetricsRegistry metricsRegistry = internalCluster().getInstance(MetricsRegistry.class);
-        InMemorySingletonMetricsExporter.INSTANCE.reset();
-
-        List<Double> buckets = Arrays.asList(1.0, 5.0, 10.0);
-        Histogram histogram = metricsRegistry.createHistogram("test-histogram", "test", "ms", buckets);
-        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;
-        ImmutableHistogramPointData histogramPointData = ((ImmutableHistogramPointData) ((ArrayList) exporter.getFinishedMetricItems()
-            .stream()
-            .filter(a -> a.getName().equals("test-histogram"))
-            .collect(Collectors.toList())
-            .get(0)
-            .getHistogramData()
-            .getPoints()).get(0));
-        assertEquals(1.0, histogramPointData.getSum(), 6.0);
-        assertEquals(1.0, histogramPointData.getMax(), 3.0);
-        assertEquals(1.0, histogramPointData.getMin(), 1.0);
-        assertEquals(buckets, histogramPointData.getBoundaries());
-    }
-
     @After
     public void reset() {
         InMemorySingletonMetricsExporter.INSTANCE.reset();

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
@@ -16,7 +16,6 @@
 import java.io.IOException;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.util.List;
 
 import io.opentelemetry.api.metrics.DoubleCounter;
 import io.opentelemetry.api.metrics.DoubleHistogram;
@@ -93,18 +92,6 @@ public Histogram createHistogram(String name, String description, String unit) {
         return new OTelHistogram(doubleHistogram);
     }
 
-    @Override
-    public Histogram createHistogram(String name, String description, String unit, List<Double> buckets) {
-        DoubleHistogram doubleHistogram = AccessController.doPrivileged(
-            (PrivilegedAction<DoubleHistogram>) () -> otelMeter.histogramBuilder(name)
-                .setUnit(unit)
-                .setDescription(description)
-                .setExplicitBucketBoundariesAdvice(buckets)
-                .build()
-        );
-        return new OTelHistogram(doubleHistogram);
-    }
-
     @Override
     public void close() throws IOException {
         meterProvider.close();

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
@@ -14,9 +14,6 @@
 import org.opensearch.telemetry.metrics.tags.Tags;
 import org.opensearch.test.OpenSearchTestCase;
 
-import java.util.Arrays;
-import java.util.List;
-
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.metrics.DoubleCounter;
 import io.opentelemetry.api.metrics.DoubleCounterBuilder;
@@ -152,34 +149,4 @@ public void testHistogram() {
         histogram.record(2.0, tags);
         verify(mockOTelDoubleHistogram).record(2.0, OTelAttributesConverter.convert(tags));
     }
-
-    @SuppressWarnings({ "rawtypes", "unchecked" })
-    public void testHistogramWithExplicitBuckets() {
-        String histogramName = "test-histogram";
-        String description = "test";
-        String unit = "1";
-        List<Double> buckets = Arrays.asList(1.0, 5.0);
-        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(histogramName)).thenReturn(mockOTelDoubleHistogramBuilder);
-        when(mockOTelDoubleHistogramBuilder.setDescription(description)).thenReturn(mockOTelDoubleHistogramBuilder);
-        when(mockOTelDoubleHistogramBuilder.setUnit(unit)).thenReturn(mockOTelDoubleHistogramBuilder);
-        when(mockOTelDoubleHistogramBuilder.setExplicitBucketBoundariesAdvice(buckets)).thenReturn(mockOTelDoubleHistogramBuilder);
-        when(mockOTelDoubleHistogramBuilder.build()).thenReturn(mockOTelDoubleHistogram);
-
-        Histogram histogram = metricsTelemetry.createHistogram(histogramName, description, unit, buckets);
-        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
@@ -18,8 +18,6 @@
 import org.opensearch.telemetry.tracing.TracingTelemetry;
 import org.opensearch.test.telemetry.tracing.MockTracingTelemetry;
 
-import java.util.List;
-
 /**
  * Mock {@link Telemetry} implementation for testing.
  */
@@ -55,11 +53,6 @@ public Histogram createHistogram(String name, String description, String unit) {
                 return NoopHistogram.INSTANCE;
             }
 
-            @Override
-            public Histogram createHistogram(String name, String description, String unit, List<Double> buckets) {
-                return NoopHistogram.INSTANCE;
-            }
-
             @Override
             public void close() {