revert + alternative approach

Author: SylvainJuge

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ComposableRuleBasedSamplerFactory.java

@@ -98,10 +98,15 @@ private static AttributeMatcher attributePatternsMatcher(
     if (attributePatternsModel == null) {
       return null;
     }
+    List<String> included = attributePatternsModel.getIncluded();
+    if (included != null && included.isEmpty()) {
+      // empty included should be treated as include everything by default.
+      included = null;
+    }
     return new AttributeMatcher(
         requireNonNull(attributePatternsModel.getKey(), "attribute_patterns key"),
         IncludeExcludePredicate.createPatternMatching(
-            attributePatternsModel.getIncluded(), attributePatternsModel.getExcluded()));
+            included, attributePatternsModel.getExcluded()));
   }
 
   // Visible for testing

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ResourceFactory.java

@@ -93,6 +93,11 @@ private static Predicate<String> 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);
   }
 }

sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ResourceFactoryTest.java

@@ -141,7 +141,12 @@ private static Stream<Arguments> createWithDetectorsArgs() {
         Arguments.of(
             Collections.singletonList("*o*"),
             Collections.singletonList("order"),
-            Resource.getDefault().toBuilder().put("color", "red").build()));
+            Resource.getDefault().toBuilder().put("color", "red").build()),
+        // empty include should be treated as include all
+        Arguments.of(
+            Collections.emptyList(),
+            Collections.singletonList("order"),
+            Resource.getDefault().toBuilder().put("color", "red").put("shape", "square").build()));
   }
 
   @ParameterizedTest

sdk/common/src/main/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicate.java

@@ -9,7 +9,6 @@
 import static java.util.stream.Collectors.joining;
 
 import java.util.Collection;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.StringJoiner;
@@ -30,29 +29,28 @@
 public final class IncludeExcludePredicate implements Predicate<String> {
 
   private final boolean globMatchingEnabled;
-  private final Set<String> included;
-  private final Set<String> excluded;
+  @Nullable private final Set<String> included;
+  @Nullable private final Set<String> excluded;
   private final Predicate<String> predicate;
 
   private IncludeExcludePredicate(
       @Nullable Collection<String> included,
       @Nullable Collection<String> excluded,
       boolean globMatchingEnabled) {
     this.globMatchingEnabled = globMatchingEnabled;
-    this.included = included == null ? Collections.emptySet() : new LinkedHashSet<>(included);
-    this.excluded = excluded == null ? Collections.emptySet() : new LinkedHashSet<>(excluded);
-    if (this.included.isEmpty() && this.excluded.isEmpty()) {
-      throw new IllegalArgumentException(
-          "At least one of include or exclude patterns must not be null or empty");
-    }
-    if (this.included.isEmpty()) {
-      this.predicate = excludedPredicate(this.excluded, globMatchingEnabled);
-    } else if (this.excluded.isEmpty()) {
-      this.predicate = includedPredicate(this.included, globMatchingEnabled);
-    } else {
+    this.included = included == null ? null : new LinkedHashSet<>(included);
+    this.excluded = excluded == null ? null : new LinkedHashSet<>(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) {
+      this.predicate = excludedPredicate(this.excluded, globMatchingEnabled);
+    } else if (this.excluded == null && this.included != null) {
+      this.predicate = includedPredicate(this.included, globMatchingEnabled);
+    } else {
+      throw new IllegalArgumentException(
+          "At least one of includedPatterns or excludedPatterns must not be null");
     }
   }
 
@@ -87,10 +85,10 @@ public boolean test(String s) {
   public String toString() {
     StringJoiner joiner = new StringJoiner(", ", "IncludeExcludePredicate{", "}");
     joiner.add("globMatchingEnabled=" + globMatchingEnabled);
-    if (!included.isEmpty()) {
+    if (included != null) {
       joiner.add("included=" + included.stream().collect(joining(", ", "[", "]")));
     }
-    if (!excluded.isEmpty()) {
+    if (excluded != null) {
       joiner.add("excluded=" + excluded.stream().collect(joining(", ", "[", "]")));
     }
     return joiner.toString();

sdk/common/src/test/java/io/opentelemetry/sdk/common/internal/IncludeExcludePredicateTest.java

@@ -21,11 +21,13 @@ class IncludeExcludePredicateTest {
   private static final Predicate<String> EXACT_INCLUDE =
       IncludeExcludePredicate.createExactMatching(singletonList("foo"), null);
   private static final Predicate<String> EXACT_EXCLUDE =
-      IncludeExcludePredicate.createExactMatching(Collections.emptyList(), singletonList("bar"));
+      IncludeExcludePredicate.createExactMatching(null, singletonList("bar"));
   private static final Predicate<String> EXACT_INCLUDE_AND_EXCLUDE =
       IncludeExcludePredicate.createExactMatching(singletonList("foo"), singletonList("bar"));
   private static final Predicate<String> EXACT_MULTI =
       IncludeExcludePredicate.createExactMatching(asList("foo", "fooo"), asList("bar", "barr"));
+  public static final Predicate<String> EXACT_INCLUDE_NONE =
+      IncludeExcludePredicate.createExactMatching(Collections.emptyList(), null);
 
   private static final Predicate<String> PATTERN_INCLUDE =
       IncludeExcludePredicate.createPatternMatching(singletonList("f?o"), null);
@@ -35,6 +37,8 @@ class IncludeExcludePredicateTest {
       IncludeExcludePredicate.createPatternMatching(singletonList("f?o"), singletonList("b?r"));
   private static final Predicate<String> PATTERN_MULTI =
       IncludeExcludePredicate.createPatternMatching(asList("f?o", "f?oo"), asList("b?r", "b?rr"));
+  public static final Predicate<String> PATTERN_INCLUDE_NONE =
+      IncludeExcludePredicate.createPatternMatching(Collections.emptyList(), null);
 
   @ParameterizedTest
   @MethodSource("testArgs")
@@ -71,6 +75,8 @@ private static Stream<Arguments> 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),
@@ -100,7 +106,9 @@ private static Stream<Arguments> 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));
+        Arguments.of(PATTERN_MULTI, "baz", false),
+        // include none
+        Arguments.of(PATTERN_INCLUDE_NONE, "foo", false));
   }
 
   @ParameterizedTest