Skip to content

Commit 32a31e2

Browse files
zeitlingerotelbot[bot]
zeitlinger
and
otelbot[bot]
authored
fix NullPointerException with declarative config (#14440)
Co-authored-by: otelbot <[email protected]>
1 parent 394d053 commit 32a31e2

File tree

2 files changed

+138
-1
lines changed

2 files changed

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

0 commit comments

Comments
 (0)