Skip to content

Commit

Permalink
Align exemplar filter names with spec (#5063)
Browse files Browse the repository at this point in the history
* Align exemplar filter names with spec

* coverage

* spotless
  • Loading branch information
jack-berg authored Dec 29, 2022
1 parent 2834b03 commit 17e8106
Show file tree
Hide file tree
Showing 14 changed files with 146 additions and 51 deletions.
6 changes: 3 additions & 3 deletions sdk-extensions/autoconfigure/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,9 @@ These properties can be used to control the maximum size of spans by placing lim

## Exemplars

| System property | Environment variable | Description |
|------------------------------|------------------------------|-------------------------------------------------------------------------------------------------------------------|
| otel.metrics.exemplar.filter | OTEL_METRICS_EXEMPLAR_FILTER | The filter for exemplar sampling. Can be `NONE`, `ALL` or `WITH_SAMPLED_TRACE`. Default is `WITH_SAMPLED_TRACE`. |
| System property | Environment variable | Description |
|------------------------------|------------------------------|-----------------------------------------------------------------------------------------------------------------|
| otel.metrics.exemplar.filter | OTEL_METRICS_EXEMPLAR_FILTER | The filter for exemplar sampling. Can be `ALWAYS_OFF`, `ALWAYS_ON` or `TRACE_BASED`. Default is `TRACE_BASED`. |

## Periodic Metric Reader

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@
import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

final class MeterProviderConfiguration {
private static final Logger LOGGER = Logger.getLogger(MeterProviderConfiguration.class.getName());

@SuppressWarnings("fallthrough")
static void configureMeterProvider(
SdkMeterProviderBuilder meterProviderBuilder,
ConfigProperties config,
Expand All @@ -30,18 +35,30 @@ static void configureMeterProvider(
metricExporterCustomizer) {

// Configure default exemplar filters.
String exemplarFilter = config.getString("otel.metrics.exemplar.filter", "with_sampled_trace");
String exemplarFilter =
config.getString("otel.metrics.exemplar.filter", "trace_based").toLowerCase(Locale.ROOT);
switch (exemplarFilter) {
case "none":
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.neverSample());
case "none": // DEPRECATED: replaced by always_off
LOGGER.log(
Level.WARNING,
"otel.metrics.exemplar.filter option \"none\" is deprecated for removal. Use \"always_off\" instead.");
case "always_off":
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.alwaysOff());
break;
case "all":
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.alwaysSample());
case "all": // DEPRECATED: replaced by always_on
LOGGER.log(
Level.WARNING,
"otel.metrics.exemplar.filter option \"all\" is deprecated for removal. Use \"always_on\" instead.");
case "always_on":
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.alwaysOn());
break;
case "with_sampled_trace":
case "with_sampled_trace": // DEPRECATED: replaced by trace_based
LOGGER.log(
Level.WARNING,
"otel.metrics.exemplar.filter option \"with_sampled_trace\" is deprecated for removal. Use \"trace_based\" instead.");
case "trace_based":
default:
SdkMeterProviderUtil.setExemplarFilter(
meterProviderBuilder, ExemplarFilter.sampleWithTraces());
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.traceBased());
break;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.autoconfigure;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.BDDAssertions.as;

import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder;
import io.opentelemetry.sdk.metrics.internal.exemplar.AlwaysOffFilter;
import io.opentelemetry.sdk.metrics.internal.exemplar.AlwaysOnFilter;
import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter;
import io.opentelemetry.sdk.metrics.internal.exemplar.TraceBasedExemplarFilter;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.assertj.core.api.ObjectAssert;
import org.junit.jupiter.api.Test;

class MeterProviderConfigurationTest {

@Test
void configureMeterProvider_ConfiguresExemplarFilter() {
assertExemplarFilter(Collections.emptyMap()).isInstanceOf(TraceBasedExemplarFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "foo"))
.isInstanceOf(TraceBasedExemplarFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "trace_based"))
.isInstanceOf(TraceBasedExemplarFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "Trace_based"))
.isInstanceOf(TraceBasedExemplarFilter.class);
assertExemplarFilter(
Collections.singletonMap("otel.metrics.exemplar.filter", "with_sampled_trace"))
.isInstanceOf(TraceBasedExemplarFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "always_off"))
.isInstanceOf(AlwaysOffFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "always_Off"))
.isInstanceOf(AlwaysOffFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "none"))
.isInstanceOf(AlwaysOffFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "always_on"))
.isInstanceOf(AlwaysOnFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "ALWAYS_ON"))
.isInstanceOf(AlwaysOnFilter.class);
assertExemplarFilter(Collections.singletonMap("otel.metrics.exemplar.filter", "all"))
.isInstanceOf(AlwaysOnFilter.class);
}

private static ObjectAssert<ExemplarFilter> assertExemplarFilter(Map<String, String> config) {
Map<String, String> configWithDefault = new HashMap<>(config);
configWithDefault.put("otel.metrics.exporter", "none");
SdkMeterProviderBuilder builder = SdkMeterProvider.builder();
MeterProviderConfiguration.configureMeterProvider(
builder,
DefaultConfigProperties.createForTest(configWithDefault),
MeterProviderConfigurationTest.class.getClassLoader(),
(a, b) -> a);
return assertThat(builder)
.extracting("exemplarFilter", as(InstanceOfAssertFactories.type(ExemplarFilter.class)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Meter build() {
.setResource(Resource.empty())
// Must register reader for real SDK.
.registerMetricReader(InMemoryMetricReader.create())
.setExemplarFilter(ExemplarFilter.neverSample())
.setExemplarFilter(ExemplarFilter.alwaysOff())
.build()
.get("io.opentelemetry.sdk.metrics");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void setup() {
.setInterval(Duration.ofSeconds(Integer.MAX_VALUE))
.build());
// Disable examplars
SdkMeterProviderUtil.setExemplarFilter(builder, ExemplarFilter.neverSample());
SdkMeterProviderUtil.setExemplarFilter(builder, ExemplarFilter.alwaysOff());
sdkMeterProvider = builder.build();
histogram = sdkMeterProvider.get("meter").histogramBuilder("histogram").build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public final class SdkMeterProviderBuilder {
*
* @see #setExemplarFilter(ExemplarFilter)
*/
private static final ExemplarFilter DEFAULT_EXEMPLAR_FILTER = ExemplarFilter.sampleWithTraces();
private static final ExemplarFilter DEFAULT_EXEMPLAR_FILTER = ExemplarFilter.traceBased();

private Clock clock = Clock.getDefault();
private Resource resource = Resource.getDefault();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.context.Context;

class NeverSampleFilter implements ExemplarFilter {
static final ExemplarFilter INSTANCE = new NeverSampleFilter();
/**
* A filter which makes no measurements eligible for being an exemplar.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class AlwaysOffFilter implements ExemplarFilter {
static final ExemplarFilter INSTANCE = new AlwaysOffFilter();

private NeverSampleFilter() {}
private AlwaysOffFilter() {}

@Override
public boolean shouldSampleMeasurement(long value, Attributes attributes, Context context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.context.Context;

class AlwaysSampleFilter implements ExemplarFilter {
static final ExemplarFilter INSTANCE = new AlwaysSampleFilter();
/**
* A filter which makes all measurements eligible for being an exemplar.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class AlwaysOnFilter implements ExemplarFilter {
static final ExemplarFilter INSTANCE = new AlwaysOnFilter();

private AlwaysSampleFilter() {}
private AlwaysOnFilter() {}

@Override
public boolean shouldSampleMeasurement(long value, Attributes attributes, Context context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ public interface ExemplarFilter {
* A filter that only accepts measurements where there is a {@code Span} in {@link Context} that
* is being sampled.
*/
static ExemplarFilter sampleWithTraces() {
return WithTraceExemplarFilter.INSTANCE;
static ExemplarFilter traceBased() {
return TraceBasedExemplarFilter.INSTANCE;
}

/** A filter that accepts any measurement. */
static ExemplarFilter alwaysSample() {
return AlwaysSampleFilter.INSTANCE;
/** A filter which makes all measurements eligible for being an exemplar. */
static ExemplarFilter alwaysOn() {
return AlwaysOnFilter.INSTANCE;
}

/** A filter that accepts no measurements. */
static ExemplarFilter neverSample() {
return NeverSampleFilter.INSTANCE;
/** A filter which makes no measurements eligible for being an exemplar. */
static ExemplarFilter alwaysOff() {
return AlwaysOffFilter.INSTANCE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;

/** Exemplar sampler that only samples measurements with associated sampled traces. */
final class WithTraceExemplarFilter implements ExemplarFilter {
/**
* Exemplar sampler that only samples measurements with associated sampled traces.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class TraceBasedExemplarFilter implements ExemplarFilter {

static final ExemplarFilter INSTANCE = new WithTraceExemplarFilter();
static final ExemplarFilter INSTANCE = new TraceBasedExemplarFilter();

private WithTraceExemplarFilter() {}
private TraceBasedExemplarFilter() {}

@Override
public boolean shouldSampleMeasurement(long value, Attributes attributes, Context context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static <T, U extends ExemplarData> AsynchronousMetricStorage<T, U> create(
MetricDescriptor.create(view, registeredView.getViewSourceInfo(), instrumentDescriptor);
Aggregator<T, U> aggregator =
((AggregatorFactory) view.getAggregation())
.createAggregator(instrumentDescriptor, ExemplarFilter.neverSample());
.createAggregator(instrumentDescriptor, ExemplarFilter.alwaysOff());
return new AsynchronousMetricStorage<>(
registeredReader,
metricDescriptor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,63 +25,59 @@ class ExemplarFilterTest {
private static final String SPAN_ID = "ff00000000000041";

@Test
void never_NeverReturnsTrue() {
void alwaysOff_NeverReturnsTrue() {
assertThat(
ExemplarFilter.neverSample()
ExemplarFilter.alwaysOff()
.shouldSampleMeasurement(1, Attributes.empty(), Context.root()))
.isFalse();
}

@Test
void always_AlwaysReturnsTrue() {
void alwaysOn_AlwaysReturnsTrue() {
assertThat(
ExemplarFilter.alwaysSample()
ExemplarFilter.alwaysOn()
.shouldSampleMeasurement(1, Attributes.empty(), Context.root()))
.isTrue();
}

@Test
void withSampledTrace_ReturnsFalseOnNoContext() {
assertThat(
ExemplarFilter.sampleWithTraces()
ExemplarFilter.traceBased()
.shouldSampleMeasurement(1, Attributes.empty(), Context.root()))
.isFalse();
}

@Test
void withSampledTrace_sampleWithTrace() {
void traceBased_sampleWithTrace() {
Context context =
Context.root()
.with(
Span.wrap(
SpanContext.createFromRemoteParent(
TRACE_ID, SPAN_ID, TraceFlags.getSampled(), TraceState.getDefault())));
assertThat(
ExemplarFilter.sampleWithTraces()
.shouldSampleMeasurement(1, Attributes.empty(), context))
assertThat(ExemplarFilter.traceBased().shouldSampleMeasurement(1, Attributes.empty(), context))
.isTrue();
}

@Test
void withSampledTrace_notSampleUnsampledTrace() {
void traceBased_notSampleUnsampledTrace() {
Context context =
Context.root()
.with(
Span.wrap(
SpanContext.createFromRemoteParent(
TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault())));
assertThat(
ExemplarFilter.sampleWithTraces()
.shouldSampleMeasurement(1, Attributes.empty(), context))
assertThat(ExemplarFilter.traceBased().shouldSampleMeasurement(1, Attributes.empty(), context))
.isFalse();
}

@Test
void setExemplarFilter() {
SdkMeterProviderBuilder builder = SdkMeterProvider.builder();
SdkMeterProviderUtil.setExemplarFilter(builder, ExemplarFilter.alwaysSample());
SdkMeterProviderUtil.setExemplarFilter(builder, ExemplarFilter.alwaysOn());
assertThat(builder)
.extracting("exemplarFilter", as(InstanceOfAssertFactories.type(ExemplarFilter.class)))
.isEqualTo(ExemplarFilter.alwaysSample());
.isEqualTo(ExemplarFilter.alwaysOn());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class SynchronousMetricStorageTest {
private final TestClock testClock = TestClock.create();
private final Aggregator<Long, LongExemplarData> aggregator =
((AggregatorFactory) Aggregation.lastValue())
.createAggregator(DESCRIPTOR, ExemplarFilter.neverSample());
.createAggregator(DESCRIPTOR, ExemplarFilter.alwaysOff());
private final AttributesProcessor attributesProcessor = AttributesProcessor.noop();

@Mock private MetricReader reader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ class TemporalMetricStorageTest {
MetricDescriptor.create("name", "description", "unit");
private static final Aggregator<DoubleAccumulation, DoubleExemplarData> SUM =
((AggregatorFactory) Aggregation.sum())
.createAggregator(DESCRIPTOR, ExemplarFilter.neverSample());
.createAggregator(DESCRIPTOR, ExemplarFilter.alwaysOff());

private static final Aggregator<DoubleAccumulation, DoubleExemplarData> ASYNC_SUM =
((AggregatorFactory) Aggregation.sum())
.createAggregator(ASYNC_DESCRIPTOR, ExemplarFilter.neverSample());
.createAggregator(ASYNC_DESCRIPTOR, ExemplarFilter.alwaysOff());

@Mock private MetricReader reader;
private RegisteredReader registeredReader;
Expand Down

0 comments on commit 17e8106

Please sign in to comment.