diff --git a/extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/metrics/CounterAdviceConfigurer.java b/extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/metrics/CounterAdviceConfigurer.java new file mode 100644 index 00000000000..0d183d35654 --- /dev/null +++ b/extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/metrics/CounterAdviceConfigurer.java @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.extension.incubator.metrics; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.metrics.DoubleCounter; +import java.util.List; + +/** Configure advice for implementation of {@link DoubleCounter}. */ +public interface CounterAdviceConfigurer { + + /** Specify the recommended set of attribute keys to be used for this counter. */ + CounterAdviceConfigurer setAttributes(List> attributes); +} diff --git a/extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/metrics/ExtendedDoubleCounterBuilder.java b/extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/metrics/ExtendedDoubleCounterBuilder.java new file mode 100644 index 00000000000..a5d0873eb01 --- /dev/null +++ b/extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/metrics/ExtendedDoubleCounterBuilder.java @@ -0,0 +1,18 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.extension.incubator.metrics; + +import io.opentelemetry.api.metrics.DoubleCounterBuilder; +import java.util.function.Consumer; + +/** Extended {@link DoubleCounterBuilder} with experimental APIs. */ +public interface ExtendedDoubleCounterBuilder extends DoubleCounterBuilder { + + /** Specify advice for counter implementations. */ + default DoubleCounterBuilder setAdvice(Consumer adviceConsumer) { + return this; + } +} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java index db4c8e46d25..db6a552bad2 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilder.java @@ -28,10 +28,10 @@ abstract class AbstractInstrumentBuilder T swapBuilder(SwapBuilder swapper) { return swapper.newBuilder( - meterProviderSharedState, meterSharedState, instrumentName, description, unit, advice); - } - - protected void setAdvice(Advice advice) { - this.advice = advice; + meterProviderSharedState, + meterSharedState, + instrumentName, + description, + unit, + adviceBuilder); } final I buildSynchronousInstrument( BiFunction instrumentFactory) { InstrumentDescriptor descriptor = - InstrumentDescriptor.create(instrumentName, description, unit, type, valueType, advice); + InstrumentDescriptor.create( + instrumentName, description, unit, type, valueType, adviceBuilder.build()); WriteableMetricStorage storage = meterSharedState.registerSynchronousMetricStorage(descriptor, meterProviderSharedState); return instrumentFactory.apply(descriptor, storage); @@ -123,7 +125,8 @@ final SdkObservableInstrument registerLongAsynchronousInstrument( final SdkObservableMeasurement buildObservableMeasurement(InstrumentType type) { InstrumentDescriptor descriptor = - InstrumentDescriptor.create(instrumentName, description, unit, type, valueType, advice); + InstrumentDescriptor.create( + instrumentName, description, unit, type, valueType, adviceBuilder.build()); return meterSharedState.registerObservableMeasurement(descriptor); } @@ -131,7 +134,8 @@ final SdkObservableMeasurement buildObservableMeasurement(InstrumentType type) { public String toString() { return this.getClass().getSimpleName() + "{descriptor=" - + InstrumentDescriptor.create(instrumentName, description, unit, type, valueType, advice) + + InstrumentDescriptor.create( + instrumentName, description, unit, type, valueType, adviceBuilder.build()) + "}"; } @@ -143,6 +147,6 @@ T newBuilder( String name, String description, String unit, - Advice advice); + Advice.AdviceBuilder adviceBuilder); } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleCounter.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleCounter.java index 6b0bbaa6876..0aa48dae4d5 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleCounter.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleCounter.java @@ -5,18 +5,22 @@ package io.opentelemetry.sdk.metrics; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleCounterBuilder; import io.opentelemetry.api.metrics.ObservableDoubleCounter; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; import io.opentelemetry.context.Context; +import io.opentelemetry.extension.incubator.metrics.CounterAdviceConfigurer; +import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleCounterBuilder; import io.opentelemetry.sdk.internal.ThrottlingLogger; import io.opentelemetry.sdk.metrics.internal.descriptor.Advice; import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor; import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState; import io.opentelemetry.sdk.metrics.internal.state.MeterSharedState; import io.opentelemetry.sdk.metrics.internal.state.WriteableMetricStorage; +import java.util.List; import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; @@ -56,7 +60,8 @@ public void add(double increment) { } static final class SdkDoubleCounterBuilder - extends AbstractInstrumentBuilder implements DoubleCounterBuilder { + extends AbstractInstrumentBuilder + implements ExtendedDoubleCounterBuilder, CounterAdviceConfigurer { SdkDoubleCounterBuilder( MeterProviderSharedState meterProviderSharedState, @@ -64,7 +69,7 @@ static final class SdkDoubleCounterBuilder String name, String description, String unit, - Advice advice) { + Advice.AdviceBuilder adviceBuilder) { super( meterProviderSharedState, sharedState, @@ -73,7 +78,7 @@ static final class SdkDoubleCounterBuilder name, description, unit, - advice); + adviceBuilder); } @Override @@ -81,6 +86,12 @@ protected SdkDoubleCounterBuilder getThis() { return this; } + @Override + public DoubleCounterBuilder setAdvice(Consumer adviceConsumer) { + adviceConsumer.accept(this); + return this; + } + @Override public SdkDoubleCounter build() { return buildSynchronousInstrument(SdkDoubleCounter::new); @@ -96,5 +107,11 @@ public ObservableDoubleCounter buildWithCallback( public ObservableDoubleMeasurement buildObserver() { return buildObservableMeasurement(InstrumentType.OBSERVABLE_COUNTER); } + + @Override + public CounterAdviceConfigurer setAttributes(List> attributes) { + adviceBuilder.setAttributes(attributes); + return this; + } } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java index 534258153ac..4e6b11ab0d5 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogram.java @@ -12,7 +12,6 @@ import io.opentelemetry.extension.incubator.metrics.DoubleHistogramAdviceConfigurer; import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder; import io.opentelemetry.sdk.internal.ThrottlingLogger; -import io.opentelemetry.sdk.metrics.internal.descriptor.Advice; import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor; import io.opentelemetry.sdk.metrics.internal.state.MeterProviderSharedState; import io.opentelemetry.sdk.metrics.internal.state.MeterSharedState; @@ -99,7 +98,7 @@ public LongHistogramBuilder ofLongs() { @Override public DoubleHistogramAdviceConfigurer setExplicitBucketBoundaries( List bucketBoundaries) { - setAdvice(Advice.create(bucketBoundaries)); + adviceBuilder.setExplicitBucketBoundaries(bucketBoundaries); return this; } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleUpDownCounter.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleUpDownCounter.java index 4c7f2c95196..5471f964905 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleUpDownCounter.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkDoubleUpDownCounter.java @@ -52,7 +52,7 @@ static final class SdkDoubleUpDownCounterBuilder String name, String description, String unit, - Advice advice) { + Advice.AdviceBuilder adviceBuilder) { super( meterProviderSharedState, sharedState, @@ -61,7 +61,7 @@ static final class SdkDoubleUpDownCounterBuilder name, description, unit, - advice); + adviceBuilder); } @Override diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongGaugeBuilder.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongGaugeBuilder.java index ea6bad737c8..8ed3a40a7f9 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongGaugeBuilder.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkLongGaugeBuilder.java @@ -22,7 +22,7 @@ final class SdkLongGaugeBuilder extends AbstractInstrumentBuilder bucketBoundaries) { List doubleBoundaries = bucketBoundaries.stream().map(Long::doubleValue).collect(Collectors.toList()); - setAdvice(Advice.create(doubleBoundaries)); + adviceBuilder.setExplicitBucketBoundaries(doubleBoundaries); return this; } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/descriptor/Advice.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/descriptor/Advice.java index a770efb5743..96302ecfbe4 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/descriptor/Advice.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/descriptor/Advice.java @@ -6,6 +6,7 @@ package io.opentelemetry.sdk.metrics.internal.descriptor; import com.google.auto.value.AutoValue; +import io.opentelemetry.api.common.AttributeKey; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -16,28 +17,64 @@ @Immutable public abstract class Advice { - private static final Advice EMPTY_ADVICE = create(null); + private static final Advice EMPTY_ADVICE = builder().build(); public static Advice empty() { return EMPTY_ADVICE; } - /** - * Creates a new {@link Advice} with the given explicit bucket histogram boundaries. - * - * @param explicitBucketBoundaries the explicit bucket histogram boundaries. - * @return a new {@link Advice} with the given bucket boundaries. - */ - public static Advice create(@Nullable List explicitBucketBoundaries) { - if (explicitBucketBoundaries != null) { - explicitBucketBoundaries = - Collections.unmodifiableList(new ArrayList<>(explicitBucketBoundaries)); - } - return new AutoValue_Advice(explicitBucketBoundaries); + public static AdviceBuilder builder() { + return new AutoValue_Advice.Builder(); } Advice() {} @Nullable public abstract List getExplicitBucketBoundaries(); + + @Nullable + public abstract List> getAttributes(); + + public boolean hasAttributes() { + return getAttributes() != null; + } + + @AutoValue.Builder + public abstract static class AdviceBuilder { + + AdviceBuilder() {} + + abstract AdviceBuilder explicitBucketBoundaries( + @Nullable List explicitBucketBoundaries); + + /** + * Sets the explicit bucket histogram boundaries. + * + * @param explicitBucketBoundaries the explicit bucket histogram boundaries. + */ + public AdviceBuilder setExplicitBucketBoundaries( + @Nullable List explicitBucketBoundaries) { + if (explicitBucketBoundaries != null) { + explicitBucketBoundaries = + Collections.unmodifiableList(new ArrayList<>(explicitBucketBoundaries)); + } + return explicitBucketBoundaries(explicitBucketBoundaries); + } + + abstract AdviceBuilder attributes(@Nullable List> attributes); + + /** + * Sets the list of the attribute keys to be used for the resulting instrument. + * + * @param attributes the list of the attribute keys. + */ + public AdviceBuilder setAttributes(@Nullable List> attributes) { + if (attributes != null) { + attributes = Collections.unmodifiableList(new ArrayList<>(attributes)); + } + return attributes(attributes); + } + + public abstract Advice build(); + } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/AdviceAttributesProcessor.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/AdviceAttributesProcessor.java new file mode 100644 index 00000000000..161dcd72832 --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/AdviceAttributesProcessor.java @@ -0,0 +1,40 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics.internal.view; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.context.Context; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +final class AdviceAttributesProcessor extends AttributesProcessor { + + private final Set> attributeKeys; + + AdviceAttributesProcessor(List> adviceAttributeKeys) { + this.attributeKeys = new HashSet<>(adviceAttributeKeys); + } + + @Override + public Attributes process(Attributes incoming, Context context) { + AttributesBuilder builder = incoming.toBuilder(); + builder.removeIf(key -> !attributeKeys.contains(key)); + return builder.build(); + } + + @Override + public boolean usesContext() { + return false; + } + + @Override + public String toString() { + return "AdviceAttributesProcessor{attributeKeys=" + attributeKeys + '}'; + } +} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistry.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistry.java index 3bba1153488..9e5bba37233 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistry.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistry.java @@ -6,6 +6,7 @@ package io.opentelemetry.sdk.metrics.internal.view; import static io.opentelemetry.sdk.metrics.internal.view.NoopAttributesProcessor.NOOP; +import static java.util.Objects.requireNonNull; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.metrics.Aggregation; @@ -17,6 +18,7 @@ import io.opentelemetry.sdk.metrics.internal.aggregator.AggregationUtil; import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorFactory; import io.opentelemetry.sdk.metrics.internal.debug.SourceInfo; +import io.opentelemetry.sdk.metrics.internal.descriptor.Advice; import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor; import io.opentelemetry.sdk.metrics.internal.export.CardinalityLimitSelector; import io.opentelemetry.sdk.metrics.internal.state.MetricStorage; @@ -25,7 +27,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; @@ -127,27 +128,32 @@ public List findViews( // No views matched, use default view RegisteredView instrumentDefaultView = - Objects.requireNonNull(instrumentDefaultRegisteredView.get(descriptor.getType())); + requireNonNull(instrumentDefaultRegisteredView.get(descriptor.getType())); + AggregatorFactory viewAggregatorFactory = (AggregatorFactory) instrumentDefaultView.getView().getAggregation(); - // If the aggregation from default aggregation selector is compatible with the instrument, use - // it - if (viewAggregatorFactory.isCompatibleWithInstrument(descriptor)) { - return Collections.singletonList(instrumentDefaultView); + if (!viewAggregatorFactory.isCompatibleWithInstrument(descriptor)) { + // The aggregation from default aggregation selector was incompatible with instrument, use + // default aggregation instead + logger.log( + Level.WARNING, + "Instrument default aggregation " + + AggregationUtil.aggregationName(instrumentDefaultView.getView().getAggregation()) + + " is incompatible with instrument " + + descriptor.getName() + + " of type " + + descriptor.getType()); + instrumentDefaultView = DEFAULT_REGISTERED_VIEW; + } + + // if the user defined an attributes advice, use it + if (descriptor.getAdvice().hasAttributes()) { + instrumentDefaultView = + applyAdviceToDefaultView(instrumentDefaultView, descriptor.getAdvice()); } - // The aggregation from default aggregation selector was incompatible with instrument, use - // default aggregation instead - logger.log( - Level.WARNING, - "Instrument default aggregation " - + AggregationUtil.aggregationName(instrumentDefaultView.getView().getAggregation()) - + " is incompatible with instrument " - + descriptor.getName() - + " of type " - + descriptor.getType()); - return Collections.singletonList(DEFAULT_REGISTERED_VIEW); + return Collections.singletonList(instrumentDefaultView); } // Matches an instrument selector against an instrument + meter. @@ -246,4 +252,14 @@ private static Pattern toRegexPattern(String globPattern) { } return Pattern.compile(patternBuilder.toString()); } + + private static RegisteredView applyAdviceToDefaultView( + RegisteredView instrumentDefaultView, Advice advice) { + return RegisteredView.create( + instrumentDefaultView.getInstrumentSelector(), + instrumentDefaultView.getView(), + new AdviceAttributesProcessor(requireNonNull(advice.getAttributes())), + instrumentDefaultView.getCardinalityLimit(), + instrumentDefaultView.getViewSourceInfo()); + } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java index ad008e65fd3..15f6ba3030f 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AbstractInstrumentBuilderTest.java @@ -41,7 +41,7 @@ void stringRepresentation() { + "unit=instrument-unit, " + "type=COUNTER, " + "valueType=LONG, " - + "advice=Advice{explicitBucketBoundaries=null}" + + "advice=Advice{explicitBucketBoundaries=null, attributes=null}" + "}}"); } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AttributesAdviceTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AttributesAdviceTest.java new file mode 100644 index 00000000000..f6b5f67f7f9 --- /dev/null +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AttributesAdviceTest.java @@ -0,0 +1,111 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics; + +import static io.opentelemetry.api.common.AttributeKey.stringKey; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static java.util.Arrays.asList; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.DoubleCounter; +import io.opentelemetry.api.metrics.DoubleCounterBuilder; +import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleCounterBuilder; +import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +class AttributesAdviceTest { + + private static final Attributes ATTRIBUTES = + Attributes.builder() + .put(stringKey("key1"), "1") + .put(stringKey("key2"), "2") + .put(stringKey("key3"), "3") + .build(); + + private SdkMeterProvider meterProvider = SdkMeterProvider.builder().build(); + + @AfterEach + void cleanup() { + meterProvider.close(); + } + + @Test + void counterWithoutAdvice() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + meterProvider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + + DoubleCounter counter = + meterProvider.get("meter").counterBuilder("counter").ofDoubles().build(); + counter.add(1, ATTRIBUTES); + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasDoubleSumSatisfying( + sum -> sum.hasPointsSatisfying(point -> point.hasAttributes(ATTRIBUTES)))); + } + + @Test + void counterWithAdvice() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + meterProvider = SdkMeterProvider.builder().registerMetricReader(reader).build(); + + DoubleCounterBuilder doubleCounterBuilder = + meterProvider.get("meter").counterBuilder("counter").ofDoubles(); + ((ExtendedDoubleCounterBuilder) doubleCounterBuilder) + .setAdvice(advice -> advice.setAttributes(asList(stringKey("key1"), stringKey("key2")))); + DoubleCounter counter = doubleCounterBuilder.build(); + counter.add(1, ATTRIBUTES); + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasDoubleSumSatisfying( + sum -> + sum.hasPointsSatisfying( + point -> + point.hasAttributesSatisfyingExactly( + equalTo(stringKey("key1"), "1"), + equalTo(stringKey("key2"), "2"))))); + } + + @Test + void counterWithAdviceAndViews() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + meterProvider = + SdkMeterProvider.builder() + .registerMetricReader(reader) + .registerView( + InstrumentSelector.builder().setType(InstrumentType.COUNTER).build(), + View.builder() + .setAttributeFilter(key -> "key2".equals(key) || "key3".equals(key)) + .build()) + .build(); + + DoubleCounterBuilder doubleCounterBuilder = + meterProvider.get("meter").counterBuilder("counter").ofDoubles(); + ((ExtendedDoubleCounterBuilder) doubleCounterBuilder) + .setAdvice(advice -> advice.setAttributes(asList(stringKey("key1"), stringKey("key2")))); + DoubleCounter counter = doubleCounterBuilder.build(); + counter.add(1, ATTRIBUTES); + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasDoubleSumSatisfying( + sum -> + sum.hasPointsSatisfying( + point -> + point.hasAttributesSatisfyingExactly( + equalTo(stringKey("key2"), "2"), + equalTo(stringKey("key3"), "3"))))); + } +} diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AdviceTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java similarity index 99% rename from sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AdviceTest.java rename to sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java index 27975d8dce6..e25eb098e9e 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/AdviceTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/ExplicitBucketBoundariesAdviceTest.java @@ -25,7 +25,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -class AdviceTest { +class ExplicitBucketBoundariesAdviceTest { private SdkMeterProvider meterProvider = SdkMeterProvider.builder().build(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableInstrumentTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableInstrumentTest.java index 2a865288678..a2b9e60a60f 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableInstrumentTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableInstrumentTest.java @@ -83,7 +83,7 @@ void stringRepresentation() { + "unit=unit, " + "type=COUNTER, " + "valueType=DOUBLE, " - + "advice=Advice{explicitBucketBoundaries=null}}" + + "advice=Advice{explicitBucketBoundaries=null, attributes=null}}" + "]}}"); } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/CallbackRegistrationTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/CallbackRegistrationTest.java index b7c1dae870b..a15f8e4affd 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/CallbackRegistrationTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/CallbackRegistrationTest.java @@ -106,7 +106,7 @@ void stringRepresentation() { + "unit=unit, " + "type=OBSERVABLE_COUNTER, " + "valueType=LONG, " - + "advice=Advice{explicitBucketBoundaries=null}" + + "advice=Advice{explicitBucketBoundaries=null, attributes=null}" + "}]}"); assertThat( CallbackRegistration.create(Arrays.asList(measurement1, measurement2), callback) @@ -120,14 +120,14 @@ void stringRepresentation() { + "unit=unit, " + "type=OBSERVABLE_COUNTER, " + "valueType=LONG, " - + "advice=Advice{explicitBucketBoundaries=null}}, " + + "advice=Advice{explicitBucketBoundaries=null, attributes=null}}, " + "InstrumentDescriptor{" + "name=long-counter, " + "description=description, " + "unit=unit, " + "type=OBSERVABLE_COUNTER, " + "valueType=LONG, " - + "advice=Advice{explicitBucketBoundaries=null}" + + "advice=Advice{explicitBucketBoundaries=null, attributes=null}" + "}]}"); } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/AdviceAttributesProcessorTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/AdviceAttributesProcessorTest.java new file mode 100644 index 00000000000..294c05ea157 --- /dev/null +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/AdviceAttributesProcessorTest.java @@ -0,0 +1,51 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics.internal.view; + +import static io.opentelemetry.api.common.AttributeKey.longKey; +import static io.opentelemetry.api.common.AttributeKey.stringKey; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.entry; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.context.Context; +import org.junit.jupiter.api.Test; + +class AdviceAttributesProcessorTest { + + @Test + void doesNotUseContext() { + assertThat(new AdviceAttributesProcessor(emptyList()).usesContext()).isFalse(); + } + + @Test + void removeUnwantedAttributes() { + AttributesProcessor processor = + new AdviceAttributesProcessor(asList(stringKey("abc"), stringKey("def"), stringKey("ghi"))); + + Attributes result = + processor.process( + Attributes.builder() + .put(stringKey("abc"), "abc") + .put(stringKey("def"), "def") + .put(longKey("ghi"), 42L) + .put(stringKey("xyz"), "xyz") + .build(), + Context.root()); + + // does not contain key "ghi" because the type is different (stringKey != longKey) + assertThat(result).containsOnly(entry(stringKey("abc"), "abc"), entry(stringKey("def"), "def")); + } + + @Test + void stringRepresentation() { + assertThat(new AdviceAttributesProcessor(singletonList(stringKey("abc"))).toString()) + .isEqualTo("AdviceAttributesProcessor{attributeKeys=[abc]}"); + } +} diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java index 906a3e0856f..abba1c32133 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/view/ViewRegistryTest.java @@ -5,11 +5,13 @@ package io.opentelemetry.sdk.metrics.internal.view; +import static io.opentelemetry.api.common.AttributeKey.stringKey; import static io.opentelemetry.sdk.metrics.internal.view.ViewRegistry.DEFAULT_REGISTERED_VIEW; import static io.opentelemetry.sdk.metrics.internal.view.ViewRegistry.toGlobPatternPredicate; import static org.assertj.core.api.Assertions.assertThat; import io.github.netmikey.logunit.api.LogCapturer; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.metrics.Aggregation; @@ -25,6 +27,7 @@ import io.opentelemetry.sdk.metrics.internal.state.MetricStorage; import java.util.Arrays; import java.util.Collections; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -431,6 +434,117 @@ void defaults() { assertThat(logs.getEvents()).hasSize(0); } + @Test + void findViews_ApplyAdvice() { + // use incompatible aggregation for histogram + DefaultAggregationSelector aggregationSelector = + instrumentType -> + instrumentType == InstrumentType.HISTOGRAM + ? Aggregation.lastValue() + : Aggregation.defaultAggregation(); + + RegisteredView registeredView = + registeredView( + InstrumentSelector.builder().setName("test").build(), + View.builder().setDescription("view applied").build()); + ViewRegistry viewRegistry = + ViewRegistry.create( + aggregationSelector, + CardinalityLimitSelector.defaultCardinalityLimitSelector(), + Collections.singletonList(registeredView)); + + // If a view matches the descriptor, use it and ignore the advice + assertThat( + viewRegistry.findViews( + InstrumentDescriptor.create( + "test", + "", + "", + InstrumentType.COUNTER, + InstrumentValueType.DOUBLE, + Advice.builder() + .setAttributes(Arrays.asList(stringKey("key1"), stringKey("key2"))) + .build()), + INSTRUMENTATION_SCOPE_INFO)) + .isEqualTo(Collections.singletonList(registeredView)); + + // If there is no matching view and attributes advice was defined, use it + assertThat( + viewRegistry.findViews( + InstrumentDescriptor.create( + "advice", + "", + "", + InstrumentType.COUNTER, + InstrumentValueType.DOUBLE, + Advice.builder() + .setAttributes(Arrays.asList(stringKey("key1"), stringKey("key2"))) + .build()), + INSTRUMENTATION_SCOPE_INFO)) + .hasSize(1) + .element(0) + .satisfies( + view -> { + assertThat(view) + .as("is the same as the default view, except the attributes processor") + .usingRecursiveComparison() + .ignoringFields("viewAttributesProcessor") + .isEqualTo(DEFAULT_REGISTERED_VIEW); + assertThat(view) + .as("has the advice attributes processor") + .extracting("viewAttributesProcessor") + .isInstanceOf(AdviceAttributesProcessor.class) + .extracting( + "attributeKeys", InstanceOfAssertFactories.collection(AttributeKey.class)) + .containsExactlyInAnyOrder(stringKey("key1"), stringKey("key2")); + }); + + // If there is no matching view and attributes advice was defined, use it - incompatible + // aggregation case + assertThat( + viewRegistry.findViews( + InstrumentDescriptor.create( + "histogram_advice", + "", + "", + InstrumentType.HISTOGRAM, + InstrumentValueType.DOUBLE, + Advice.builder() + .setAttributes(Arrays.asList(stringKey("key1"), stringKey("key2"))) + .build()), + INSTRUMENTATION_SCOPE_INFO)) + .hasSize(1) + .element(0) + .satisfies( + view -> { + assertThat(view) + .as("is the same as the default view, except the attributes processor") + .usingRecursiveComparison() + .ignoringFields("viewAttributesProcessor") + .isEqualTo(DEFAULT_REGISTERED_VIEW); + assertThat(view) + .as("has the advice attributes processor") + .extracting("viewAttributesProcessor") + .isInstanceOf(AdviceAttributesProcessor.class) + .extracting( + "attributeKeys", InstanceOfAssertFactories.collection(AttributeKey.class)) + .containsExactlyInAnyOrder(stringKey("key1"), stringKey("key2")); + }); + + // if advice is not defined, use the default view + assertThat( + viewRegistry.findViews( + InstrumentDescriptor.create( + "advice", + "", + "", + InstrumentType.COUNTER, + InstrumentValueType.DOUBLE, + Advice.empty()), + INSTRUMENTATION_SCOPE_INFO)) + .isEqualTo(Collections.singletonList(DEFAULT_REGISTERED_VIEW)); + } + @Test void matchesName() { assertThat(toGlobPatternPredicate("foo").test("foo")).isTrue();