From 0944a1c2e841561cdc4fafa3a4f84f5dde06211d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 7 Feb 2024 03:57:56 +0000 Subject: [PATCH] Added custom sampler support based on action in request (#10136) * Added custom sampler support based on action in request Signed-off-by: Dev Agarwal * UT Fix Signed-off-by: Dev Agarwal * Added Transport action sampler, which will sample based on different probability for all actions Signed-off-by: Dev Agarwal * Added Transport action sampler, which will sample based on different probability for all actions. Also added setting to define order of samplers Signed-off-by: Dev Agarwal * Added missing java-doc Signed-off-by: Dev Agarwal * Moving sampler class settings to OtelTelemetry setting Signed-off-by: Dev Agarwal * Minor refactor Signed-off-by: Dev Agarwal * Refactored to use chain of samplers Signed-off-by: Dev Agarwal * Addressed comments Signed-off-by: Dev Agarwal * Addressed comments to move action_probability to OtelTelemetrySettings Signed-off-by: Dev Agarwal * Updated eror msg returned when Sampler class is not found Signed-off-by: Dev Agarwal * Added UT for OTelSamplerFactory Signed-off-by: Dev Agarwal * minor refactor Signed-off-by: Dev Agarwal * minor refactor Signed-off-by: Dev Agarwal * spotless check Signed-off-by: Dev Agarwal * Updating OtelTelemetryPlugin.get() method Signed-off-by: Dev Agarwal * Addressed comments Signed-off-by: Dev Agarwal * minor refactor Signed-off-by: Dev Agarwal * addressed comments Signed-off-by: Dev Agarwal * Updated transport action sampler Signed-off-by: Dev Agarwal * Empty-Commit Signed-off-by: Dev Agarwal * Empty-Commit Signed-off-by: Dev Agarwal --------- Signed-off-by: Dev Agarwal (cherry picked from commit 445bf1f6313016e59d7f63ba7725e7088cb83ba3) Signed-off-by: github-actions[bot] --- CHANGELOG.md | 2 + .../telemetry/OTelTelemetryPlugin.java | 4 +- .../telemetry/OTelTelemetrySettings.java | 42 +++++++ .../tracing/OTelResourceProvider.java | 4 +- .../tracing/sampler/OTelSamplerFactory.java | 96 +++++++++++++++ .../tracing/sampler/ProbabilisticSampler.java | 44 +++++-- .../ProbabilisticTransportActionSampler.java | 99 +++++++++++++++ .../tracing/sampler/RequestSampler.java | 25 ++-- .../telemetry/OTelTelemetryPluginTests.java | 6 +- .../sampler/OTelSamplerFactoryTests.java | 42 +++++++ .../sampler/ProbabilisticSamplerTests.java | 22 ++-- ...babilisticTransportActionSamplerTests.java | 52 ++++++++ .../tracing/sampler/RequestSamplerTests.java | 115 ++++++++++++------ .../telemetry/TelemetrySettings.java | 3 +- .../telemetry/TelemetrySettingsTests.java | 64 ++++++++++ 15 files changed, 547 insertions(+), 73 deletions(-) create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactory.java create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSampler.java create mode 100644 plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactoryTests.java create mode 100644 plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSamplerTests.java create mode 100644 server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 67bcad972c367..c14198a448937 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -142,6 +142,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added Support for dynamically adding SearchRequestOperationsListeners with SearchRequestOperationsCompositeListenerFactory ([#11526](https://github.com/opensearch-project/OpenSearch/pull/11526)) - [Query Insights] Query Insights Framework which currently supports retrieving the most time-consuming queries within the last configured time window ([#11903](https://github.com/opensearch-project/OpenSearch/pull/11903)) - [Query Insights] Implement Top N Queries feature to collect and gather information about high latency queries in a window ([#11904](https://github.com/opensearch-project/OpenSearch/pull/11904)) +- Add override support for sampling based on action ([#9621](https://github.com/opensearch-project/OpenSearch/issues/9621)) +- Added custom sampler support based on transport action in request ([#9621](https://github.com/opensearch-project/OpenSearch/issues/9621)) ### Deprecated 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 297ae8873636f..000fd09d43c18 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 @@ -53,7 +53,9 @@ public List> getSettings() { OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING, OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, - OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING + OTelTelemetrySettings.OTEL_TRACER_SPAN_SAMPLER_CLASS_SETTINGS, + OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING, + OTelTelemetrySettings.TRACER_SAMPLER_ACTION_PROBABILITY ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java index b31ed320d737e..95ce6918fcb70 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java @@ -13,15 +13,21 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.telemetry.metrics.exporter.OTelMetricsExporterFactory; import org.opensearch.telemetry.tracing.exporter.OTelSpanExporterFactory; +import org.opensearch.telemetry.tracing.sampler.OTelSamplerFactory; +import org.opensearch.telemetry.tracing.sampler.ProbabilisticSampler; +import org.opensearch.telemetry.tracing.sampler.ProbabilisticTransportActionSampler; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.util.Arrays; +import java.util.List; import io.opentelemetry.exporter.logging.LoggingMetricExporter; import io.opentelemetry.exporter.logging.LoggingSpanExporter; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.trace.export.SpanExporter; +import io.opentelemetry.sdk.trace.samplers.Sampler; /** * OTel specific telemetry settings. @@ -110,4 +116,40 @@ private OTelTelemetrySettings() {} Setting.Property.NodeScope, Setting.Property.Final ); + + /** + * Samplers orders setting. + */ + @SuppressWarnings("unchecked") + public static final Setting>> OTEL_TRACER_SPAN_SAMPLER_CLASS_SETTINGS = Setting.listSetting( + "telemetry.otel.tracer.span.sampler.classes", + Arrays.asList(ProbabilisticTransportActionSampler.class.getName(), ProbabilisticSampler.class.getName()), + sampler -> { + // Check we ourselves are not being called by unprivileged code. + SpecialPermission.check(); + try { + return AccessController.doPrivileged((PrivilegedExceptionAction>) () -> { + final ClassLoader loader = OTelSamplerFactory.class.getClassLoader(); + return (Class) loader.loadClass(sampler); + }); + } catch (PrivilegedActionException ex) { + throw new IllegalStateException("Unable to load sampler class: " + sampler, ex.getCause()); + } + }, + Setting.Property.NodeScope, + Setting.Property.Final + ); + + /** + * Probability of action based sampler + */ + 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 + ); + } 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 39b97b7d6441a..475fc09d04bff 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,7 @@ 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.OTelSamplerFactory; import org.opensearch.telemetry.tracing.sampler.RequestSampler; import java.security.AccessController; @@ -60,7 +60,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(OTelSamplerFactory.create(telemetrySettings, settings))) ) ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactory.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactory.java new file mode 100644 index 0000000000000..b9d5c07a40cd8 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactory.java @@ -0,0 +1,96 @@ +/* + * 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.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.SpecialPermission; +import org.opensearch.common.settings.Settings; +import org.opensearch.telemetry.OTelTelemetrySettings; +import org.opensearch.telemetry.TelemetrySettings; + +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.security.AccessController; +import java.security.PrivilegedExceptionAction; +import java.util.List; +import java.util.ListIterator; + +import io.opentelemetry.sdk.trace.samplers.Sampler; + +/** + * Factory class to create the instance of OTelSampler + */ +public class OTelSamplerFactory { + + /** + * Logger instance for logging messages related to the OTelSamplerFactory. + */ + private static final Logger logger = LogManager.getLogger(OTelSamplerFactory.class); + + /** + * Base constructor. + */ + private OTelSamplerFactory() { + + } + + /** + * Creates the {@link Sampler} instances based on the TRACER_SPAN_SAMPLER_CLASSES value. + * + * @param telemetrySettings TelemetrySettings. + * @param settings the settings + * @return list of samplers. + */ + public static Sampler create(TelemetrySettings telemetrySettings, Settings settings) { + List> samplersNameList = OTelTelemetrySettings.OTEL_TRACER_SPAN_SAMPLER_CLASS_SETTINGS.get(settings); + ListIterator> li = samplersNameList.listIterator(samplersNameList.size()); + + Sampler fallbackSampler = null; + + // Iterating samplers list in reverse order to create chain of sampler + while (li.hasPrevious()) { + Class samplerName = li.previous(); + fallbackSampler = instantiateSampler(samplerName, telemetrySettings, settings, fallbackSampler); + } + + return fallbackSampler; + } + + private static Sampler instantiateSampler( + Class samplerClassName, + TelemetrySettings telemetrySettings, + Settings settings, + Sampler fallbackSampler + ) { + try { + // Check we ourselves are not being called by unprivileged code. + SpecialPermission.check(); + + return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + try { + // Define the method type which receives TelemetrySettings & Sampler as arguments + MethodType methodType = MethodType.methodType(Sampler.class, TelemetrySettings.class, Settings.class, Sampler.class); + + return (Sampler) MethodHandles.publicLookup() + .findStatic(samplerClassName, "create", methodType) + .invokeExact(telemetrySettings, settings, fallbackSampler); + } catch (Throwable e) { + if (e.getCause() instanceof NoSuchMethodException) { + throw new IllegalStateException("No create method exist in [" + samplerClassName + "]", e.getCause()); + } else { + throw new IllegalStateException("Sampler instantiation failed for class [" + samplerClassName + "]", e.getCause()); + } + } + }); + } catch (Exception e) { + throw new IllegalStateException("Sampler instantiation failed for class [" + samplerClassName + "]", e.getCause()); + } + } +} 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 774070aa39df6..d7fe92b1f3495 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 @@ -8,6 +8,7 @@ package org.opensearch.telemetry.tracing.sampler; +import org.opensearch.common.settings.Settings; import org.opensearch.telemetry.TelemetrySettings; import java.util.List; @@ -18,14 +19,18 @@ 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.SamplingDecision; import io.opentelemetry.sdk.trace.samplers.SamplingResult; /** - * 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 final Settings settings; + private final Sampler fallbackSampler; + private double samplingRatio; /** @@ -33,21 +38,24 @@ public class ProbabilisticSampler implements Sampler { * * @param telemetrySettings Telemetry settings. */ - public ProbabilisticSampler(TelemetrySettings telemetrySettings) { + private ProbabilisticSampler(TelemetrySettings telemetrySettings, Settings settings, Sampler fallbackSampler) { this.telemetrySettings = Objects.requireNonNull(telemetrySettings); + this.settings = Objects.requireNonNull(settings); this.samplingRatio = telemetrySettings.getSamplingProbability(); this.defaultSampler = Sampler.traceIdRatioBased(samplingRatio); + this.fallbackSampler = fallbackSampler; } - Sampler getSampler() { - double newSamplingRatio = telemetrySettings.getSamplingProbability(); - if (isSamplingRatioChanged(newSamplingRatio)) { - synchronized (this) { - this.samplingRatio = newSamplingRatio; - defaultSampler = Sampler.traceIdRatioBased(samplingRatio); - } - } - return defaultSampler; + /** + * Create probabilistic sampler. + * + * @param telemetrySettings the telemetry settings + * @param settings the settings + * @param fallbackSampler the fallback sampler + * @return the probabilistic sampler + */ + public static Sampler create(TelemetrySettings telemetrySettings, Settings settings, Sampler fallbackSampler) { + return new ProbabilisticSampler(telemetrySettings, settings, fallbackSampler); } private boolean isSamplingRatioChanged(double newSamplingRatio) { @@ -67,7 +75,19 @@ public SamplingResult shouldSample( Attributes attributes, List parentLinks ) { - return getSampler().shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + double newSamplingRatio = telemetrySettings.getSamplingProbability(); + if (isSamplingRatioChanged(newSamplingRatio)) { + synchronized (this) { + this.samplingRatio = newSamplingRatio; + defaultSampler = Sampler.traceIdRatioBased(samplingRatio); + } + } + final SamplingResult result = defaultSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + if (result.getDecision() != SamplingDecision.DROP && fallbackSampler != null) { + return fallbackSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + } else { + return result; + } } @Override diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSampler.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSampler.java new file mode 100644 index 0000000000000..93a8edaaaa760 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSampler.java @@ -0,0 +1,99 @@ +/* + * 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.Settings; +import org.opensearch.telemetry.OTelTelemetrySettings; +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.SamplingDecision; +import io.opentelemetry.sdk.trace.samplers.SamplingResult; + +import static org.opensearch.telemetry.tracing.AttributeNames.TRANSPORT_ACTION; + +/** + * ProbabilisticTransportActionSampler sampler samples request with action based on defined probability + */ +public class ProbabilisticTransportActionSampler implements Sampler { + + private final Sampler fallbackSampler; + private Sampler actionSampler; + private final TelemetrySettings telemetrySettings; + private final Settings settings; + private double actionSamplingRatio; + + /** + * Creates ProbabilisticTransportActionSampler sampler + * @param telemetrySettings TelemetrySettings + */ + private ProbabilisticTransportActionSampler(TelemetrySettings telemetrySettings, Settings settings, Sampler fallbackSampler) { + this.telemetrySettings = Objects.requireNonNull(telemetrySettings); + this.settings = Objects.requireNonNull(settings); + this.actionSamplingRatio = OTelTelemetrySettings.TRACER_SAMPLER_ACTION_PROBABILITY.get(settings); + this.actionSampler = Sampler.traceIdRatioBased(actionSamplingRatio); + this.fallbackSampler = fallbackSampler; + } + + /** + * Create probabilistic transport action sampler. + * + * @param telemetrySettings the telemetry settings + * @param settings the settings + * @param fallbackSampler the fallback sampler + * @return the probabilistic transport action sampler + */ + public static Sampler create(TelemetrySettings telemetrySettings, Settings settings, Sampler fallbackSampler) { + return new ProbabilisticTransportActionSampler(telemetrySettings, settings, fallbackSampler); + } + + @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) { + final SamplingResult result = actionSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + if (result.getDecision() != SamplingDecision.DROP && fallbackSampler != null) { + return fallbackSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + } + return result; + } + if (fallbackSampler != null) return fallbackSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + + return SamplingResult.drop(); + } + + 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/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..87c2849173aff 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 @@ -18,21 +18,20 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.sdk.trace.samplers.SamplingResult; +import static org.opensearch.telemetry.tracing.AttributeNames.TRACE; + /** - * HeadBased sampler + * RequestSampler based on 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 fallbackSampler; /** - * Creates Head based sampler - * @param defaultSampler defaultSampler + * Creates request sampler which applies based on all applicable sampler + * @param fallbackSampler Sampler */ - public RequestSampler(Sampler defaultSampler) { - this.defaultSampler = defaultSampler; + public RequestSampler(Sampler fallbackSampler) { + this.fallbackSampler = fallbackSampler; } @Override @@ -44,15 +43,15 @@ public SamplingResult shouldSample( Attributes attributes, List parentLinks ) { - final String trace = attributes.get(AttributeKey.stringKey(TRACE)); if (trace != null) { return (Boolean.parseBoolean(trace) == true) ? SamplingResult.recordAndSample() : SamplingResult.drop(); - } else { - return defaultSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); } - + if (fallbackSampler != null) { + return fallbackSampler.shouldSample(parentContext, traceId, name, spanKind, attributes, parentLinks); + } + return SamplingResult.recordAndSample(); } @Override 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 2fcf89947e537..4a1301588dad2 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 @@ -30,9 +30,11 @@ import static org.opensearch.telemetry.OTelTelemetryPlugin.OTEL_TRACER_NAME; import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING; +import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_TRACER_SPAN_SAMPLER_CLASS_SETTINGS; 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.OTelTelemetrySettings.TRACER_SAMPLER_ACTION_PROBABILITY; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; @@ -69,7 +71,9 @@ public void testGetTelemetry() { TRACER_EXPORTER_DELAY_SETTING, TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, - OTEL_METRICS_EXPORTER_CLASS_SETTING + OTEL_TRACER_SPAN_SAMPLER_CLASS_SETTINGS, + OTEL_METRICS_EXPORTER_CLASS_SETTING, + TRACER_SAMPLER_ACTION_PROBABILITY ), oTelTelemetryPlugin.getSettings() ); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactoryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactoryTests.java new file mode 100644 index 0000000000000..39ccf299dfdc4 --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/OTelSamplerFactoryTests.java @@ -0,0 +1,42 @@ +/* + * 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.OTelTelemetrySettings; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Set; + +import io.opentelemetry.sdk.trace.samplers.Sampler; + +import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; + +public class OTelSamplerFactoryTests extends OpenSearchTestCase { + + public void testDefaultCreate() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + Sampler sampler = OTelSamplerFactory.create(telemetrySettings, Settings.EMPTY); + assertEquals(sampler.getClass(), ProbabilisticTransportActionSampler.class); + } + + public void testCreateWithSingleSampler() { + Settings settings = Settings.builder() + .put(OTelTelemetrySettings.OTEL_TRACER_SPAN_SAMPLER_CLASS_SETTINGS.getKey(), ProbabilisticSampler.class.getName()) + .build(); + + ClusterSettings clusterSettings = new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, clusterSettings); + Sampler sampler = OTelSamplerFactory.create(telemetrySettings, settings); + assertTrue(sampler instanceof ProbabilisticSampler); + } +} 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 639dc341ef0db..a094cd0119f5e 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 @@ -15,18 +15,21 @@ import java.util.Set; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.context.Context; import io.opentelemetry.sdk.trace.samplers.Sampler; 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_PROBABILITY; +import static org.mockito.Mockito.mock; public class ProbabilisticSamplerTests extends OpenSearchTestCase { // When ProbabilisticSampler is created with OTelTelemetrySettings as null public void testProbabilisticSamplerWithNullSettings() { // Verify that the constructor throws IllegalArgumentException when given null settings - assertThrows(NullPointerException.class, () -> { new ProbabilisticSampler(null); }); + assertThrows(NullPointerException.class, () -> { ProbabilisticSampler.create(null, null, null); }); } public void testDefaultGetSampler() { @@ -37,10 +40,9 @@ public void testDefaultGetSampler() { ); // Probabilistic Sampler - ProbabilisticSampler probabilisticSampler = new ProbabilisticSampler(telemetrySettings); + Sampler probabilisticSampler = ProbabilisticSampler.create(telemetrySettings, Settings.EMPTY, null); - assertNotNull(probabilisticSampler.getSampler()); - assertEquals(0.01, probabilisticSampler.getSamplingRatio(), 0.0d); + assertEquals(0.01, ((ProbabilisticSampler) probabilisticSampler).getSamplingRatio(), 0.0d); } public void testGetSamplerWithUpdatedSamplingRatio() { @@ -51,14 +53,16 @@ public void testGetSamplerWithUpdatedSamplingRatio() { ); // Probabilistic Sampler - ProbabilisticSampler probabilisticSampler = new ProbabilisticSampler(telemetrySettings); - assertEquals(0.01d, probabilisticSampler.getSamplingRatio(), 0.0d); + Sampler probabilisticSampler = ProbabilisticSampler.create(telemetrySettings, Settings.EMPTY, null); + + assertEquals(0.01d, ((ProbabilisticSampler) probabilisticSampler).getSamplingRatio(), 0.0d); telemetrySettings.setSamplingProbability(0.02); + // Need to call shouldSample() to update the value of samplingRatio + probabilisticSampler.shouldSample(mock(Context.class), "00000000000000000000000000000000", "", SpanKind.INTERNAL, null, null); + // Need to call getSampler() to update the value of tracerHeadSamplerSamplingRatio - Sampler updatedProbabilisticSampler = probabilisticSampler.getSampler(); - assertEquals(0.02, probabilisticSampler.getSamplingRatio(), 0.0d); + assertEquals(0.02, ((ProbabilisticSampler) probabilisticSampler).getSamplingRatio(), 0.0d); } - } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSamplerTests.java new file mode 100644 index 0000000000000..261b0252fef60 --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticTransportActionSamplerTests.java @@ -0,0 +1,52 @@ +/* + * 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.Sampler; +import io.opentelemetry.sdk.trace.samplers.SamplingResult; + +import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; +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 ProbabilisticTransportActionSamplerTests extends OpenSearchTestCase { + + public void testGetSamplerWithActionSamplingRatio() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + + // ProbabilisticTransportActionSampler + Sampler probabilisticTransportActionSampler = ProbabilisticTransportActionSampler.create(telemetrySettings, Settings.EMPTY, null); + + SamplingResult result = probabilisticTransportActionSampler.shouldSample( + mock(Context.class), + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + Attributes.builder().put(TRANSPORT_ACTION, "dummy_action").build(), + Collections.emptyList() + ); + // Verify that ProbabilisticTransportActionSampler returned SamplingResult.recordAndSample() as all actions will be sampled + assertEquals(SamplingResult.recordAndSample(), result); + assertEquals(0.001, ((ProbabilisticTransportActionSampler) probabilisticTransportActionSampler).getSamplingRatio(), 0.000d); + } +} 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..da234ca13dc9d 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,9 +8,14 @@ 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 org.junit.Before; import java.util.Collections; +import java.util.Set; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; @@ -19,29 +24,29 @@ 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.TelemetrySettings.TRACER_ENABLED_SETTING; +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 { + private ClusterSettings clusterSettings; + private TelemetrySettings telemetrySettings; + private RequestSampler requestSampler; + private Context parentContext; + + @Before + public void init() { + clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + Sampler fallbackSampler = OTelSamplerFactory.create(telemetrySettings, Settings.EMPTY); + requestSampler = new RequestSampler(fallbackSampler); + parentContext = mock(Context.class); + } public void testShouldSampleWithTraceAttributeAsTrue() { - - // 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 - Context parentContext = mock(Context.class); Attributes attributes = Attributes.of(AttributeKey.stringKey("trace"), "true"); - // Call shouldSample on HeadSampler SamplingResult result = requestSampler.shouldSample( parentContext, "traceId", @@ -50,43 +55,85 @@ public void testShouldSampleWithTraceAttributeAsTrue() { attributes, Collections.emptyList() ); - assertEquals(SamplingResult.recordAndSample(), result); + } + + public void testShouldSampleWithTraceAttributeAsFalse() { + Attributes attributes = Attributes.of(AttributeKey.stringKey("trace"), "false"); - // Verify that the default sampler's shouldSample method was not called - verify(defaultSampler, never()).shouldSample(any(), anyString(), anyString(), any(), any(), any()); + SamplingResult result = requestSampler.shouldSample( + parentContext, + "traceId", + "spanName", + SpanKind.INTERNAL, + attributes, + Collections.emptyList() + ); + assertEquals(SamplingResult.drop(), result); } - public void testShouldSampleWithoutTraceAttribute() { + public void testShouldSampleForProbabilisticSampler() { + clusterSettings.applySettings( + Settings.builder() + .put("telemetry.tracer.sampler.probability", "1.0") + .put("telemetry.otel.tracer.span.sampler.classes", "org.opensearch.telemetry.tracing.sampler.ProbabilisticSampler") + .build() + ); + + Attributes attributes = Attributes.builder().build(); + + SamplingResult result = requestSampler.shouldSample( + parentContext, + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + attributes, + Collections.emptyList() + ); - // Create a mock default sampler - Sampler defaultSampler = mock(Sampler.class); - when(defaultSampler.shouldSample(any(), anyString(), anyString(), any(), any(), any())).thenReturn( - SamplingResult.recordAndSample() + // Verify that request is sampled + assertEquals(SamplingResult.recordAndSample(), result); + + clusterSettings.applySettings(Settings.builder().put("telemetry.tracer.sampler.probability", "0.0").build()); + result = requestSampler.shouldSample( + parentContext, + "00000000000000000000000000000000", + "spanName", + SpanKind.INTERNAL, + attributes, + Collections.emptyList() ); + assertEquals(SamplingResult.drop(), result); - // Create an instance of HeadSampler with the mock default sampler - RequestSampler requestSampler = new RequestSampler(defaultSampler); + } - // Create a mock Context and Attributes + public void testShouldSampleForProbabilisticTransportActionSampler() { + clusterSettings.applySettings( + Settings.builder() + .put( + "telemetry.otel.tracer.span.sampler.classes", + "org.opensearch.telemetry.tracing.sampler.ProbabilisticTransportActionSampler" + ) + .build() + ); + 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.empty(); + Attributes attributes = Attributes.builder().put(TRANSPORT_ACTION, "dummy_action").build(); - // Call shouldSample on HeadSampler + // Calling shouldSample to update samplingRatio SamplingResult result = requestSampler.shouldSample( parentContext, - "traceId", + "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()); } } diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index 24dcab98c8870..4b8897a318531 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -66,7 +66,6 @@ public class TelemetrySettings { private volatile boolean tracingEnabled; private volatile double samplingProbability; - private final boolean tracingFeatureEnabled; private final boolean metricsFeatureEnabled; @@ -98,6 +97,7 @@ public void setSamplingProbability(double samplingProbability) { /** * Get sampling ratio + * @return double */ public double getSamplingProbability() { return samplingProbability; @@ -110,4 +110,5 @@ public boolean isTracingFeatureEnabled() { public boolean isMetricsFeatureEnabled() { return metricsFeatureEnabled; } + } diff --git a/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java b/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java new file mode 100644 index 0000000000000..4c96f79b30d55 --- /dev/null +++ b/server/src/test/java/org/opensearch/telemetry/TelemetrySettingsTests.java @@ -0,0 +1,64 @@ +/* + * 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; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Set; + +import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; + +public class TelemetrySettingsTests extends OpenSearchTestCase { + + public void testSetTracingEnabledOrDisabled() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + + // Validation for tracingEnabled as true + telemetrySettings.setTracingEnabled(true); + assertTrue(telemetrySettings.isTracingEnabled()); + + // Validation for tracingEnabled as false + telemetrySettings.setTracingEnabled(false); + assertFalse(telemetrySettings.isTracingEnabled()); + } + + public void testSetSamplingProbability() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + + // Validating default sample rate i.e 1% + assertEquals(0.01, telemetrySettings.getSamplingProbability(), 0.00d); + + // Validating override for sampling for 100% request + telemetrySettings.setSamplingProbability(1.00); + assertEquals(1.00, telemetrySettings.getSamplingProbability(), 0.00d); + + // Validating override for sampling for 50% request + telemetrySettings.setSamplingProbability(0.50); + assertEquals(0.50, telemetrySettings.getSamplingProbability(), 0.00d); + } + + public void testGetSamplingProbability() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)); + TelemetrySettings telemetrySettings = new TelemetrySettings(Settings.EMPTY, clusterSettings); + + // 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()); + + // Validating if default sampling is updated to 2% + assertEquals(0.02, telemetrySettings.getSamplingProbability(), 0.00d); + } + +}