Skip to content

Commit cd56e52

Browse files
jack-bergjack-berg
committed
Enforce IncludedExcludeModel .included and .excluded are not empty
1 parent ec002c3 commit cd56e52

7 files changed

Lines changed: 148 additions & 27 deletions

File tree

‎exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/internal/PrometheusComponentProvider.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.exporter.prometheus.internal;
77

8+
import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
89
import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
910
import io.opentelemetry.exporter.prometheus.PrometheusHttpServer;
1011
import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder;
@@ -59,10 +60,14 @@ public MetricReader create(DeclarativeConfigProperties config) {
5960
if (withResourceConstantLabels != null) {
6061
List<String> included = withResourceConstantLabels.getScalarList("included", String.class);
6162
List<String> excluded = withResourceConstantLabels.getScalarList("excluded", String.class);
62-
if (included != null || excluded != null) {
63-
prometheusBuilder.setAllowedResourceAttributesFilter(
64-
IncludeExcludePredicate.createPatternMatching(included, excluded));
63+
if (included != null && included.isEmpty()) {
64+
throw new DeclarativeConfigException("included must not be empty");
6565
}
66+
if (excluded != null && excluded.isEmpty()) {
67+
throw new DeclarativeConfigException("excluded must not be empty");
68+
}
69+
prometheusBuilder.setAllowedResourceAttributesFilter(
70+
IncludeExcludePredicate.createPatternMatching(included, excluded));
6671
}
6772

6873
return prometheusBuilder.build();

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.sdk.extension.incubator.fileconfig;
7+
8+
import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
9+
import io.opentelemetry.sdk.common.internal.IncludeExcludePredicate;
10+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.IncludeExcludeModel;
11+
import java.util.List;
12+
import java.util.function.Predicate;
13+
14+
final class IncludeExcludeFactory implements Factory<IncludeExcludeModel, Predicate<String>> {
15+
16+
private static final IncludeExcludeFactory INSTANCE = new IncludeExcludeFactory();
17+
18+
private IncludeExcludeFactory() {}
19+
20+
static IncludeExcludeFactory getInstance() {
21+
return INSTANCE;
22+
}
23+
24+
@Override
25+
public Predicate<String> create(IncludeExcludeModel model, DeclarativeConfigContext context) {
26+
List<String> included = model.getIncluded();
27+
if (included != null && included.isEmpty()) {
28+
throw new DeclarativeConfigException(".included must not be empty");
29+
}
30+
List<String> excluded = model.getExcluded();
31+
if (excluded != null && excluded.isEmpty()) {
32+
throw new DeclarativeConfigException(".excluded must not be empty");
33+
}
34+
35+
return IncludeExcludePredicate.createPatternMatching(included, excluded);
36+
}
37+
}

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import io.opentelemetry.api.common.Attributes;
1212
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
13-
import io.opentelemetry.sdk.common.internal.IncludeExcludePredicate;
1413
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.AttributeNameValueModel;
1514
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalResourceDetectionModel;
1615
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalResourceDetectorModel;
@@ -21,7 +20,6 @@
2120
import java.util.Collections;
2221
import java.util.List;
2322
import java.util.function.Predicate;
24-
import javax.annotation.Nullable;
2523

2624
final class ResourceFactory implements Factory<ResourceModel, Resource> {
2725

@@ -49,8 +47,11 @@ public Resource create(ResourceModel model, DeclarativeConfigContext context) {
4947
}
5048
}
5149

50+
IncludeExcludeModel attributesIncludeExcludeModel = detectionModel.getAttributes();
5251
Predicate<String> detectorAttributeFilter =
53-
detectorAttributeFilter(detectionModel.getAttributes());
52+
attributesIncludeExcludeModel == null
53+
? ResourceFactory::matchAll
54+
: IncludeExcludeFactory.getInstance().create(attributesIncludeExcludeModel, context);
5455
Attributes filteredDetectedAttributes =
5556
detectedResourceBuilder.build().getAttributes().toBuilder()
5657
.removeIf(attributeKey -> !detectorAttributeFilter.test(attributeKey.getKey()))
@@ -84,13 +85,4 @@ public Resource create(ResourceModel model, DeclarativeConfigContext context) {
8485
private static boolean matchAll(String attributeKey) {
8586
return true;
8687
}
87-
88-
private static Predicate<String> detectorAttributeFilter(
89-
@Nullable IncludeExcludeModel includedExcludeModel) {
90-
if (includedExcludeModel == null) {
91-
return ResourceFactory::matchAll;
92-
}
93-
return IncludeExcludePredicate.createPatternMatching(
94-
includedExcludeModel.getIncluded(), includedExcludeModel.getExcluded());
95-
}
9688
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@
55

66
package io.opentelemetry.sdk.extension.incubator.fileconfig;
77

8-
import io.opentelemetry.sdk.common.internal.IncludeExcludePredicate;
98
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.IncludeExcludeModel;
109
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ViewStreamModel;
1110
import io.opentelemetry.sdk.metrics.View;
1211
import io.opentelemetry.sdk.metrics.ViewBuilder;
13-
import java.util.List;
1412

1513
final class ViewFactory implements Factory<ViewStreamModel, View> {
1614

@@ -33,9 +31,8 @@ public View create(ViewStreamModel model, DeclarativeConfigContext context) {
3331
}
3432
IncludeExcludeModel attributeKeys = model.getAttributeKeys();
3533
if (attributeKeys != null) {
36-
List<String> included = attributeKeys.getIncluded();
37-
List<String> excluded = attributeKeys.getExcluded();
38-
builder.setAttributeFilter(IncludeExcludePredicate.createExactMatching(included, excluded));
34+
builder.setAttributeFilter(
35+
IncludeExcludeFactory.getInstance().create(attributeKeys, context));
3936
}
4037
if (model.getAggregation() != null) {
4138
builder.setAggregation(

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.sdk.extension.incubator.fileconfig;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
10+
11+
import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
12+
import io.opentelemetry.common.ComponentLoader;
13+
import io.opentelemetry.sdk.common.internal.IncludeExcludePredicate;
14+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.IncludeExcludeModel;
15+
import java.util.Arrays;
16+
import java.util.Collections;
17+
import java.util.function.Predicate;
18+
import java.util.stream.Stream;
19+
import org.junit.jupiter.params.ParameterizedTest;
20+
import org.junit.jupiter.params.provider.Arguments;
21+
import org.junit.jupiter.params.provider.MethodSource;
22+
23+
class IncludeExcludeFactoryTest {
24+
25+
private final DeclarativeConfigContext context =
26+
new DeclarativeConfigContext(ComponentLoader.forClassLoader(getClass().getClassLoader()));
27+
28+
@ParameterizedTest
29+
@MethodSource("createArguments")
30+
void create(IncludeExcludeModel model, Predicate<String> expectedPredicate) {
31+
Predicate<String> predicate = IncludeExcludeFactory.getInstance().create(model, context);
32+
assertThat(predicate.toString()).isEqualTo(expectedPredicate.toString());
33+
}
34+
35+
private static Stream<Arguments> createArguments() {
36+
return Stream.of(
37+
// included null, excluded null
38+
Arguments.of(
39+
new IncludeExcludeModel().withIncluded(null).withExcluded(null),
40+
IncludeExcludePredicate.createPatternMatching(null, null)),
41+
// included present, excluded null
42+
Arguments.of(
43+
new IncludeExcludeModel().withIncluded(Arrays.asList("foo", "bar")).withExcluded(null),
44+
IncludeExcludePredicate.createPatternMatching(Arrays.asList("foo", "bar"), null)),
45+
// included null, excluded present
46+
Arguments.of(
47+
new IncludeExcludeModel()
48+
.withIncluded(null)
49+
.withExcluded(Collections.singletonList("baz")),
50+
IncludeExcludePredicate.createPatternMatching(null, Collections.singletonList("baz"))),
51+
// both included and excluded present
52+
Arguments.of(
53+
new IncludeExcludeModel()
54+
.withIncluded(Arrays.asList("foo", "bar"))
55+
.withExcluded(Collections.singletonList("baz")),
56+
IncludeExcludePredicate.createPatternMatching(
57+
Arrays.asList("foo", "bar"), Collections.singletonList("baz"))));
58+
}
59+
60+
@ParameterizedTest
61+
@MethodSource("createInvalidArguments")
62+
void createInvalid(IncludeExcludeModel model, String expectedMessage) {
63+
assertThatThrownBy(() -> IncludeExcludeFactory.getInstance().create(model, context))
64+
.isInstanceOf(DeclarativeConfigException.class)
65+
.hasMessage(expectedMessage);
66+
}
67+
68+
private static Stream<Arguments> createInvalidArguments() {
69+
return Stream.of(
70+
Arguments.of(
71+
new IncludeExcludeModel().withIncluded(Collections.emptyList()).withExcluded(null),
72+
".included must not be empty"),
73+
Arguments.of(
74+
new IncludeExcludeModel().withIncluded(null).withExcluded(Collections.emptyList()),
75+
".excluded must not be empty"));
76+
}
77+
}

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,6 @@ private static Stream<Arguments> createWithDetectorsArgs() {
145145
Collections.singletonList("*o*"),
146146
Collections.singletonList("order"),
147147
Resource.getDefault().toBuilder().put("color", "red").build()),
148-
// empty or missing include should be treated as include all
149-
Arguments.of(
150-
Collections.emptyList(),
151-
Collections.singletonList("order"),
152-
Resource.getDefault().toBuilder().put("color", "red").put("shape", "square").build()),
153148
Arguments.of(
154149
null,
155150
Collections.singletonList("order"),
@@ -191,6 +186,24 @@ private static Stream<Arguments> createInvalidDetectorsArgs() {
191186
new ExperimentalResourceDetectionModel()
192187
.withDetectors(
193188
Collections.singletonList(new ExperimentalResourceDetectorModel()))),
194-
"resource detector must have exactly one entry but has 0"));
189+
"resource detector must have exactly one entry but has 0"),
190+
Arguments.of(
191+
new ResourceModel()
192+
.withDetectionDevelopment(
193+
new ExperimentalResourceDetectionModel()
194+
.withAttributes(
195+
new IncludeExcludeModel()
196+
.withIncluded(Collections.emptyList())
197+
.withExcluded(null))),
198+
".included must not be empty"),
199+
Arguments.of(
200+
new ResourceModel()
201+
.withDetectionDevelopment(
202+
new ExperimentalResourceDetectionModel()
203+
.withAttributes(
204+
new IncludeExcludeModel()
205+
.withIncluded(null)
206+
.withExcluded(Collections.emptyList()))),
207+
".excluded must not be empty"));
195208
}
196209
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ void create() {
4242
.setName("name")
4343
.setDescription("description")
4444
.setAttributeFilter(
45-
IncludeExcludePredicate.createExactMatching(
45+
IncludeExcludePredicate.createPatternMatching(
4646
Arrays.asList("foo", "bar"), Collections.singletonList("baz")))
4747
.setAggregation(
4848
Aggregation.explicitBucketHistogram(

0 commit comments

Comments
 (0)