From f1966585401e8cf3a96b63021a0391c5de7b690a Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 01:14:47 +0530 Subject: [PATCH] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/OTelTelemetryPlugin.java | 4 +- .../metrics/OTelMetricsTelemetry.java | 7 +-- .../tracing/OTelResourceProvider.java | 16 +++-- .../tracing/OTelTracingTelemetry.java | 7 +-- .../telemetry/OTelTelemetryPluginTests.java | 6 +- .../metrics/OTelMetricsTelemetryTests.java | 8 +-- .../tracing/OTelTracingTelemetryTests.java | 10 +-- .../sampler/ProbabilisticSamplerTests.java | 5 +- .../common/settings/ClusterSettings.java | 6 +- .../telemetry/TelemetrySettings.java | 28 --------- .../metrics/MetricsRegistryFactory.java | 5 -- .../telemetry/metrics/WrappedCounter.java | 51 --------------- .../metrics/WrappedMetricsRegistry.java | 49 --------------- .../metrics/MetricsRegistryFactoryTests.java | 6 +- .../metrics/WrappedCounterTests.java | 63 ------------------- .../metrics/WrappedMetricsRegistryTests.java | 49 --------------- 16 files changed, 34 insertions(+), 286 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java delete mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java delete mode 100644 server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java delete mode 100644 server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 5aa31a86bf4e8..b007cc60e304d 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -21,7 +21,7 @@ import java.util.List; import java.util.Optional; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.sdk.OpenTelemetrySdk; /** * Telemetry plugin based on Otel @@ -64,7 +64,7 @@ public String getName() { } private Telemetry telemetry(TelemetrySettings telemetrySettings) { - final OpenTelemetry openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); + final OpenTelemetrySdk openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); return new OTelTelemetry(new OTelTracingTelemetry(openTelemetry), new OTelMetricsTelemetry(openTelemetry)); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index bc0b144e63ad5..5538f5b3625e4 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -15,7 +15,6 @@ import java.security.AccessController; import java.security.PrivilegedAction; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleUpDownCounter; import io.opentelemetry.api.metrics.Meter; @@ -26,14 +25,14 @@ */ public class OTelMetricsTelemetry implements MetricsTelemetry { private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); - private final OpenTelemetry openTelemetry; + private final OpenTelemetrySdk openTelemetry; private final Meter otelMeter; /** * Constructor. * @param openTelemetry telemetry. */ - public OTelMetricsTelemetry(OpenTelemetry openTelemetry) { + public OTelMetricsTelemetry(OpenTelemetrySdk openTelemetry) { this.openTelemetry = openTelemetry; this.otelMeter = openTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); } @@ -66,7 +65,7 @@ public Counter createUpDownCounter(String name, String description, String unit) public void close() { // There is no harm closing the openTelemetry multiple times. try { - ((OpenTelemetrySdk) openTelemetry).getSdkMeterProvider().close(); + openTelemetry.getSdkMeterProvider().close(); } catch (Exception e) { logger.warn("Error while closing Opentelemetry", e); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java index d436264bd0557..a6a1f12aab8a9 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java @@ -19,7 +19,6 @@ import java.security.PrivilegedAction; import java.util.concurrent.TimeUnit; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; @@ -47,11 +46,11 @@ private OTelResourceProvider() {} * Creates OpenTelemetry instance with default configuration * @param telemetrySettings telemetry settings * @param settings cluster settings - * @return OpenTelemetry instance + * @return OpenTelemetrySdk instance */ - public static OpenTelemetry get(TelemetrySettings telemetrySettings, Settings settings) { + public static OpenTelemetrySdk get(TelemetrySettings telemetrySettings, Settings settings) { return AccessController.doPrivileged( - (PrivilegedAction) () -> get( + (PrivilegedAction) () -> get( settings, OTelSpanExporterFactory.create(settings), ContextPropagators.create(W3CTraceContextPropagator.getInstance()), @@ -66,9 +65,14 @@ public static OpenTelemetry get(TelemetrySettings telemetrySettings, Settings se * @param spanExporter span exporter instance * @param contextPropagators context propagator instance * @param sampler sampler instance - * @return Opentelemetry instance + * @return OpenTelemetrySdk instance */ - public static OpenTelemetry get(Settings settings, SpanExporter spanExporter, ContextPropagators contextPropagators, Sampler sampler) { + public static OpenTelemetrySdk get( + Settings settings, + SpanExporter spanExporter, + ContextPropagators contextPropagators, + Sampler sampler + ) { Resource resource = Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, "OpenSearch")); SdkTracerProvider sdkTracerProvider = createSdkTracerProvider(settings, spanExporter, sampler, resource); SdkMeterProvider sdkMeterProvider = createSdkMetricProvider(settings, resource); diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 6d3fc6f11404b..ea7bce5080e76 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -13,7 +13,6 @@ import org.opensearch.telemetry.OTelAttributesConverter; import org.opensearch.telemetry.OTelTelemetryPlugin; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; import io.opentelemetry.sdk.OpenTelemetrySdk; @@ -23,14 +22,14 @@ public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); - private final OpenTelemetry openTelemetry; + private final OpenTelemetrySdk openTelemetry; private final io.opentelemetry.api.trace.Tracer otelTracer; /** * Creates OTel based Telemetry * @param openTelemetry OpenTelemetry instance */ - public OTelTracingTelemetry(OpenTelemetry openTelemetry) { + public OTelTracingTelemetry(OpenTelemetrySdk openTelemetry) { this.openTelemetry = openTelemetry; this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); @@ -39,7 +38,7 @@ public OTelTracingTelemetry(OpenTelemetry openTelemetry) { @Override public void close() { try { - ((OpenTelemetrySdk) openTelemetry).getSdkTracerProvider().close(); + openTelemetry.getSdkTracerProvider().close(); } catch (Exception e) { logger.warn("Error while closing Opentelemetry", e); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 795cdeb2ebc56..2a6b9d687d2d9 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -32,7 +32,6 @@ import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING; -import static org.opensearch.telemetry.TelemetrySettings.METRICS_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; @@ -51,10 +50,7 @@ public void setup() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); oTelTelemetryModulePlugin = new OTelTelemetryPlugin(settings); telemetry = oTelTelemetryModulePlugin.getTelemetry( - new TelemetrySettings( - Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY, METRICS_ENABLED_SETTING)) - ) + new TelemetrySettings(Settings.EMPTY, new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY))) ); tracingTelemetry = telemetry.get().getTracingTelemetry(); metricsTelemetry = telemetry.get().getMetricsTelemetry(); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index c0780fe309908..cf398e3467401 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -13,7 +13,6 @@ import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleCounterBuilder; import io.opentelemetry.api.metrics.DoubleUpDownCounter; @@ -21,6 +20,7 @@ import io.opentelemetry.api.metrics.LongCounterBuilder; import io.opentelemetry.api.metrics.LongUpDownCounterBuilder; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.sdk.OpenTelemetrySdk; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -32,7 +32,7 @@ public void testCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); @@ -58,7 +58,7 @@ public void testCounterNegativeValue() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); @@ -81,7 +81,7 @@ public void testUpDownCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Meter mockMeter = mock(Meter.class); DoubleUpDownCounter mockOTelUpDownDoubleCounter = mock(DoubleUpDownCounter.class); LongUpDownCounterBuilder mockOTelLongUpDownCounterBuilder = mock(LongUpDownCounterBuilder.class); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 0181033bcc9c7..d9d66f4ab4098 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -15,10 +15,10 @@ import java.util.Collections; import java.util.Map; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.sdk.OpenTelemetrySdk; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -29,7 +29,7 @@ public class OTelTracingTelemetryTests extends OpenSearchTestCase { public void testCreateSpanWithoutParent() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -47,7 +47,7 @@ public void testCreateSpanWithoutParent() { } public void testCreateSpanWithParent() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -71,7 +71,7 @@ public void testCreateSpanWithParent() { } public void testCreateSpanWithParentWithMultipleAttributes() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -117,7 +117,7 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at } public void testGetContextPropagator() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java index 5cf7cf127822c..639dc341ef0db 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java @@ -18,7 +18,6 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; -import static org.opensearch.telemetry.TelemetrySettings.METRICS_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; @@ -34,7 +33,7 @@ public void testDefaultGetSampler() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, METRICS_ENABLED_SETTING)) + new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) ); // Probabilistic Sampler @@ -48,7 +47,7 @@ public void testGetSamplerWithUpdatedSamplingRatio() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, METRICS_ENABLED_SETTING)) + new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) ); // Probabilistic Sampler diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 6de70c02e69ef..032027384f106 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -692,10 +692,6 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_SETTING ), List.of(FeatureFlags.TELEMETRY), - List.of( - TelemetrySettings.TRACER_ENABLED_SETTING, - TelemetrySettings.TRACER_SAMPLER_PROBABILITY, - TelemetrySettings.METRICS_ENABLED_SETTING - ) + List.of(TelemetrySettings.TRACER_ENABLED_SETTING, TelemetrySettings.TRACER_SAMPLER_PROBABILITY) ); } diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index 6c437911989b8..edb20cfa9dfc5 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -28,13 +28,6 @@ public class TelemetrySettings { Setting.Property.Dynamic ); - public static final Setting METRICS_ENABLED_SETTING = Setting.boolSetting( - "telemetry.metrics.enabled", - false, - Setting.Property.NodeScope, - Setting.Property.Dynamic - ); - /** * Probability of sampler */ @@ -60,16 +53,12 @@ public class TelemetrySettings { private volatile boolean tracingEnabled; private volatile double samplingProbability; - private volatile boolean metricsEnabled; - public TelemetrySettings(Settings settings, ClusterSettings clusterSettings) { this.tracingEnabled = TRACER_ENABLED_SETTING.get(settings); this.samplingProbability = TRACER_SAMPLER_PROBABILITY.get(settings); - this.metricsEnabled = METRICS_ENABLED_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(TRACER_ENABLED_SETTING, this::setTracingEnabled); clusterSettings.addSettingsUpdateConsumer(TRACER_SAMPLER_PROBABILITY, this::setSamplingProbability); - clusterSettings.addSettingsUpdateConsumer(METRICS_ENABLED_SETTING, this::setMetricsEnabled); } public void setTracingEnabled(boolean tracingEnabled) { @@ -94,21 +83,4 @@ public void setSamplingProbability(double samplingProbability) { public double getSamplingProbability() { return samplingProbability; } - - /** - * update the metrics enabled property. - * @param metricsEnabled metrics enabled. - */ - public void setMetricsEnabled(boolean metricsEnabled) { - this.metricsEnabled = metricsEnabled; - } - - /** - * Returns whether metrics are enabled or not. - * @return enabled/disabled flag. - */ - public boolean isMetricsEnabled() { - return metricsEnabled; - } - } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java index 8a4c859495de6..c7e2229c18437 100644 --- a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java @@ -65,7 +65,6 @@ public void close() { private MetricsRegistry metricsRegistry(Optional telemetry) { MetricsRegistry metricsRegistry = telemetry.map(Telemetry::getMetricsTelemetry) .map(metricsTelemetry -> createDefaultMetricsRegistry(metricsTelemetry)) - .map(defaultMetricsRegistry -> createWrappedMetricsRegistry(defaultMetricsRegistry)) .orElse(NoopMetricsRegistry.INSTANCE); return metricsRegistry; } @@ -74,8 +73,4 @@ private MetricsRegistry createDefaultMetricsRegistry(MetricsTelemetry metricsTel return new DefaultMetricsRegistry(metricsTelemetry); } - private MetricsRegistry createWrappedMetricsRegistry(MetricsRegistry metricsRegistry) { - return new WrappedMetricsRegistry(telemetrySettings, metricsRegistry); - } - } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java deleted file mode 100644 index f42467679b2b8..0000000000000 --- a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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.InternalApi; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.tags.Tags; - -/** - * Wrapper implementation of {@link Counter}. This delegates call to right {@link Counter} based on the settings - */ -@InternalApi -public class WrappedCounter implements Counter { - - private final TelemetrySettings telemetrySettings; - private final Counter delegate; - - /** - * Constructor - * @param telemetrySettings telemetry settings. - * @param delegate delegate counter. - */ - public WrappedCounter(TelemetrySettings telemetrySettings, Counter delegate) { - this.telemetrySettings = telemetrySettings; - this.delegate = delegate; - } - - @Override - public void add(double value) { - if (isMetricsEnabled()) { - delegate.add(value); - } - } - - @Override - public void add(double value, Tags tags) { - if (isMetricsEnabled()) { - delegate.add(value, tags); - } - } - - private boolean isMetricsEnabled() { - return telemetrySettings.isMetricsEnabled(); - } -} diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java deleted file mode 100644 index c83ef21501d4e..0000000000000 --- a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * 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.InternalApi; -import org.opensearch.telemetry.TelemetrySettings; - -import java.io.IOException; - -/** - * Wrapper implementation of {@link MetricsRegistry}. This delegates call to right {@link MetricsRegistry} based on the settings - */ -@InternalApi -public class WrappedMetricsRegistry implements MetricsRegistry { - - private final MetricsRegistry defaultMetricsRegistry; - private final TelemetrySettings telemetrySettings; - - /** - * Constructor. - * @param defaultMetricsRegistry default meter registry. - * @param telemetrySettings telemetry settings. - */ - public WrappedMetricsRegistry(TelemetrySettings telemetrySettings, MetricsRegistry defaultMetricsRegistry) { - this.telemetrySettings = telemetrySettings; - this.defaultMetricsRegistry = defaultMetricsRegistry; - } - - @Override - public Counter createCounter(String name, String description, String unit) { - return new WrappedCounter(telemetrySettings, defaultMetricsRegistry.createCounter(name, description, unit)); - } - - @Override - public Counter createUpDownCounter(String name, String description, String unit) { - return new WrappedCounter(telemetrySettings, defaultMetricsRegistry.createUpDownCounter(name, description, unit)); - } - - @Override - public void close() throws IOException { - defaultMetricsRegistry.close(); - } -} diff --git a/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java index 6094d280c20a9..5d5ea62dd161e 100644 --- a/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java @@ -37,7 +37,7 @@ public void close() { metricsRegistryFactory.close(); } - public void testGetMeterRegistrywithUnavailableMetricsTelemetry() { + public void testGetMeterRegistryWithUnavailableMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); @@ -51,7 +51,7 @@ public void testGetMeterRegistrywithUnavailableMetricsTelemetry() { assertTrue(metricsRegistry.createUpDownCounter("test", "test", "test") == NoopCounter.INSTANCE); } - public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { + public void testGetMetricsWithAvailableMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); @@ -59,7 +59,7 @@ public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.of(mockTelemetry)); MetricsRegistry metricsRegistry = metricsRegistryFactory.getMetricsRegistry(); - assertTrue(metricsRegistry instanceof WrappedMetricsRegistry); + assertTrue(metricsRegistry instanceof DefaultMetricsRegistry); } diff --git a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java deleted file mode 100644 index e01d200bb8527..0000000000000 --- a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * 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.settings.ClusterSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.tags.Tags; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; - -public class WrappedCounterTests extends OpenSearchTestCase { - - public void testCounterWithDisabledMetrics() throws Exception { - Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); - Counter mockCounter = mock(Counter.class); - - WrappedCounter wrappedCounter = new WrappedCounter(telemetrySettings, mockCounter); - wrappedCounter.add(1.0); - verify(mockCounter, never()).add(1.0); - - Tags tags = Tags.create().addTag("test", "test"); - wrappedCounter.add(1.0, tags); - verify(mockCounter, never()).add(1.0, tags); - - } - - public void testCounterWithEnabledMetrics() throws Exception { - Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), true).build(); - TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); - Counter mockCounter = mock(Counter.class); - - WrappedCounter wrappedCounter = new WrappedCounter(telemetrySettings, mockCounter); - wrappedCounter.add(1.0); - verify(mockCounter).add(1.0); - - Tags tags = Tags.create().addTag("test", "test"); - wrappedCounter.add(1.0, tags); - verify(mockCounter).add(1.0, tags); - } - - private Set> getClusterSettings() { - Set> allTracerSettings = new HashSet<>(); - ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); - return allTracerSettings; - } -} diff --git a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java deleted file mode 100644 index 119f6536b5d2e..0000000000000 --- a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * 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.settings.ClusterSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class WrappedMetricsRegistryTests extends OpenSearchTestCase { - - public void testCounter() throws Exception { - Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); - MetricsTelemetry mockMetricsTelemetry = mock(MetricsTelemetry.class); - Counter mockCounter = mock(Counter.class); - Counter mockUpDownCounter = mock(Counter.class); - DefaultMetricsRegistry defaultMeterRegistry = new DefaultMetricsRegistry(mockMetricsTelemetry); - - when(mockMetricsTelemetry.createCounter("test", "test", "test")).thenReturn(mockCounter); - when(mockMetricsTelemetry.createUpDownCounter("test", "test", "test")).thenReturn(mockUpDownCounter); - - WrappedMetricsRegistry wrappedMeterRegistry = new WrappedMetricsRegistry(telemetrySettings, defaultMeterRegistry); - assertTrue(wrappedMeterRegistry.createCounter("test", "test", "test") instanceof WrappedCounter); - assertTrue(wrappedMeterRegistry.createUpDownCounter("test", "test", "test") instanceof WrappedCounter); - } - - private Set> getClusterSettings() { - Set> allTracerSettings = new HashSet<>(); - ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); - return allTracerSettings; - } - -}