diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ResourceFactory.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ResourceFactory.java index 3f849c836af..e43de54a34c 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ResourceFactory.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ResourceFactory.java @@ -93,11 +93,6 @@ private static Predicate detectorAttributeFilter( if (included == null && excluded == null) { return ResourceFactory::matchAll; } - // when "included" is omitted in configuration, we get an empty list - // in this context it should be interpreted as "include everything" - if (included != null && included.isEmpty()) { - included = null; - } return IncludeExcludePredicate.createPatternMatching(included, excluded); } } diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ViewFactory.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ViewFactory.java index 2f4c598201b..1860c3dea9c 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ViewFactory.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ViewFactory.java @@ -35,9 +35,7 @@ public View create(ViewStreamModel model, DeclarativeConfigContext context) { if (attributeKeys != null) { List included = attributeKeys.getIncluded(); List excluded = attributeKeys.getExcluded(); - if (included != null || excluded != null) { - builder.setAttributeFilter(IncludeExcludePredicate.createExactMatching(included, excluded)); - } + builder.setAttributeFilter(IncludeExcludePredicate.createExactMatching(included, excluded)); } if (model.getAggregation() != null) { builder.setAggregation( diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ComposableRuleBasedSamplerFactoryTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ComposableRuleBasedSamplerFactoryTest.java index 8a8da213fd1..68525e0abd0 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ComposableRuleBasedSamplerFactoryTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ComposableRuleBasedSamplerFactoryTest.java @@ -162,7 +162,7 @@ private static Stream createTestCases() { private static final AttributeKey HTTP_PATH = AttributeKey.stringKey("http.path"); @ParameterizedTest - @MethodSource("declarativeCOnfigSamplingPredicateArgs") + @MethodSource("declarativeConfigSamplingPredicateArgs") void declarativeConfigSamplingPredicate( DeclarativeConfigSamplingPredicate predicate, Context context, @@ -174,7 +174,7 @@ void declarativeConfigSamplingPredicate( } @SuppressWarnings("unused") - private static Stream declarativeCOnfigSamplingPredicateArgs() { + private static Stream declarativeConfigSamplingPredicateArgs() { DeclarativeConfigSamplingPredicate matchAll = new DeclarativeConfigSamplingPredicate(null, null, null, null); DeclarativeConfigSamplingPredicate valuesMatcher = diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicate.java b/sdk/common/src/main/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicate.java index dd6ba4e63b4..5475c1e2f4d 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicate.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicate.java @@ -38,26 +38,28 @@ private IncludeExcludePredicate( @Nullable Collection excluded, boolean globMatchingEnabled) { this.globMatchingEnabled = globMatchingEnabled; - this.included = included == null ? null : new LinkedHashSet<>(included); - this.excluded = excluded == null ? null : new LinkedHashSet<>(excluded); + this.included = copyIfNotEmpty(included); + this.excluded = copyIfNotEmpty(excluded); if (this.included != null && this.excluded != null) { this.predicate = includedPredicate(this.included, globMatchingEnabled) .and(excludedPredicate(this.excluded, globMatchingEnabled)); - } else if (this.included == null && this.excluded != null) { + } else if (this.excluded != null) { this.predicate = excludedPredicate(this.excluded, globMatchingEnabled); - } else if (this.excluded == null && this.included != null) { + } else if (this.included != null) { this.predicate = includedPredicate(this.included, globMatchingEnabled); } else { - throw new IllegalArgumentException( - "At least one of includedPatterns or excludedPatterns must not be null"); + // include everything by default if both included and excluded are null or empt + this.predicate = s -> true; } } /** * Create a (case-sensitive) exact matching include exclude predicate. * - * @throws IllegalArgumentException if {@code included} AND {@code excluded} are null. + *

When {@code included} is empty or {@literal null}, all values are included by default. + * + *

When {@code excluded} is empty or {@literal null}, no value is excluded by default. */ public static Predicate createExactMatching( @Nullable Collection included, @Nullable Collection excluded) { @@ -67,9 +69,11 @@ public static Predicate createExactMatching( /** * Create a pattern matching include exclude predicate. * - *

See {@link GlobUtil} for pattern matching details. + *

When {@code included} is empty or {@literal null}, all values are included by default. + * + *

When {@code excluded} is empty or {@literal null}, no value is excluded by default. * - * @throws IllegalArgumentException if {@code included} AND {@code excluded} are null. + *

See {@link GlobUtil} for pattern matching details. */ public static Predicate createPatternMatching( @Nullable Collection included, @Nullable Collection excluded) { @@ -119,4 +123,9 @@ private static Predicate excludedPredicate( } return result; } + + @Nullable + private static Set copyIfNotEmpty(@Nullable Collection collection) { + return collection == null || collection.isEmpty() ? null : new LinkedHashSet<>(collection); + } } diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicateTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicateTest.java index 55b2c02472f..5eb6e0a02f7 100644 --- a/sdk/common/src/test/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicateTest.java +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicateTest.java @@ -9,9 +9,13 @@ import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.function.Predicate; import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -26,8 +30,6 @@ class IncludeExcludePredicateTest { IncludeExcludePredicate.createExactMatching(singletonList("foo"), singletonList("bar")); private static final Predicate EXACT_MULTI = IncludeExcludePredicate.createExactMatching(asList("foo", "fooo"), asList("bar", "barr")); - public static final Predicate EXACT_INCLUDE_NONE = - IncludeExcludePredicate.createExactMatching(Collections.emptyList(), null); private static final Predicate PATTERN_INCLUDE = IncludeExcludePredicate.createPatternMatching(singletonList("f?o"), null); @@ -37,8 +39,6 @@ class IncludeExcludePredicateTest { IncludeExcludePredicate.createPatternMatching(singletonList("f?o"), singletonList("b?r")); private static final Predicate PATTERN_MULTI = IncludeExcludePredicate.createPatternMatching(asList("f?o", "f?oo"), asList("b?r", "b?rr")); - public static final Predicate PATTERN_INCLUDE_NONE = - IncludeExcludePredicate.createPatternMatching(Collections.emptyList(), null); @ParameterizedTest @MethodSource("testArgs") @@ -75,8 +75,6 @@ private static Stream testArgs() { Arguments.of(EXACT_MULTI, "bar", false), Arguments.of(EXACT_MULTI, "barr", false), Arguments.of(EXACT_MULTI, "baz", false), - // include none - Arguments.of(EXACT_INCLUDE_NONE, "foo", false), // pattern matching // include only Arguments.of(PATTERN_INCLUDE, "foo", true), @@ -106,9 +104,7 @@ private static Stream testArgs() { Arguments.of(PATTERN_MULTI, "bAr", false), Arguments.of(PATTERN_MULTI, "barr", false), Arguments.of(PATTERN_MULTI, "bArr", false), - Arguments.of(PATTERN_MULTI, "baz", false), - // include none - Arguments.of(PATTERN_INCLUDE_NONE, "foo", false)); + Arguments.of(PATTERN_MULTI, "baz", false)); } @ParameterizedTest @@ -140,4 +136,30 @@ private static Stream stringRepresentationArgs() { PATTERN_MULTI, "IncludeExcludePredicate{globMatchingEnabled=true, included=[f?o, f?oo], excluded=[b?r, b?rr]}")); } + + @Test + void emptyOrNullShouldIncludeAll() { + shouldIncludeAll(null, null); + shouldIncludeAll(Collections.emptyList(), null); + shouldIncludeAll(null, Collections.emptyList()); + shouldIncludeAll(Collections.emptyList(), Collections.emptyList()); + } + + private static void shouldIncludeAll( + @Nullable Collection included, @Nullable Collection excluded) { + Arrays.asList("foo", "fooo", "fOo", "FOO", "bar", "bAr", "barr", "bArr", "baz") + .forEach( + s -> { + Predicate exactMatching = + IncludeExcludePredicate.createExactMatching(included, excluded); + assertThat(exactMatching.test(s)).isTrue(); + assertThat(exactMatching.toString()) + .isEqualTo("IncludeExcludePredicate{globMatchingEnabled=false}"); + Predicate patternMatching = + IncludeExcludePredicate.createPatternMatching(included, excluded); + assertThat(patternMatching.test(s)).isTrue(); + assertThat(patternMatching.toString()) + .isEqualTo("IncludeExcludePredicate{globMatchingEnabled=true}"); + }); + } } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewBuilder.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewBuilder.java index d2ba1b53e33..a78b5fbe41e 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewBuilder.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/ViewBuilder.java @@ -74,6 +74,11 @@ public ViewBuilder setAggregation(Aggregation aggregation) { */ public ViewBuilder setAttributeFilter(Set keysToRetain) { Objects.requireNonNull(keysToRetain, "keysToRetain"); + if (keysToRetain.isEmpty()) { + // include/exclude predicate requires to include or exclude at least one, we have to skip it + // when we don't want to include any key. + return setAttributeFilter(s -> false); + } return setAttributeFilter(IncludeExcludePredicate.createExactMatching(keysToRetain, null)); }