Skip to content

Commit 826c4e9

Browse files
jack-bergjack-berg
authored
Improve pattern for validating and loading SDK extension plugins (#7947)
1 parent da310cc commit 826c4e9

17 files changed

+290
-359
lines changed

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

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,8 @@
77

88
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalComposableParentThresholdSamplerModel;
99
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalComposableProbabilitySamplerModel;
10-
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalComposableRuleBasedSamplerModel;
1110
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalComposableSamplerModel;
1211
import io.opentelemetry.sdk.extension.incubator.trace.samplers.ComposableSampler;
13-
import java.util.Map;
1412

1513
final class ComposableSamplerFactory
1614
implements Factory<ExperimentalComposableSamplerModel, ComposableSampler> {
@@ -26,33 +24,46 @@ static ComposableSamplerFactory getInstance() {
2624
@Override
2725
public ComposableSampler create(
2826
ExperimentalComposableSamplerModel model, DeclarativeConfigContext context) {
27+
// We don't use the variable till later but call validate first to confirm there are not
28+
// multiple samplers.
29+
ConfigKeyValue samplerKeyValue =
30+
FileConfigUtil.validateSingleKeyValue(context, model, "composable sampler");
31+
2932
if (model.getAlwaysOn() != null) {
3033
return ComposableSampler.alwaysOn();
3134
}
3235
if (model.getAlwaysOff() != null) {
3336
return ComposableSampler.alwaysOff();
3437
}
35-
ExperimentalComposableProbabilitySamplerModel probability = model.getProbability();
36-
if (probability != null) {
37-
Double ratio = probability.getRatio();
38-
if (ratio == null) {
39-
ratio = 1.0d;
40-
}
41-
return ComposableSampler.probability(ratio);
38+
if (model.getProbability() != null) {
39+
return createProbabilitySampler(model.getProbability());
4240
}
43-
ExperimentalComposableRuleBasedSamplerModel ruleBased = model.getRuleBased();
44-
if (ruleBased != null) {
45-
return ComposableRuleBasedSamplerFactory.getInstance().create(ruleBased, context);
41+
if (model.getRuleBased() != null) {
42+
return ComposableRuleBasedSamplerFactory.getInstance().create(model.getRuleBased(), context);
4643
}
47-
ExperimentalComposableParentThresholdSamplerModel parentThreshold = model.getParentThreshold();
48-
if (parentThreshold != null) {
49-
ExperimentalComposableSamplerModel rootModel =
50-
FileConfigUtil.requireNonNull(parentThreshold.getRoot(), "parent threshold sampler root");
51-
ComposableSampler rootSampler = INSTANCE.create(rootModel, context);
52-
return ComposableSampler.parentThreshold(rootSampler);
44+
if (model.getParentThreshold() != null) {
45+
return createParentThresholdSampler(model.getParentThreshold(), context);
5346
}
54-
Map.Entry<String, ?> keyValue =
55-
FileConfigUtil.getSingletonMapEntry(model.getAdditionalProperties(), "composable sampler");
56-
return context.loadComponent(ComposableSampler.class, keyValue.getKey(), keyValue.getValue());
47+
48+
return context.loadComponent(ComposableSampler.class, samplerKeyValue);
49+
}
50+
51+
private static ComposableSampler createProbabilitySampler(
52+
ExperimentalComposableProbabilitySamplerModel probabilityModel) {
53+
Double ratio = probabilityModel.getRatio();
54+
if (ratio == null) {
55+
ratio = 1.0d;
56+
}
57+
return ComposableSampler.probability(ratio);
58+
}
59+
60+
private static ComposableSampler createParentThresholdSampler(
61+
ExperimentalComposableParentThresholdSamplerModel parentThresholdModel,
62+
DeclarativeConfigContext context) {
63+
ExperimentalComposableSamplerModel rootModel =
64+
FileConfigUtil.requireNonNull(
65+
parentThresholdModel.getRoot(), "parent threshold sampler root");
66+
ComposableSampler rootSampler = INSTANCE.create(rootModel, context);
67+
return ComposableSampler.parentThreshold(rootSampler);
5768
}
5869
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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.DeclarativeConfigProperties;
9+
10+
/** A key value pair for a YAML mapping node. */
11+
class ConfigKeyValue {
12+
13+
private final String key;
14+
private final DeclarativeConfigProperties value;
15+
16+
private ConfigKeyValue(String key, DeclarativeConfigProperties value) {
17+
this.key = key;
18+
this.value = value;
19+
}
20+
21+
static ConfigKeyValue of(String key, DeclarativeConfigProperties value) {
22+
return new ConfigKeyValue(key, value);
23+
}
24+
25+
String getKey() {
26+
return key;
27+
}
28+
29+
DeclarativeConfigProperties getValue() {
30+
return value;
31+
}
32+
}

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ SpiHelper getSpiHelper() {
8282
* @throws DeclarativeConfigException if no matching providers are found, or if multiple are found
8383
* (i.e. conflict), or if {@link ComponentProvider#create(DeclarativeConfigProperties)} throws
8484
*/
85-
@SuppressWarnings({"unchecked", "rawtypes"})
86-
<T> T loadComponent(Class<T> type, String name, Object model) {
87-
DeclarativeConfigProperties config =
88-
DeclarativeConfiguration.toConfigProperties(model, spiHelper.getComponentLoader());
85+
@SuppressWarnings({"unchecked"})
86+
<T> T loadComponent(Class<T> type, ConfigKeyValue configKeyValue) {
87+
String name = configKeyValue.getKey();
88+
DeclarativeConfigProperties config = configKeyValue.getValue();
8989

9090
// TODO(jack-berg): cache loaded component providers
9191
List<ComponentProvider> componentProviders = spiHelper.load(ComponentProvider.class);
@@ -115,6 +115,9 @@ <T> T loadComponent(Class<T> type, String name, Object model) {
115115

116116
try {
117117
Object component = provider.create(config);
118+
if (component instanceof Closeable) {
119+
closeables.add((Closeable) component);
120+
}
118121
if (component != null && !type.isInstance(component)) {
119122
throw new DeclarativeConfigException(
120123
"Error configuring "

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

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,55 +8,37 @@
88
import static java.util.stream.Collectors.joining;
99

1010
import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
11-
import java.util.Map;
11+
import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
12+
import java.util.Set;
1213
import javax.annotation.Nullable;
1314

1415
final class FileConfigUtil {
1516

1617
private FileConfigUtil() {}
1718

18-
static <T> T assertNotNull(@Nullable T object, String description) {
19-
if (object == null) {
20-
throw new NullPointerException(description + " is null");
21-
}
22-
return object;
23-
}
24-
2519
static <T> T requireNonNull(@Nullable T object, String description) {
2620
if (object == null) {
2721
throw new DeclarativeConfigException(description + " is required but is null");
2822
}
2923
return object;
3024
}
3125

32-
static <T> Map.Entry<String, T> getSingletonMapEntry(
33-
Map<String, T> additionalProperties, String resourceName) {
34-
if (additionalProperties.isEmpty()) {
35-
throw new DeclarativeConfigException(resourceName + " must be set");
36-
}
37-
if (additionalProperties.size() > 1) {
38-
throw new DeclarativeConfigException(
39-
"Invalid configuration - multiple "
40-
+ resourceName
41-
+ "s set: "
42-
+ additionalProperties.keySet().stream().collect(joining(",", "[", "]")));
43-
}
44-
return additionalProperties.entrySet().stream()
45-
.findFirst()
46-
.orElseThrow(
47-
() ->
48-
new IllegalStateException(
49-
"Missing " + resourceName + ". This is a programming error."));
50-
}
51-
52-
static void requireNullResource(
53-
@Nullable Object resource, String resourceName, Map<String, ?> additionalProperties) {
54-
if (resource != null) {
26+
static ConfigKeyValue validateSingleKeyValue(
27+
DeclarativeConfigContext context, Object model, String resourceName) {
28+
DeclarativeConfigProperties modelConfigProperties =
29+
DeclarativeConfiguration.toConfigProperties(
30+
model, context.getSpiHelper().getComponentLoader());
31+
Set<String> propertyKeys = modelConfigProperties.getPropertyKeys();
32+
if (propertyKeys.size() != 1) {
33+
String suffix =
34+
propertyKeys.isEmpty()
35+
? ""
36+
: ": " + propertyKeys.stream().collect(joining(",", "[", "]"));
5537
throw new DeclarativeConfigException(
56-
"Invalid configuration - multiple "
57-
+ resourceName
58-
+ "s set: "
59-
+ additionalProperties.keySet().stream().collect(joining(",", "[", "]")));
38+
resourceName + " must have exactly one entry but has " + propertyKeys.size() + suffix);
6039
}
40+
String key = propertyKeys.iterator().next();
41+
DeclarativeConfigProperties value = modelConfigProperties.getStructured(key);
42+
return ConfigKeyValue.of(key, value == null ? DeclarativeConfigProperties.empty() : value);
6143
}
6244
}

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

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

88
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LogRecordExporterModel;
99
import io.opentelemetry.sdk.logs.export.LogRecordExporter;
10-
import java.util.LinkedHashMap;
11-
import java.util.Map;
1210

1311
final class LogRecordExporterFactory implements Factory<LogRecordExporterModel, LogRecordExporter> {
1412
private static final LogRecordExporterFactory INSTANCE = new LogRecordExporterFactory();
@@ -21,26 +19,8 @@ static LogRecordExporterFactory getInstance() {
2119

2220
@Override
2321
public LogRecordExporter create(LogRecordExporterModel model, DeclarativeConfigContext context) {
24-
Map<String, Object> exporterResourceByName = new LinkedHashMap<>();
25-
26-
if (model.getOtlpHttp() != null) {
27-
exporterResourceByName.put("otlp_http", model.getOtlpHttp());
28-
}
29-
if (model.getOtlpGrpc() != null) {
30-
exporterResourceByName.put("otlp_grpc", model.getOtlpGrpc());
31-
}
32-
if (model.getOtlpFileDevelopment() != null) {
33-
exporterResourceByName.put("otlp_file/development", model.getOtlpFileDevelopment());
34-
}
35-
if (model.getConsole() != null) {
36-
exporterResourceByName.put("console", model.getConsole());
37-
}
38-
exporterResourceByName.putAll(model.getAdditionalProperties());
39-
40-
Map.Entry<String, ?> keyValue =
41-
FileConfigUtil.getSingletonMapEntry(exporterResourceByName, "log record exporter");
42-
LogRecordExporter metricExporter =
43-
context.loadComponent(LogRecordExporter.class, keyValue.getKey(), keyValue.getValue());
44-
return context.addCloseable(metricExporter);
22+
ConfigKeyValue logRecordExporterKeyValue =
23+
FileConfigUtil.validateSingleKeyValue(context, model, "log record exporter");
24+
return context.loadComponent(LogRecordExporter.class, logRecordExporterKeyValue);
4525
}
4626
}

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

Lines changed: 51 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,13 @@
99
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.BatchLogRecordProcessorModel;
1010
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LogRecordExporterModel;
1111
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LogRecordProcessorModel;
12-
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LogRecordProcessorPropertyModel;
1312
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.SimpleLogRecordProcessorModel;
1413
import io.opentelemetry.sdk.logs.LogRecordProcessor;
1514
import io.opentelemetry.sdk.logs.export.BatchLogRecordProcessor;
1615
import io.opentelemetry.sdk.logs.export.BatchLogRecordProcessorBuilder;
1716
import io.opentelemetry.sdk.logs.export.LogRecordExporter;
1817
import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor;
1918
import java.time.Duration;
20-
import java.util.Map;
2119

2220
final class LogRecordProcessorFactory
2321
implements Factory<LogRecordProcessorModel, LogRecordProcessor> {
@@ -33,54 +31,61 @@ static LogRecordProcessorFactory getInstance() {
3331
@Override
3432
public LogRecordProcessor create(
3533
LogRecordProcessorModel model, DeclarativeConfigContext context) {
36-
BatchLogRecordProcessorModel batchModel = model.getBatch();
37-
if (batchModel != null) {
38-
LogRecordExporterModel exporterModel =
39-
FileConfigUtil.requireNonNull(
40-
batchModel.getExporter(), "batch log record processor exporter");
34+
// We don't use the variable till later but call validate first to confirm there are not
35+
// multiple samplers.
36+
ConfigKeyValue processorKeyValue =
37+
FileConfigUtil.validateSingleKeyValue(context, model, "log record processor");
4138

42-
LogRecordExporter logRecordExporter =
43-
LogRecordExporterFactory.getInstance().create(exporterModel, context);
44-
BatchLogRecordProcessorBuilder builder = BatchLogRecordProcessor.builder(logRecordExporter);
45-
if (batchModel.getExportTimeout() != null) {
46-
builder.setExporterTimeout(Duration.ofMillis(batchModel.getExportTimeout()));
47-
}
48-
if (batchModel.getMaxExportBatchSize() != null) {
49-
builder.setMaxExportBatchSize(batchModel.getMaxExportBatchSize());
50-
}
51-
if (batchModel.getMaxQueueSize() != null) {
52-
builder.setMaxQueueSize(batchModel.getMaxQueueSize());
53-
}
54-
if (batchModel.getScheduleDelay() != null) {
55-
builder.setScheduleDelay(Duration.ofMillis(batchModel.getScheduleDelay()));
56-
}
57-
MeterProvider meterProvider = context.getMeterProvider();
58-
if (meterProvider != null) {
59-
builder.setMeterProvider(meterProvider);
60-
}
61-
62-
return context.addCloseable(builder.build());
39+
if (model.getBatch() != null) {
40+
return createBatchLogRecordProcessor(model.getBatch(), context);
41+
}
42+
if (model.getSimple() != null) {
43+
return createSimpleLogRecordProcessor(model.getSimple(), context);
6344
}
6445

65-
SimpleLogRecordProcessorModel simpleModel = model.getSimple();
66-
if (simpleModel != null) {
67-
LogRecordExporterModel exporterModel =
68-
FileConfigUtil.requireNonNull(
69-
simpleModel.getExporter(), "simple log record processor exporter");
70-
LogRecordExporter logRecordExporter =
71-
LogRecordExporterFactory.getInstance().create(exporterModel, context);
72-
MeterProvider meterProvider = context.getMeterProvider();
73-
return context.addCloseable(
74-
SimpleLogRecordProcessor.builder(logRecordExporter)
75-
.setMeterProvider(() -> meterProvider)
76-
.build());
46+
return context.loadComponent(LogRecordProcessor.class, processorKeyValue);
47+
}
48+
49+
private static LogRecordProcessor createBatchLogRecordProcessor(
50+
BatchLogRecordProcessorModel batchModel, DeclarativeConfigContext context) {
51+
LogRecordExporterModel exporterModel =
52+
FileConfigUtil.requireNonNull(
53+
batchModel.getExporter(), "batch log record processor exporter");
54+
55+
LogRecordExporter logRecordExporter =
56+
LogRecordExporterFactory.getInstance().create(exporterModel, context);
57+
BatchLogRecordProcessorBuilder builder = BatchLogRecordProcessor.builder(logRecordExporter);
58+
if (batchModel.getExportTimeout() != null) {
59+
builder.setExporterTimeout(Duration.ofMillis(batchModel.getExportTimeout()));
60+
}
61+
if (batchModel.getMaxExportBatchSize() != null) {
62+
builder.setMaxExportBatchSize(batchModel.getMaxExportBatchSize());
7763
}
64+
if (batchModel.getMaxQueueSize() != null) {
65+
builder.setMaxQueueSize(batchModel.getMaxQueueSize());
66+
}
67+
if (batchModel.getScheduleDelay() != null) {
68+
builder.setScheduleDelay(Duration.ofMillis(batchModel.getScheduleDelay()));
69+
}
70+
MeterProvider meterProvider = context.getMeterProvider();
71+
if (meterProvider != null) {
72+
builder.setMeterProvider(meterProvider);
73+
}
74+
75+
return context.addCloseable(builder.build());
76+
}
7877

79-
Map.Entry<String, LogRecordProcessorPropertyModel> keyValue =
80-
FileConfigUtil.getSingletonMapEntry(
81-
model.getAdditionalProperties(), "log record processor");
82-
LogRecordProcessor logRecordProcessor =
83-
context.loadComponent(LogRecordProcessor.class, keyValue.getKey(), keyValue.getValue());
84-
return context.addCloseable(logRecordProcessor);
78+
private static LogRecordProcessor createSimpleLogRecordProcessor(
79+
SimpleLogRecordProcessorModel simpleModel, DeclarativeConfigContext context) {
80+
LogRecordExporterModel exporterModel =
81+
FileConfigUtil.requireNonNull(
82+
simpleModel.getExporter(), "simple log record processor exporter");
83+
LogRecordExporter logRecordExporter =
84+
LogRecordExporterFactory.getInstance().create(exporterModel, context);
85+
MeterProvider meterProvider = context.getMeterProvider();
86+
return context.addCloseable(
87+
SimpleLogRecordProcessor.builder(logRecordExporter)
88+
.setMeterProvider(() -> meterProvider)
89+
.build());
8590
}
8691
}

0 commit comments

Comments
 (0)