Skip to content

Commit 745a45f

Browse files
zeitlingerzeitlinger
committed
fix NullPointerException with declarative config
1 parent 12dac23 commit 745a45f

File tree

2 files changed

+133
-1
lines changed

2 files changed

+133
-1
lines changed

‎instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/CommonConfig.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package io.opentelemetry.instrumentation.api.incubator.config.internal;
77

8+
import static java.util.Collections.emptyList;
89
import static java.util.Collections.emptyMap;
910

1011
import io.opentelemetry.api.incubator.config.ConfigProvider;
@@ -46,28 +47,33 @@ public CommonConfig(InstrumentationConfig config) {
4647
getFromConfigProviderOrFallback(
4748
config,
4849
InstrumentationConfigUtil::peerServiceMapping,
50+
emptyMap(),
4951
() ->
5052
config.getMap("otel.instrumentation.common.peer-service-mapping", emptyMap())));
5153

5254
clientRequestHeaders =
5355
getFromConfigProviderOrFallback(
5456
config,
5557
InstrumentationConfigUtil::httpClientRequestCapturedHeaders,
58+
emptyList(),
5659
() -> config.getList("otel.instrumentation.http.client.capture-request-headers"));
5760
clientResponseHeaders =
5861
getFromConfigProviderOrFallback(
5962
config,
6063
InstrumentationConfigUtil::httpClientResponseCapturedHeaders,
64+
emptyList(),
6165
() -> config.getList("otel.instrumentation.http.client.capture-response-headers"));
6266
serverRequestHeaders =
6367
getFromConfigProviderOrFallback(
6468
config,
6569
InstrumentationConfigUtil::httpServerRequestCapturedHeaders,
70+
emptyList(),
6671
() -> config.getList("otel.instrumentation.http.server.capture-request-headers"));
6772
serverResponseHeaders =
6873
getFromConfigProviderOrFallback(
6974
config,
7075
InstrumentationConfigUtil::httpServerResponseCapturedHeaders,
76+
emptyList(),
7177
() -> config.getList("otel.instrumentation.http.server.capture-response-headers"));
7278
knownHttpRequestMethods =
7379
new HashSet<>(
@@ -154,11 +160,14 @@ public String getTraceFlagsKey() {
154160
private static <T> T getFromConfigProviderOrFallback(
155161
InstrumentationConfig config,
156162
Function<ConfigProvider, T> getFromConfigProvider,
163+
T defaultValue,
157164
Supplier<T> fallback) {
158165
ConfigProvider configProvider = config.getConfigProvider();
159166
if (configProvider != null) {
160-
return getFromConfigProvider.apply(configProvider);
167+
T value = getFromConfigProvider.apply(configProvider);
168+
return value != null ? value : defaultValue;
161169
}
170+
// fallback doesn't return null, so we can safely call it
162171
return fallback.get();
163172
}
164173
}

‎javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/config/ConfigPropertiesBridgeTest.java‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package io.opentelemetry.javaagent.tooling.config;
2+
3+
import static java.util.Arrays.asList;
4+
import static java.util.Collections.emptyMap;
5+
import static org.assertj.core.api.Assertions.assertThat;
6+
7+
import io.opentelemetry.instrumentation.api.incubator.config.internal.CommonConfig;
8+
import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
9+
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
10+
import io.opentelemetry.sdk.extension.incubator.fileconfig.SdkConfigProvider;
11+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ClientModel;
12+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalGeneralInstrumentationModel;
13+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalHttpInstrumentationModel;
14+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalPeerInstrumentationModel;
15+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.InstrumentationModel;
16+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel;
17+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ServerModel;
18+
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ServiceMappingModel;
19+
import java.util.HashMap;
20+
import java.util.Map;
21+
import java.util.stream.Stream;
22+
import org.assertj.core.api.InstanceOfAssertFactories;
23+
import org.junit.jupiter.params.ParameterizedTest;
24+
import org.junit.jupiter.params.provider.Arguments;
25+
import org.junit.jupiter.params.provider.MethodSource;
26+
27+
/**
28+
* common config that is read from declarative config or config properties in a different way
29+
*
30+
* <p>cannot test CommonConfig in its own module, because there is no implementation of
31+
* InstrumentationConfig in that module
32+
*/
33+
class ConfigPropertiesBridgeTest {
34+
35+
public static Stream<Arguments> emptyGeneralConfig() {
36+
OpenTelemetryConfigurationModel emptyModel =
37+
new OpenTelemetryConfigurationModel()
38+
.withAdditionalProperty("instrumentation/development", new InstrumentationModel());
39+
40+
DefaultConfigProperties configProperties = DefaultConfigProperties.createFromMap(emptyMap());
41+
42+
return Stream.of(
43+
Arguments.of("config properties", new ConfigPropertiesBridge(configProperties, null)),
44+
Arguments.of(
45+
"declarative config",
46+
new ConfigPropertiesBridge(configProperties, SdkConfigProvider.create(emptyModel))));
47+
}
48+
49+
@ParameterizedTest(name = "{0}")
50+
@MethodSource("emptyGeneralConfig")
51+
void testEmptyGeneralConfig(String name, InstrumentationConfig config) {
52+
CommonConfig commonConfig = new CommonConfig(config);
53+
assertThat(commonConfig.getPeerServiceResolver()).extracting("mapping").isEqualTo(emptyMap());
54+
assertThat(commonConfig.getClientRequestHeaders()).isEmpty();
55+
assertThat(commonConfig.getServerRequestHeaders()).isEmpty();
56+
assertThat(commonConfig.getClientResponseHeaders()).isEmpty();
57+
assertThat(commonConfig.getServerResponseHeaders()).isEmpty();
58+
}
59+
60+
public static Stream<Arguments> fullGeneralConfig() {
61+
InstrumentationModel model =
62+
new InstrumentationModel()
63+
.withGeneral(
64+
new ExperimentalGeneralInstrumentationModel()
65+
.withPeer(
66+
new ExperimentalPeerInstrumentationModel()
67+
.withServiceMapping(
68+
asList(
69+
new ServiceMappingModel()
70+
.withService("cats-service")
71+
.withPeer("1.2.3.4"),
72+
new ServiceMappingModel()
73+
.withService("dogs-api")
74+
.withPeer("dogs-abcdef123.serverlessapis.com"))))
75+
.withHttp(
76+
new ExperimentalHttpInstrumentationModel()
77+
.withClient(
78+
new ClientModel()
79+
.withRequestCapturedHeaders(asList("header1", "header2"))
80+
.withResponseCapturedHeaders(asList("header3", "header4")))
81+
.withServer(
82+
new ServerModel()
83+
.withRequestCapturedHeaders(asList("header5", "header6"))
84+
.withResponseCapturedHeaders(asList("header7", "header8")))));
85+
OpenTelemetryConfigurationModel emptyModel =
86+
new OpenTelemetryConfigurationModel()
87+
.withAdditionalProperty("instrumentation/development", model);
88+
89+
DefaultConfigProperties configProperties =
90+
DefaultConfigProperties.createFromMap(getProperties());
91+
92+
return Stream.of(
93+
Arguments.of("config properties", new ConfigPropertiesBridge(configProperties, null)),
94+
Arguments.of(
95+
"declarative config",
96+
new ConfigPropertiesBridge(configProperties, SdkConfigProvider.create(emptyModel))));
97+
}
98+
99+
private static Map<String, String> getProperties() {
100+
Map<String, String> properties = new HashMap<>();
101+
properties.put(
102+
"otel.instrumentation.common.peer-service-mapping",
103+
"1.2.3.4=cats-service,dogs-abcdef123.serverlessapis.com=dogs-api");
104+
properties.put("otel.instrumentation.http.client.capture-request-headers", "header1,header2");
105+
properties.put("otel.instrumentation.http.client.capture-response-headers", "header3,header4");
106+
properties.put("otel.instrumentation.http.server.capture-request-headers", "header5,header6");
107+
properties.put("otel.instrumentation.http.server.capture-response-headers", "header7,header8");
108+
return properties;
109+
}
110+
111+
@ParameterizedTest(name = "{0}")
112+
@MethodSource("fullGeneralConfig")
113+
void testFullGeneralConfig(String name, InstrumentationConfig config) {
114+
CommonConfig commonConfig = new CommonConfig(config);
115+
assertThat(commonConfig.getPeerServiceResolver())
116+
.extracting("mapping", InstanceOfAssertFactories.MAP)
117+
.containsOnlyKeys("1.2.3.4", "dogs-abcdef123.serverlessapis.com");
118+
assertThat(commonConfig.getClientRequestHeaders()).containsExactly("header1", "header2");
119+
assertThat(commonConfig.getClientResponseHeaders()).containsExactly("header3", "header4");
120+
assertThat(commonConfig.getServerRequestHeaders()).containsExactly("header5", "header6");
121+
assertThat(commonConfig.getServerResponseHeaders()).containsExactly("header7", "header8");
122+
}
123+
}

0 commit comments

Comments
 (0)