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 14a19f122c17b..5881753876a34 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 @@ -12,7 +12,6 @@ import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.metrics.exporter.OTelMetricsExporterFactory; import org.opensearch.telemetry.tracing.exporter.OTelSpanExporterFactory; -import org.opensearch.telemetry.tracing.sampler.ProbabilisticSampler; import org.opensearch.telemetry.tracing.sampler.RequestSampler; import java.security.AccessController; @@ -54,7 +53,7 @@ public static OpenTelemetrySdk get(TelemetrySettings telemetrySettings, Settings settings, OTelSpanExporterFactory.create(settings), ContextPropagators.create(W3CTraceContextPropagator.getInstance()), - Sampler.parentBased(new RequestSampler(new ProbabilisticSampler(telemetrySettings))) + Sampler.parentBased(new RequestSampler(telemetrySettings)) ) ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSampler.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSampler.java index 30c9f03bb1fd5..3a8dcb5451971 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSampler.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSampler.java @@ -10,34 +10,24 @@ import org.opensearch.telemetry.TelemetrySettings; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; -import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.trace.Span; -import io.opentelemetry.api.trace.SpanContext; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.data.LinkData; import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.sdk.trace.samplers.SamplingResult; -import static org.opensearch.telemetry.tracing.AttributeNames.TRANSPORT_ACTION; - /** - * ProbabilisticSampler implements a head-based sampling strategy based on provided settings. + * ProbabilisticSampler implements a probability sampling strategy based on configured sampling ratio. */ public class ProbabilisticSampler implements Sampler { private Sampler defaultSampler; private final TelemetrySettings telemetrySettings; private double samplingRatio; - private final Map actionBasedSampler; - private final Map actionBasedSamplingProbability; - /** * Constructor * @@ -47,56 +37,12 @@ public ProbabilisticSampler(TelemetrySettings telemetrySettings) { this.telemetrySettings = Objects.requireNonNull(telemetrySettings); this.samplingRatio = telemetrySettings.getSamplingProbability(); this.defaultSampler = Sampler.traceIdRatioBased(samplingRatio); - this.actionBasedSampler = new HashMap<>(); - this.actionBasedSamplingProbability = new HashMap<>(); - } - - /** - * Returns custom sampler based on the type of request - * @param attributes Telemetry attributes - */ - Sampler getSampler(Attributes attributes) { - // Evaluate if sampling is overridden at action level - final String action = attributes.get(AttributeKey.stringKey(TRANSPORT_ACTION)); - if (action != null && telemetrySettings.isActionSamplingOverrideSet(action)) { - if (isActionSamplerUpdateRequired(action)) { - synchronized (this) { - updateActionSampler(action); - } - } - return actionBasedSampler.get(action); - } else { - // Update default sampling if no override is present for action - double newSamplingRatio = telemetrySettings.getSamplingProbability(); - if (isSamplingRatioChanged(newSamplingRatio)) { - synchronized (this) { - this.samplingRatio = newSamplingRatio; - defaultSampler = Sampler.traceIdRatioBased(samplingRatio); - } - } - return defaultSampler; - } } private boolean isSamplingRatioChanged(double newSamplingRatio) { return Double.compare(this.samplingRatio, newSamplingRatio) != 0; } - private boolean isActionSamplerUpdateRequired(String action) { - return (!actionBasedSampler.containsKey(action) - || (actionBasedSamplingProbability.get(action) != telemetrySettings.getActionSamplingProbability(action))); - } - - private void updateActionSampler(String action) { - double samplingRatio = telemetrySettings.getActionSamplingProbability(action); - this.actionBasedSamplingProbability.put(action, samplingRatio); - this.actionBasedSampler.put(action, Sampler.traceIdRatioBased(samplingRatio)); - } - - double getActionSamplingRatio(String action) { - return actionBasedSamplingProbability.get(action); - } - double getSamplingRatio() { return samplingRatio; } @@ -110,10 +56,12 @@ public SamplingResult shouldSample( Attributes attributes, List parentLinks ) { - // Use action sampler only if it is root span & override is present for the action - SpanContext parentSpanContext = Span.fromContext(parentContext).getSpanContext(); - if (!parentSpanContext.isValid()) { - return getSampler(attributes).shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + double newSamplingRatio = telemetrySettings.getSamplingProbability(); + if (isSamplingRatioChanged(newSamplingRatio)) { + synchronized (this) { + this.samplingRatio = newSamplingRatio; + defaultSampler = Sampler.traceIdRatioBased(samplingRatio); + } } return defaultSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/RequestSampler.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/RequestSampler.java index 9ea681370a3ec..7be48dcba64d1 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/RequestSampler.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/RequestSampler.java @@ -8,6 +8,8 @@ package org.opensearch.telemetry.tracing.sampler; +import org.opensearch.telemetry.TelemetrySettings; + import java.util.List; import io.opentelemetry.api.common.AttributeKey; @@ -18,21 +20,24 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.sdk.trace.samplers.SamplingResult; +import static org.opensearch.telemetry.tracing.AttributeNames.TRACE; +import static org.opensearch.telemetry.tracing.AttributeNames.TRANSPORT_ACTION; + /** * HeadBased sampler */ public class RequestSampler implements Sampler { private final Sampler defaultSampler; - // TODO: Pick value of TRACE from PR #9415. - private static final String TRACE = "trace"; + private final Sampler actionSampler; /** - * Creates Head based sampler - * @param defaultSampler defaultSampler + * Creates action sampler which samples request for all actions based on defined probability + * @param telemetrySettings TelemetrySettings */ - public RequestSampler(Sampler defaultSampler) { - this.defaultSampler = defaultSampler; + public RequestSampler(TelemetrySettings telemetrySettings) { + this.defaultSampler = new ProbabilisticSampler(telemetrySettings); + this.actionSampler = new TransportActionSampler(telemetrySettings); } @Override @@ -44,15 +49,18 @@ public SamplingResult shouldSample( Attributes attributes, List parentLinks ) { - final String trace = attributes.get(AttributeKey.stringKey(TRACE)); + final String action = attributes.get(AttributeKey.stringKey(TRANSPORT_ACTION)); + // Determine the sampling decision based on the availability of trace information and action, + // either recording and sampling, delegating to action sampler, or falling back to default sampler. if (trace != null) { return (Boolean.parseBoolean(trace) == true) ? SamplingResult.recordAndSample() : SamplingResult.drop(); + } else if (action != null) { + return actionSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); } else { return defaultSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); } - } @Override diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/TransportActionSampler.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/TransportActionSampler.java new file mode 100644 index 0000000000000..f89756aa7cba0 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/TransportActionSampler.java @@ -0,0 +1,84 @@ +/* + * 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.tracing.sampler; + +import org.opensearch.telemetry.TelemetrySettings; + +import java.util.List; +import java.util.Objects; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.trace.data.LinkData; +import io.opentelemetry.sdk.trace.samplers.Sampler; +import io.opentelemetry.sdk.trace.samplers.SamplingResult; + +import static org.opensearch.telemetry.tracing.AttributeNames.TRANSPORT_ACTION; + +/** + * TransportActionSampler sampler samples request with action based on defined probability + */ +public class TransportActionSampler implements Sampler { + private Sampler actionSampler; + private final TelemetrySettings telemetrySettings; + private double actionSamplingRatio; + + /** + * Creates TransportActionSampler sampler + * @param telemetrySettings TelemetrySettings + */ + public TransportActionSampler(TelemetrySettings telemetrySettings) { + this.telemetrySettings = Objects.requireNonNull(telemetrySettings); + this.actionSamplingRatio = telemetrySettings.getActionSamplingProbability(); + this.actionSampler = Sampler.traceIdRatioBased(actionSamplingRatio); + } + + @Override + public SamplingResult shouldSample( + Context parentContext, + String traceId, + String name, + SpanKind spanKind, + Attributes attributes, + List parentLinks + ) { + final String action = attributes.get(AttributeKey.stringKey(TRANSPORT_ACTION)); + if (action != null) { + double newActionSamplingRatio = telemetrySettings.getActionSamplingProbability(); + if (isSamplingRatioChanged(newActionSamplingRatio)) { + synchronized (this) { + this.actionSamplingRatio = newActionSamplingRatio; + actionSampler = Sampler.traceIdRatioBased(actionSamplingRatio); + } + } + return actionSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + } + return SamplingResult.drop(); + } + + private boolean isSamplingRatioChanged(double newSamplingRatio) { + return Double.compare(this.actionSamplingRatio, newSamplingRatio) != 0; + } + + double getSamplingRatio() { + return actionSamplingRatio; + } + + @Override + public String getDescription() { + return "Transport Action Sampler"; + } + + @Override + public String toString() { + return getDescription(); + } +} 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 8d8028162ae85..fcbb185fee49a 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 @@ -13,18 +13,16 @@ import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.test.OpenSearchTestCase; -import java.util.HashMap; -import java.util.Map; import java.util.Set; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.sdk.internal.AttributesMap; -import io.opentelemetry.sdk.trace.samplers.Sampler; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.context.Context; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_ACTION_PROBABILITY; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; +import static org.mockito.Mockito.mock; public class ProbabilisticSamplerTests extends OpenSearchTestCase { @@ -41,12 +39,9 @@ public void testDefaultGetSampler() { new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, TRACER_SAMPLER_ACTION_PROBABILITY)) ); - AttributesMap attributes = AttributesMap.create(1, 100); - // Probabilistic Sampler ProbabilisticSampler probabilisticSampler = new ProbabilisticSampler(telemetrySettings); - assertNotNull(probabilisticSampler.getSampler(attributes)); assertEquals(0.01, probabilisticSampler.getSamplingRatio(), 0.0d); } @@ -57,54 +52,17 @@ public void testGetSamplerWithUpdatedSamplingRatio() { new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, TRACER_SAMPLER_ACTION_PROBABILITY)) ); - AttributesMap attributes = AttributesMap.create(1, 100); - // Probabilistic Sampler ProbabilisticSampler probabilisticSampler = new ProbabilisticSampler(telemetrySettings); - assertNotNull(probabilisticSampler.getSampler(attributes)); assertEquals(0.01d, probabilisticSampler.getSamplingRatio(), 0.0d); telemetrySettings.setSamplingProbability(0.02); - // Need to call getSampler() to update the value of tracerHeadSamplerSamplingRatio - Sampler updatedProbabilisticSampler = probabilisticSampler.getSampler(attributes); - assertEquals(0.02, probabilisticSampler.getSamplingRatio(), 0.0d); - } + // Need to call shouldSample() to update the value of samplingRatio + probabilisticSampler.shouldSample(mock(Context.class), "00000000000000000000000000000000", "", SpanKind.INTERNAL, null, null); - public void testGetSamplerWithCustomActionSamplingRatio() { - 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, TRACER_SAMPLER_ACTION_PROBABILITY)) - ); - telemetrySettings.setSamplingProbability(0.02); - AttributesMap attributes = AttributesMap.create(1, 100); - - // Probabilistic Sampler - ProbabilisticSampler probabilisticSampler = new ProbabilisticSampler(telemetrySettings); - - Map filters = new HashMap<>(); - // Setting 100% sampling for cluster/coordination/join request - filters.put("internal:cluster/coordination/join", 1.00); - telemetrySettings.setActionSamplingProbability(filters); - - attributes.put(AttributeKey.stringKey("action"), "internal:cluster/coordination/join"); - probabilisticSampler.getSampler(attributes); - - // Validates sampling probability for cluster coordination request as override is present for it. - assertEquals(1.00, probabilisticSampler.getActionSamplingRatio("internal:cluster/coordination/join"), 0.0d); - // Validates sampling probability for cluster coordination request second call. - assertEquals(1.00, probabilisticSampler.getActionSamplingRatio("internal:cluster/coordination/join"), 0.0d); - - // Updating sampling ratio to 30% - filters.put("internal:cluster/coordination/join", 0.30); - telemetrySettings.setActionSamplingProbability(filters); // Need to call getSampler() to update the value of tracerHeadSamplerSamplingRatio - probabilisticSampler.getSampler(attributes); - - // Validates sampling probability for cluster coordination as override is present for it. - assertEquals(0.30, probabilisticSampler.getActionSamplingRatio("internal:cluster/coordination/join"), 0.0d); + assertEquals(0.02, probabilisticSampler.getSamplingRatio(), 0.0d); } - } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/RequestSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/RequestSamplerTests.java index facf04623ec46..baa54e59e7280 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/RequestSamplerTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/RequestSamplerTests.java @@ -8,36 +8,39 @@ package org.opensearch.telemetry.tracing.sampler; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.test.OpenSearchTestCase; import java.util.Collections; +import java.util.Set; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.context.Context; -import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.sdk.trace.samplers.SamplingResult; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; +import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_ACTION_PROBABILITY; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; +import static org.opensearch.telemetry.tracing.AttributeNames.TRANSPORT_ACTION; 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 RequestSamplerTests extends OpenSearchTestCase { public void testShouldSampleWithTraceAttributeAsTrue() { + 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, TRACER_SAMPLER_ACTION_PROBABILITY)) + ); + // Create an instance of requestSampler + RequestSampler requestSampler = new RequestSampler(telemetrySettings); - // Create a mock default sampler - Sampler defaultSampler = mock(Sampler.class); - when(defaultSampler.shouldSample(any(), anyString(), anyString(), any(), any(), any())).thenReturn(SamplingResult.drop()); - - // Create an instance of HeadSampler with the mock default sampler - RequestSampler requestSampler = new RequestSampler(defaultSampler); - - // Create a mock Context and Attributes + // Create a mock Context and Attributes with trace as true Context parentContext = mock(Context.class); Attributes attributes = Attributes.of(AttributeKey.stringKey("trace"), "true"); @@ -52,25 +55,21 @@ public void testShouldSampleWithTraceAttributeAsTrue() { ); assertEquals(SamplingResult.recordAndSample(), result); - - // Verify that the default sampler's shouldSample method was not called - verify(defaultSampler, never()).shouldSample(any(), anyString(), anyString(), any(), any(), any()); } - public void testShouldSampleWithoutTraceAttribute() { - - // Create a mock default sampler - Sampler defaultSampler = mock(Sampler.class); - when(defaultSampler.shouldSample(any(), anyString(), anyString(), any(), any(), any())).thenReturn( - SamplingResult.recordAndSample() + public void testShouldSampleWithTraceAttributeAsFalse() { + 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, TRACER_SAMPLER_ACTION_PROBABILITY)) ); - // Create an instance of HeadSampler with the mock default sampler - RequestSampler requestSampler = new RequestSampler(defaultSampler); + // Create an instance of requestSampler + RequestSampler requestSampler = new RequestSampler(telemetrySettings); - // Create a mock Context and Attributes + // Create a mock Context and Attributes with trace as false Context parentContext = mock(Context.class); - Attributes attributes = Attributes.empty(); + Attributes attributes = Attributes.of(AttributeKey.stringKey("trace"), "false"); // Call shouldSample on HeadSampler SamplingResult result = requestSampler.shouldSample( @@ -81,12 +80,74 @@ public void testShouldSampleWithoutTraceAttribute() { attributes, Collections.emptyList() ); + assertEquals(SamplingResult.drop(), result); + } + + public void testShouldSampleWithoutTraceAttribute() { + ClusterSettings clusterSettings = new ClusterSettings( + Settings.EMPTY, + Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, TRACER_SAMPLER_ACTION_PROBABILITY) + ); + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + + // Create an instance of requestSampler + RequestSampler requestSampler = new RequestSampler(telemetrySettings); + + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "1.0").build()); + + // Create a mock Context and Attributes with dummy action + Context parentContext = mock(Context.class); + Attributes attributes = Attributes.builder().put(TRANSPORT_ACTION, "dummy_action").build(); + + // Calling shouldSample to update samplingRatio + SamplingResult result = requestSampler.shouldSample( + parentContext, + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + attributes, + Collections.emptyList() + ); - // Verify that HeadSampler returned SamplingResult.recordAndSample() + // Verify that request is sampled assertEquals(SamplingResult.recordAndSample(), result); - // Verify that the default sampler's shouldSample method was called - verify(defaultSampler).shouldSample(any(), anyString(), anyString(), any(), any(), any()); + // Verify that sampler dropped the request + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.0").build()); + result = requestSampler.shouldSample( + parentContext, + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + attributes, + Collections.emptyList() + ); + assertEquals(SamplingResult.drop(), result); + + // Verify that request is sampled when probability is set to 1 + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.sampler.probability", "1.0").build()); + result = requestSampler.shouldSample( + parentContext, + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + Attributes.empty(), + Collections.emptyList() + ); + assertEquals(SamplingResult.recordAndSample(), result); + + // Verify that request is not sampled when probability is set to 0 + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.sampler.probability", "0.0").build()); + result = requestSampler.shouldSample( + parentContext, + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + Attributes.empty(), + Collections.emptyList() + ); + assertEquals(SamplingResult.drop(), result); + } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/TransportActionSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/TransportActionSamplerTests.java new file mode 100644 index 0000000000000..5d89a4de27fdd --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/TransportActionSamplerTests.java @@ -0,0 +1,84 @@ +/* + * 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.tracing.sampler; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Collections; +import java.util.Set; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.trace.samplers.SamplingResult; + +import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_ACTION_PROBABILITY; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; +import static org.opensearch.telemetry.tracing.AttributeNames.TRANSPORT_ACTION; +import static org.mockito.Mockito.mock; + +public class TransportActionSamplerTests extends OpenSearchTestCase { + + public void testGetSamplerWithDefaultActionSamplingRatio() { + 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, TRACER_SAMPLER_ACTION_PROBABILITY)) + ); + + // TransportActionSampler + TransportActionSampler transportActionSampler = new TransportActionSampler(telemetrySettings); + + // Validates if default sampling of 1% + assertEquals(0.001d, transportActionSampler.getSamplingRatio(), 0.0d); + } + + public void testGetSamplerWithUpdatedActionSamplingRatio() { + ClusterSettings clusterSettings = new ClusterSettings( + Settings.EMPTY, + Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, TRACER_SAMPLER_ACTION_PROBABILITY) + ); + + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + + // TransportActionSampler + TransportActionSampler transportActionSampler = new TransportActionSampler(telemetrySettings); + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "1.0").build()); + + // Need to call shouldSample() to update the value of samplingRatio + SamplingResult result = transportActionSampler.shouldSample( + mock(Context.class), + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + Attributes.builder().put(TRANSPORT_ACTION, "dummy_action").build(), + Collections.emptyList() + ); + // Verify that TransportActionSampler returned SamplingResult.recordAndSample() as all actions will be sampled + assertEquals(SamplingResult.recordAndSample(), result); + assertEquals(1.0, transportActionSampler.getSamplingRatio(), 0.000d); + + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.5").build()); + result = transportActionSampler.shouldSample( + mock(Context.class), + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + Attributes.builder().put(TRANSPORT_ACTION, "dummy_action").build(), + Collections.emptyList() + ); + assertEquals(0.5, transportActionSampler.getSamplingRatio(), 0.000d); + } + +} diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index 61fbaa2678bf0..cc8421756e2a5 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -14,9 +14,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import java.util.HashMap; -import java.util.Map; - /** * Wrapper class to encapsulate tracing related settings * @@ -70,29 +67,31 @@ public class TelemetrySettings { /** * Probability of action based sampler */ - public static final Setting.AffixSetting TRACER_SAMPLER_ACTION_PROBABILITY = Setting.affixKeySetting( - "telemetry.tracer.action.sampler.", - "probability", - (ns, key) -> Setting.doubleSetting(key, 0.00d, 0.00d, 1.00d, Setting.Property.Dynamic, Setting.Property.NodeScope) + public static final Setting TRACER_SAMPLER_ACTION_PROBABILITY = Setting.doubleSetting( + "telemetry.tracer.action.sampler.probability", + 0.001d, + 0.000d, + 1.00d, + Setting.Property.NodeScope, + Setting.Property.Dynamic ); private volatile boolean tracingEnabled; private volatile double samplingProbability; - + private volatile double actionSamplingProbability; private final boolean tracingFeatureEnabled; private final boolean metricsFeatureEnabled; - private volatile Map affixSamplingProbability; public TelemetrySettings(Settings settings, ClusterSettings clusterSettings) { this.tracingEnabled = TRACER_ENABLED_SETTING.get(settings); this.samplingProbability = TRACER_SAMPLER_PROBABILITY.get(settings); this.tracingFeatureEnabled = TRACER_FEATURE_ENABLED_SETTING.get(settings); this.metricsFeatureEnabled = METRICS_FEATURE_ENABLED_SETTING.get(settings); - this.affixSamplingProbability = new HashMap<>(); + this.actionSamplingProbability = TRACER_SAMPLER_ACTION_PROBABILITY.get(settings); clusterSettings.addSettingsUpdateConsumer(TRACER_ENABLED_SETTING, this::setTracingEnabled); clusterSettings.addSettingsUpdateConsumer(TRACER_SAMPLER_PROBABILITY, this::setSamplingProbability); - clusterSettings.addAffixMapUpdateConsumer(TRACER_SAMPLER_ACTION_PROBABILITY, this::setActionSamplingProbability, (a, b) -> {}); + clusterSettings.addSettingsUpdateConsumer(TRACER_SAMPLER_ACTION_PROBABILITY, this::setActionSamplingProbability); } public void setTracingEnabled(boolean tracingEnabled) { @@ -112,15 +111,11 @@ public void setSamplingProbability(double samplingProbability) { } /** - * Set sampling ratio for action - * @param filters map of action and corresponding value + * Set sampling ratio for all action + * @param actionSamplingProbability double */ - public void setActionSamplingProbability(Map filters) { - synchronized (this) { - for (String name : filters.keySet()) { - this.affixSamplingProbability.put(name, filters.get(name)); - } - } + public void setActionSamplingProbability(double actionSamplingProbability) { + this.actionSamplingProbability = actionSamplingProbability; } /** @@ -139,21 +134,11 @@ public boolean isMetricsFeatureEnabled() { return metricsFeatureEnabled; } - /** - * Returns if sampling override is set for action - * @param action string - * @return boolean if sampling override is present for an action - */ - public boolean isActionSamplingOverrideSet(String action) { - return affixSamplingProbability.containsKey(action); - } - /** * Get action sampling ratio - * @param action string - * @return double value of sampling probability for that action + * @return double value of sampling probability for actions */ - public double getActionSamplingProbability(String action) { - return this.affixSamplingProbability.get(action); + public double getActionSamplingProbability() { + return this.actionSamplingProbability; } } diff --git a/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java b/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java index 789d7693190ff..d6a09992ce671 100644 --- a/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java +++ b/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java @@ -44,11 +44,11 @@ public void testIsActionSamplingOverrideSet() { TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); // There should be no override for action initially - assertFalse(telemetrySettings.isActionSamplingOverrideSet("dummy_action")); + assertEquals(0.001, telemetrySettings.getActionSamplingProbability(), 0.000d); - clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.dummy_action.probability", "0.01").build()); + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.01").build()); // Validating if value of override for action 'dummy_action' is correct - assertTrue(telemetrySettings.isActionSamplingOverrideSet("dummy_action")); + assertEquals(0.01, telemetrySettings.getActionSamplingProbability(), 0.00d); } public void testSetSamplingProbability() { @@ -80,10 +80,13 @@ public void testGetSamplingProbability() { // Validating default value of Sampling is 1% assertEquals(0.01, telemetrySettings.getSamplingProbability(), 0.00d); - clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.sampler.probability", "0.02").build()); + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.02").build()); - // Validating if default sampling is updated to 2% - assertEquals(0.02, telemetrySettings.getSamplingProbability(), 0.00d); + // Validating if action sampling is updated to 2% + assertEquals(0.02, telemetrySettings.getActionSamplingProbability(), 0.00d); + + // Validating if default sampling is still 1% + assertEquals(0.01, telemetrySettings.getSamplingProbability(), 0.00d); } public void testSetActionSamplingProbability() { @@ -93,10 +96,10 @@ public void testSetActionSamplingProbability() { ); TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); - clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.dummy_action.probability", "0.5").build()); + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.5").build()); // Validating if value of override for action 'dummy_action' is correct - assertEquals(0.5, telemetrySettings.getActionSamplingProbability("dummy_action"), 0.00d); + assertEquals(0.5, telemetrySettings.getActionSamplingProbability(), 0.00d); } @@ -107,13 +110,13 @@ public void testGetActionSamplingProbability() { ); TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); - clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.dummy_action.probability", "0.01").build()); + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.01").build()); // Validating if value of override for action 'dummy_action' is correct i.e. 1% - assertEquals(0.01, telemetrySettings.getActionSamplingProbability("dummy_action"), 0.00d); + assertEquals(0.01, telemetrySettings.getActionSamplingProbability(), 0.00d); - clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.dummy_action.probability", "0.02").build()); + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.action.sampler.probability", "0.02").build()); // Validating if value of override for action 'dummy_action' is correct i.e. 2% - assertEquals(0.02, telemetrySettings.getActionSamplingProbability("dummy_action"), 0.00d); + assertEquals(0.02, telemetrySettings.getActionSamplingProbability(), 0.00d); } }