From 9a67369e484b9b5249fe5740bb6c228e8aab8652 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Mon, 16 Oct 2023 23:41:14 +0530 Subject: [PATCH] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/tracing/DefaultTracer.java | 5 ++ .../opensearch/telemetry/tracing/Tracer.java | 6 ++ .../telemetry/tracing/noop/NoopTracer.java | 5 ++ .../telemetry/tracing/DefaultTracerTests.java | 1 + .../TelemetryMetricsEnabledCounterIT.java | 22 ++++++ ...elemetryMetricsEnabledUpDownCounterIT.java | 72 ------------------- .../telemetry/OTelTelemetryPluginTests.java | 5 +- .../sampler/ProbabilisticSamplerTests.java | 4 +- .../main/java/org/opensearch/node/Node.java | 6 +- .../telemetry/TelemetrySettings.java | 26 ++----- .../telemetry/tracing/WrappedTracer.java | 5 ++ .../channels/TraceableHttpChannel.java | 4 +- .../channels/TraceableRestChannel.java | 4 +- .../TraceableTcpTransportChannel.java | 4 +- .../TraceableTransportResponseHandler.java | 4 +- .../listener/TraceableActionListener.java | 4 +- .../metrics/MetricsRegistryFactoryTests.java | 6 +- .../telemetry/tracing/TracerFactoryTests.java | 8 +-- .../telemetry/tracing/WrappedTracerTests.java | 11 +-- 19 files changed, 76 insertions(+), 126 deletions(-) delete mode 100644 plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledUpDownCounterIT.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java index 79b7e4aca6c2f..a3bb64ea392a9 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/DefaultTracer.java @@ -85,6 +85,11 @@ public SpanScope withSpanInScope(Span span) { return DefaultSpanScope.create(span, tracerContextStorage).attach(); } + @Override + public boolean isRecording() { + return true; + } + private Span createSpan(SpanCreationContext spanCreationContext, Span parentSpan) { return tracingTelemetry.createSpan(spanCreationContext, parentSpan); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java index 49295f417df9e..d885e709d185b 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/Tracer.java @@ -53,4 +53,10 @@ public interface Tracer extends HttpTracer, Closeable { */ SpanScope withSpanInScope(Span span); + /** + * Tells if the traces are being recorded or not + * @return boolean + */ + boolean isRecording(); + } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java index c073e8d3e766f..50452ff5fe3b4 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/noop/NoopTracer.java @@ -54,6 +54,11 @@ public SpanScope withSpanInScope(Span span) { return SpanScope.NO_OP; } + @Override + public boolean isRecording() { + return false; + } + @Override public void close() { diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java index 0a717e993cb81..2a791f1ae4164 100644 --- a/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/tracing/DefaultTracerTests.java @@ -62,6 +62,7 @@ public void testCreateSpan() { String spanName = defaultTracer.getCurrentSpan().getSpan().getSpanName(); assertEquals("span_name", spanName); + assertTrue(defaultTracer.isRecording()); } @SuppressWarnings("unchecked") diff --git a/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledCounterIT.java b/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledCounterIT.java index 4fa739cb15967..ac6aa09fe604c 100644 --- a/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledCounterIT.java +++ b/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledCounterIT.java @@ -15,6 +15,7 @@ import org.opensearch.telemetry.OTelTelemetrySettings; import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.test.OpenSearchSingleNodeTestCase; +import org.junit.After; import java.util.ArrayList; import java.util.Arrays; @@ -60,4 +61,25 @@ public void testCounter() throws Exception { .getValue(); assertEquals(1.0, value, 0.0); } + + public void testUpDownCounter() throws Exception { + + MetricsRegistry metricsRegistry = node().injector().getInstance(MetricsRegistry.class); + + Counter counter = metricsRegistry.createUpDownCounter("test-up-down-counter", "test", "1"); + counter.add(1.0); + counter.add(-2.0); + // Sleep for about 2s to wait for metrics to be published. + Thread.sleep(2000); + + InMemorySingletonMetricsExporter exporter = InMemorySingletonMetricsExporter.INSTANCE; + double value = ((DoublePointData) ((ArrayList) exporter.getFinishedMetricItems().get(0).getDoubleSumData().getPoints()).get(0)) + .getValue(); + assertEquals(-1.0, value, 0.0); + } + + @After + public void reset() { + InMemorySingletonMetricsExporter.INSTANCE.reset(); + } } diff --git a/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledUpDownCounterIT.java b/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledUpDownCounterIT.java deleted file mode 100644 index e95d930dc9ddc..0000000000000 --- a/plugins/telemetry-otel/src/internalClusterTest/java/org/opensearch/telemetry/metrics/TelemetryMetricsEnabledUpDownCounterIT.java +++ /dev/null @@ -1,72 +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.Settings; -import org.opensearch.common.unit.TimeValue; -import org.opensearch.plugins.Plugin; -import org.opensearch.telemetry.IntegrationTestOTelTelemetryPlugin; -import org.opensearch.telemetry.OTelTelemetrySettings; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.test.OpenSearchSingleNodeTestCase; -import org.junit.After; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; - -import io.opentelemetry.sdk.metrics.data.DoublePointData; - -public class TelemetryMetricsEnabledUpDownCounterIT extends OpenSearchSingleNodeTestCase { - - @Override - protected Settings nodeSettings() { - return Settings.builder() - .put(super.nodeSettings()) - .put(TelemetrySettings.METRICS_FEATURE_ENABLED_SETTING.getKey(), true) - .put( - OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.getKey(), - "org.opensearch.telemetry.metrics.InMemorySingletonMetricsExporter" - ) - .put(TelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING.getKey(), TimeValue.timeValueSeconds(1)) - .build(); - } - - @Override - protected Collection> getPlugins() { - return Arrays.asList(IntegrationTestOTelTelemetryPlugin.class); - } - - @Override - protected boolean addMockTelemetryPlugin() { - return false; - } - - public void testUpDownCounter() throws Exception { - - MetricsRegistry metricsRegistry = node().injector().getInstance(MetricsRegistry.class); - - Counter counter = metricsRegistry.createUpDownCounter("test-up-down-counter", "test", "1"); - counter.add(1.0); - counter.add(-2.0); - // Sleep for about 2s to wait for metrics to be published. - Thread.sleep(2000); - - InMemorySingletonMetricsExporter exporter = InMemorySingletonMetricsExporter.INSTANCE; - double value = ((DoublePointData) ((ArrayList) exporter.getFinishedMetricItems().get(0).getDoubleSumData().getPoints()).get(0)) - .getValue(); - assertEquals(-1.0, value, 0.0); - } - - @After - public void reset() { - InMemorySingletonMetricsExporter.INSTANCE.reset(); - } - -} 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 7da48cfba6657..2fcf89947e537 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 @@ -51,10 +51,7 @@ public void setup() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); oTelTelemetryPlugin = new OTelTelemetryPlugin(settings); telemetry = oTelTelemetryPlugin.getTelemetry( - TelemetrySettings.create( - Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY)) - ) + 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/tracing/sampler/ProbabilisticSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java index 8112d94b76374..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 @@ -31,7 +31,7 @@ public void testProbabilisticSamplerWithNullSettings() { public void testDefaultGetSampler() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create( + TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) ); @@ -45,7 +45,7 @@ public void testDefaultGetSampler() { public void testGetSamplerWithUpdatedSamplingRatio() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create( + TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) ); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index ed1d0d2436a5b..633e0030d29de 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -598,15 +598,15 @@ protected Node( TracerFactory tracerFactory; MetricsRegistryFactory metricsRegistryFactory; if (FeatureFlags.isEnabled(TELEMETRY)) { - final TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, clusterService.getClusterSettings()); + final TelemetrySettings telemetrySettings = new TelemetrySettings(settings, clusterService.getClusterSettings()); List telemetryPlugins = pluginsService.filterPlugins(TelemetryPlugin.class); TelemetryModule telemetryModule = new TelemetryModule(telemetryPlugins, telemetrySettings); - if (TelemetrySettings.isTracerFeatureEnabled()) { + if (telemetrySettings.isTracerFeatureEnabled()) { tracerFactory = new TracerFactory(telemetrySettings, telemetryModule.getTelemetry(), threadPool.getThreadContext()); } else { tracerFactory = new NoopTracerFactory(); } - if (TelemetrySettings.isMetricsFeatureEnabled()) { + if (telemetrySettings.isMetricsFeatureEnabled()) { metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, telemetryModule.getTelemetry()); } else { metricsRegistryFactory = new NoopMetricsRegistryFactory(); diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index 18ad3ba8b743b..341459e4d471a 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -67,14 +67,10 @@ public class TelemetrySettings { private volatile boolean tracingEnabled; private volatile double samplingProbability; - private static Settings settingz; + private Settings settingz; - public static TelemetrySettings create(Settings settings, ClusterSettings clusterSettings) { - settingz = settings; - return new TelemetrySettings(settings, clusterSettings); - } - - private TelemetrySettings(Settings settings, ClusterSettings clusterSettings) { + public TelemetrySettings(Settings settings, ClusterSettings clusterSettings) { + this.settingz = settings; this.tracingEnabled = TRACER_ENABLED_SETTING.get(settings); this.samplingProbability = TRACER_SAMPLER_PROBABILITY.get(settings); @@ -105,19 +101,11 @@ public double getSamplingProbability() { return samplingProbability; } - public static boolean isTracerFeatureEnabled() { - if (settingz != null) { - return TRACER_FEATURE_ENABLED_SETTING.get(settingz); - } else { - return false; - } + public boolean isTracerFeatureEnabled() { + return TRACER_FEATURE_ENABLED_SETTING.get(settingz); } - public static boolean isMetricsFeatureEnabled() { - if (settingz != null) { - return METRICS_FEATURE_ENABLED_SETTING.get(settingz); - } else { - return false; - } + public boolean isMetricsFeatureEnabled() { + return METRICS_FEATURE_ENABLED_SETTING.get(settingz); } } diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java index b2308402379ac..631fb8242d78e 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/WrappedTracer.java @@ -59,6 +59,11 @@ public SpanScope withSpanInScope(Span span) { return getDelegateTracer().withSpanInScope(span); } + @Override + public boolean isRecording() { + return getDelegateTracer().isRecording(); + } + @Override public void close() throws IOException { defaultTracer.close(); diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableHttpChannel.java b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableHttpChannel.java index 2d615a17a2c21..c985b123f34b3 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableHttpChannel.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableHttpChannel.java @@ -8,11 +8,9 @@ package org.opensearch.telemetry.tracing.channels; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.action.ActionListener; import org.opensearch.http.HttpChannel; import org.opensearch.http.HttpResponse; -import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.telemetry.tracing.listener.TraceableActionListener; @@ -50,7 +48,7 @@ private TraceableHttpChannel(HttpChannel delegate, Span span, Tracer tracer) { * @return http channel */ public static HttpChannel create(HttpChannel delegate, Span span, Tracer tracer) { - if (FeatureFlags.isEnabled(FeatureFlags.TELEMETRY) == true && TelemetrySettings.isTracerFeatureEnabled()) { + if (tracer.isRecording()) { return new TraceableHttpChannel(delegate, span, tracer); } else { return delegate; diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableRestChannel.java b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableRestChannel.java index 316e6bd76bbc8..1a7c5e66e392a 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableRestChannel.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableRestChannel.java @@ -9,13 +9,11 @@ package org.opensearch.telemetry.tracing.channels; import org.opensearch.common.io.stream.BytesStreamOutput; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestResponse; -import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.Tracer; @@ -54,7 +52,7 @@ private TraceableRestChannel(RestChannel delegate, Span span, Tracer tracer) { * @return rest channel */ public static RestChannel create(RestChannel delegate, Span span, Tracer tracer) { - if (FeatureFlags.isEnabled(FeatureFlags.TELEMETRY) == true && TelemetrySettings.isTracerFeatureEnabled()) { + if (tracer.isRecording()) { return new TraceableRestChannel(delegate, span, tracer); } else { return delegate; diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableTcpTransportChannel.java b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableTcpTransportChannel.java index 1e2f2a3d97e7d..974a436ece3bf 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableTcpTransportChannel.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/channels/TraceableTcpTransportChannel.java @@ -9,10 +9,8 @@ package org.opensearch.telemetry.tracing.channels; import org.opensearch.Version; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.action.ActionListener; import org.opensearch.core.transport.TransportResponse; -import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.Tracer; @@ -53,7 +51,7 @@ public TraceableTcpTransportChannel(TcpTransportChannel delegate, Span span, Tra * @return transport channel */ public static TransportChannel create(TcpTransportChannel delegate, final Span span, final Tracer tracer) { - if (FeatureFlags.isEnabled(FeatureFlags.TELEMETRY) == true && TelemetrySettings.isTracerFeatureEnabled()) { + if (tracer.isRecording()) { delegate.getChannel().addCloseListener(new ActionListener() { @Override public void onResponse(Void unused) { diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/handler/TraceableTransportResponseHandler.java b/server/src/main/java/org/opensearch/telemetry/tracing/handler/TraceableTransportResponseHandler.java index bb4b1c7a9ae0a..809043dfdea31 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/handler/TraceableTransportResponseHandler.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/handler/TraceableTransportResponseHandler.java @@ -8,10 +8,8 @@ package org.opensearch.telemetry.tracing.handler; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.transport.TransportResponse; -import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.Tracer; @@ -56,7 +54,7 @@ public static TransportResponseHandler create( Span span, Tracer tracer ) { - if (FeatureFlags.isEnabled(FeatureFlags.TELEMETRY) == true && TelemetrySettings.isTracerFeatureEnabled()) { + if (tracer.isRecording()) { return new TraceableTransportResponseHandler(delegate, span, tracer); } else { return delegate; diff --git a/server/src/main/java/org/opensearch/telemetry/tracing/listener/TraceableActionListener.java b/server/src/main/java/org/opensearch/telemetry/tracing/listener/TraceableActionListener.java index 22c496b768120..3997b34de7406 100644 --- a/server/src/main/java/org/opensearch/telemetry/tracing/listener/TraceableActionListener.java +++ b/server/src/main/java/org/opensearch/telemetry/tracing/listener/TraceableActionListener.java @@ -8,9 +8,7 @@ package org.opensearch.telemetry.tracing.listener; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.action.ActionListener; -import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.tracing.Span; import org.opensearch.telemetry.tracing.SpanScope; import org.opensearch.telemetry.tracing.Tracer; @@ -48,7 +46,7 @@ private TraceableActionListener(ActionListener delegate, Span span, Tr * @return action listener */ public static ActionListener create(ActionListener delegate, Span span, Tracer tracer) { - if (FeatureFlags.isEnabled(FeatureFlags.TELEMETRY) == true && TelemetrySettings.isTracerFeatureEnabled()) { + if (tracer.isRecording()) { return new TraceableActionListener(delegate, span, tracer); } else { return delegate; 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 f0aec29b6e1e5..80942123fd4fd 100644 --- a/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java @@ -39,7 +39,7 @@ public void close() { public void testGetMeterRegistryWithUnavailableMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.empty()); @@ -53,7 +53,7 @@ public void testGetMeterRegistryWithUnavailableMetricsTelemetry() { public void testGetMetricsWithAvailableMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getMetricsTelemetry()).thenReturn(mock(MetricsTelemetry.class)); metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.of(mockTelemetry)); @@ -65,7 +65,7 @@ public void testGetMetricsWithAvailableMetricsTelemetry() { public void testNullMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.METRICS_FEATURE_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getMetricsTelemetry()).thenReturn(null); metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.of(mockTelemetry)); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java index 84581e111a024..3a388be22445e 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/TracerFactoryTests.java @@ -40,7 +40,7 @@ public void close() { public void testGetTracerWithUnavailableTracingTelemetryReturnsNoopTracer() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); tracerFactory = new TracerFactory(telemetrySettings, Optional.empty(), new ThreadContext(Settings.EMPTY)); @@ -60,7 +60,7 @@ public void testGetTracerWithUnavailableTracingTelemetryReturnsNoopTracer() { public void testGetTracerWithUnavailableTracingTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); tracerFactory = new TracerFactory(telemetrySettings, Optional.empty(), new ThreadContext(Settings.EMPTY)); @@ -73,7 +73,7 @@ public void testGetTracerWithUnavailableTracingTelemetry() { public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); tracerFactory = new TracerFactory(telemetrySettings, Optional.of(mockTelemetry), new ThreadContext(Settings.EMPTY)); @@ -85,7 +85,7 @@ public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { public void testNullTracer() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_FEATURE_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); when(mockTelemetry.getTracingTelemetry()).thenReturn(null); tracerFactory = new TracerFactory(telemetrySettings, Optional.of(mockTelemetry), new ThreadContext(Settings.EMPTY)); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java index 645384b4bce0b..8606104d26103 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/WrappedTracerTests.java @@ -26,39 +26,42 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class WrappedTracerTests extends OpenSearchTestCase { public void testStartSpanWithTracingDisabledInvokesNoopTracer() throws Exception { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); DefaultTracer mockDefaultTracer = mock(DefaultTracer.class); try (WrappedTracer wrappedTracer = new WrappedTracer(telemetrySettings, mockDefaultTracer)) { SpanCreationContext spanCreationContext = SpanCreationContext.internal().name("foo"); wrappedTracer.startSpan(spanCreationContext); assertTrue(wrappedTracer.getDelegateTracer() instanceof NoopTracer); + assertFalse(wrappedTracer.isRecording()); verify(mockDefaultTracer, never()).startSpan(SpanCreationContext.internal().name("foo")); } } public void testStartSpanWithTracingEnabledInvokesDefaultTracer() throws Exception { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); DefaultTracer mockDefaultTracer = mock(DefaultTracer.class); - + when(mockDefaultTracer.isRecording()).thenReturn(true); try (WrappedTracer wrappedTracer = new WrappedTracer(telemetrySettings, mockDefaultTracer)) { SpanCreationContext spanCreationContext = SpanCreationContext.internal().name("foo"); wrappedTracer.startSpan(spanCreationContext); assertTrue(wrappedTracer.getDelegateTracer() instanceof DefaultTracer); + assertTrue(wrappedTracer.isRecording()); verify(mockDefaultTracer).startSpan(eq(spanCreationContext)); } } public void testStartSpanWithTracingEnabledInvokesDefaultTracerWithAttr() throws Exception { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); - TelemetrySettings telemetrySettings = TelemetrySettings.create(settings, new ClusterSettings(settings, getClusterSettings())); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); DefaultTracer mockDefaultTracer = mock(DefaultTracer.class); Attributes attributes = Attributes.create().addAttribute("key", "value"); try (WrappedTracer wrappedTracer = new WrappedTracer(telemetrySettings, mockDefaultTracer)) {