Skip to content

Commit d97e4af

Browse files
jack-bergjack-berg
authored
Add null guards 4 (#8968)
1 parent b36245e commit d97e4af

11 files changed

Lines changed: 170 additions & 23 deletions

File tree

‎all/src/test/java/io/opentelemetry/all/NullGuardsTest.java‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import io.opentelemetry.api.common.KeyValue;
1111
import io.opentelemetry.api.common.Value;
1212
import io.opentelemetry.common.impl.ApiUsageLogger;
13+
import io.opentelemetry.context.Context;
1314
import io.opentelemetry.context.propagation.ContextPropagators;
1415
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
1516
import java.io.File;
@@ -43,7 +44,14 @@
4344
import java.util.Map;
4445
import java.util.Objects;
4546
import java.util.Set;
47+
import java.util.concurrent.Callable;
48+
import java.util.concurrent.Executor;
49+
import java.util.concurrent.ExecutorService;
50+
import java.util.concurrent.ScheduledExecutorService;
51+
import java.util.function.BiConsumer;
52+
import java.util.function.BiFunction;
4653
import java.util.function.Consumer;
54+
import java.util.function.Function;
4755
import java.util.function.Predicate;
4856
import java.util.function.Supplier;
4957
import java.util.jar.JarEntry;
@@ -111,24 +119,81 @@ class NullGuardsTest {
111119
* fallbacks in runtime-defaulted artifacts.
112120
*/
113121
private static final Map<String, GuardStyle> METHOD_STYLE_OVERRIDES =
114-
Map.of(
115-
overrideKey(GlobalOpenTelemetry.class, "set", void.class, OpenTelemetry.class),
116-
GuardStyle.FAIL_FAST,
117-
overrideKey(GlobalOpenTelemetry.class, "set", void.class, Supplier.class),
118-
GuardStyle.FAIL_FAST,
119-
overrideKey(
122+
Map.ofEntries(
123+
Map.entry(
124+
overrideKey(GlobalOpenTelemetry.class, "set", void.class, OpenTelemetry.class),
125+
GuardStyle.FAIL_FAST),
126+
Map.entry(
127+
overrideKey(GlobalOpenTelemetry.class, "set", void.class, Supplier.class),
128+
GuardStyle.FAIL_FAST),
129+
Map.entry(
130+
overrideKey(
120131
OpenTelemetry.class,
121132
"propagating",
122133
OpenTelemetry.class,
123134
ContextPropagators.class),
124-
GuardStyle.FAIL_FAST,
135+
GuardStyle.FAIL_FAST),
136+
// Context wrappers are setup-time adapters. Fail fast rather than defer an NPE until use
137+
// or substitute a noop that could discard application work or return an invalid result.
138+
Map.entry(
139+
overrideKey(Context.class, "taskWrapping", Executor.class, Executor.class),
140+
GuardStyle.FAIL_FAST),
141+
Map.entry(
142+
overrideKey(
143+
Context.class, "taskWrapping", ExecutorService.class, ExecutorService.class),
144+
GuardStyle.FAIL_FAST),
145+
Map.entry(
146+
overrideKey(
147+
Context.class,
148+
"taskWrapping",
149+
ScheduledExecutorService.class,
150+
ScheduledExecutorService.class),
151+
GuardStyle.FAIL_FAST),
152+
Map.entry(
153+
overrideKey(Context.class, "wrap", Executor.class, Executor.class),
154+
GuardStyle.FAIL_FAST),
155+
Map.entry(
156+
overrideKey(Context.class, "wrap", ExecutorService.class, ExecutorService.class),
157+
GuardStyle.FAIL_FAST),
158+
Map.entry(
159+
overrideKey(
160+
Context.class,
161+
"wrap",
162+
ScheduledExecutorService.class,
163+
ScheduledExecutorService.class),
164+
GuardStyle.FAIL_FAST),
165+
Map.entry(
166+
overrideKey(Context.class, "wrap", Runnable.class, Runnable.class),
167+
GuardStyle.FAIL_FAST),
168+
Map.entry(
169+
overrideKey(Context.class, "wrap", Callable.class, Callable.class),
170+
GuardStyle.FAIL_FAST),
171+
Map.entry(
172+
overrideKey(Context.class, "wrapFunction", Function.class, Function.class),
173+
GuardStyle.FAIL_FAST),
174+
Map.entry(
175+
overrideKey(Context.class, "wrapFunction", BiFunction.class, BiFunction.class),
176+
GuardStyle.FAIL_FAST),
177+
Map.entry(
178+
overrideKey(Context.class, "wrapConsumer", Consumer.class, Consumer.class),
179+
GuardStyle.FAIL_FAST),
180+
Map.entry(
181+
overrideKey(Context.class, "wrapConsumer", BiConsumer.class, BiConsumer.class),
182+
GuardStyle.FAIL_FAST),
183+
Map.entry(
184+
overrideKey(Context.class, "wrapSupplier", Supplier.class, Supplier.class),
185+
GuardStyle.FAIL_FAST),
125186
// Value.empty() cannot preserve these generic return types. We may reevaluate in future.
126-
overrideKey(Value.class, "of", Value.class, String.class), GuardStyle.FAIL_FAST,
127-
overrideKey(Value.class, "of", Value.class, byte[].class), GuardStyle.FAIL_FAST,
128-
overrideKey(Value.class, "of", Value.class, Value[].class), GuardStyle.FAIL_FAST,
129-
overrideKey(Value.class, "of", Value.class, List.class), GuardStyle.FAIL_FAST,
130-
overrideKey(Value.class, "of", Value.class, KeyValue[].class), GuardStyle.FAIL_FAST,
131-
overrideKey(Value.class, "of", Value.class, Map.class), GuardStyle.FAIL_FAST);
187+
Map.entry(
188+
overrideKey(Value.class, "of", Value.class, String.class), GuardStyle.FAIL_FAST),
189+
Map.entry(
190+
overrideKey(Value.class, "of", Value.class, byte[].class), GuardStyle.FAIL_FAST),
191+
Map.entry(
192+
overrideKey(Value.class, "of", Value.class, Value[].class), GuardStyle.FAIL_FAST),
193+
Map.entry(overrideKey(Value.class, "of", Value.class, List.class), GuardStyle.FAIL_FAST),
194+
Map.entry(
195+
overrideKey(Value.class, "of", Value.class, KeyValue[].class), GuardStyle.FAIL_FAST),
196+
Map.entry(overrideKey(Value.class, "of", Value.class, Map.class), GuardStyle.FAIL_FAST));
132197

133198
private static String overrideKey(
134199
Class<?> owner, String name, Class<?> returnType, Class<?>... paramTypes) {

‎api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,11 @@ public static String getInvalid() {
5555
* @return {@code true} if the span identifier is valid.
5656
*/
5757
public static boolean isValid(CharSequence spanId) {
58-
return spanId != null
59-
&& spanId.length() == HEX_LENGTH
58+
if (spanId == null) {
59+
ApiUsageLogger.logNullParam(SpanId.class, "isValid", "spanId");
60+
return false;
61+
}
62+
return spanId.length() == HEX_LENGTH
6063
&& !INVALID.contentEquals(spanId)
6164
&& OtelEncodingUtils.isValidBase16String(spanId);
6265
}
@@ -72,9 +75,13 @@ public static boolean isValid(CharSequence spanId) {
7275
* @return the lowercase hex (base16) representation of the {@code SpanId}.
7376
*/
7477
public static String fromBytes(byte[] spanIdBytes) {
75-
if (spanIdBytes == null || spanIdBytes.length < BYTES_LENGTH) {
76-
ApiUsageLogger.logUsageIssue(SpanId.class, "fromBytes", "spanIdBytes is null or too short");
77-
return INVALID;
78+
if (spanIdBytes == null) {
79+
ApiUsageLogger.logNullParam(SpanId.class, "fromBytes", "spanIdBytes");
80+
return getInvalid();
81+
}
82+
if (spanIdBytes.length < BYTES_LENGTH) {
83+
ApiUsageLogger.logUsageIssue(SpanId.class, "fromBytes", "spanIdBytes is too short");
84+
return getInvalid();
7885
}
7986
char[] result = TemporaryBuffers.chars(HEX_LENGTH);
8087
OtelEncodingUtils.bytesToBase16(spanIdBytes, result, BYTES_LENGTH);

‎api/all/src/main/java/io/opentelemetry/api/trace/TraceFlags.java‎

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

66
package io.opentelemetry.api.trace;
77

8+
import io.opentelemetry.common.impl.ApiUsageLogger;
89
import javax.annotation.concurrent.Immutable;
910

1011
/**
@@ -54,6 +55,10 @@ static TraceFlags getSampled() {
5455
* @return the {@link TraceFlags} converted from the given lowercase hex (base16) representation.
5556
*/
5657
static TraceFlags fromHex(CharSequence src, int srcOffset) {
58+
if (src == null) {
59+
ApiUsageLogger.logNullParam(TraceFlags.class, "fromHex", "src");
60+
return getDefault();
61+
}
5762
return ImmutableTraceFlags.fromHex(src, srcOffset);
5863
}
5964

‎api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,11 @@ public static String getInvalid() {
5959
* @return {@code true} if the {@code TraceId} is valid.
6060
*/
6161
public static boolean isValid(CharSequence traceId) {
62-
return traceId != null
63-
&& traceId.length() == HEX_LENGTH
62+
if (traceId == null) {
63+
ApiUsageLogger.logNullParam(TraceId.class, "isValid", "traceId");
64+
return false;
65+
}
66+
return traceId.length() == HEX_LENGTH
6467
&& !INVALID.contentEquals(traceId)
6568
&& OtelEncodingUtils.isValidBase16String(traceId);
6669
}
@@ -76,8 +79,12 @@ public static boolean isValid(CharSequence traceId) {
7679
* @return the lowercase hex (base16) representation of the {@code TraceId}.
7780
*/
7881
public static String fromBytes(byte[] traceIdBytes) {
79-
if (traceIdBytes == null || traceIdBytes.length < BYTES_LENGTH) {
80-
ApiUsageLogger.logUsageIssue(TraceId.class, "fromBytes", "traceIdBytes is null or too short");
82+
if (traceIdBytes == null) {
83+
ApiUsageLogger.logNullParam(TraceId.class, "fromBytes", "traceIdBytes");
84+
return INVALID;
85+
}
86+
if (traceIdBytes.length < BYTES_LENGTH) {
87+
ApiUsageLogger.logUsageIssue(TraceId.class, "fromBytes", "traceIdBytes is too short");
8188
return INVALID;
8289
}
8390
char[] result = TemporaryBuffers.chars(HEX_LENGTH);

‎api/all/src/main/java/io/opentelemetry/api/trace/TracerProvider.java‎

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

66
package io.opentelemetry.api.trace;
77

8+
import io.opentelemetry.common.impl.ApiUsageLogger;
89
import javax.annotation.concurrent.ThreadSafe;
910

1011
/**
@@ -52,6 +53,11 @@ static TracerProvider noop() {
5253
* @since 1.4.0
5354
*/
5455
default TracerBuilder tracerBuilder(String instrumentationScopeName) {
56+
if (instrumentationScopeName == null) {
57+
ApiUsageLogger.logNullParam(
58+
TracerProvider.class, "tracerBuilder", "instrumentationScopeName");
59+
return DefaultTracerBuilder.getInstance();
60+
}
5561
return DefaultTracerBuilder.getInstance();
5662
}
5763
}

‎api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import io.opentelemetry.api.trace.TraceFlags;
1717
import io.opentelemetry.api.trace.TraceId;
1818
import io.opentelemetry.api.trace.TraceState;
19+
import io.opentelemetry.common.impl.ApiUsageLogger;
1920
import io.opentelemetry.context.Context;
2021
import io.opentelemetry.context.propagation.TextMapGetter;
2122
import io.opentelemetry.context.propagation.TextMapPropagator;
@@ -94,7 +95,12 @@ public Collection<String> fields() {
9495

9596
@Override
9697
public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
97-
if (context == null || setter == null) {
98+
if (context == null) {
99+
ApiUsageLogger.logNullParam(TextMapPropagator.class, "inject", "context");
100+
return;
101+
}
102+
if (setter == null) {
103+
ApiUsageLogger.logNullParam(TextMapPropagator.class, "inject", "setter");
98104
return;
99105
}
100106

@@ -132,9 +138,11 @@ public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> se
132138
@Override
133139
public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
134140
if (context == null) {
141+
ApiUsageLogger.logNullParam(TextMapPropagator.class, "extract", "context");
135142
return Context.root();
136143
}
137144
if (getter == null) {
145+
ApiUsageLogger.logNullParam(TextMapPropagator.class, "extract", "getter");
138146
return context;
139147
}
140148

‎context/src/main/java/io/opentelemetry/context/ArrayBasedContext.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
package io.opentelemetry.context;
2424

25+
import io.opentelemetry.common.impl.ApiUsageLogger;
2526
import java.util.Arrays;
2627
import javax.annotation.Nullable;
2728

@@ -55,6 +56,10 @@ private ArrayBasedContext(Object[] entries) {
5556
@Nullable
5657
@SuppressWarnings("ReferenceEquality")
5758
public <V> V get(ContextKey<V> key) {
59+
if (key == null) {
60+
ApiUsageLogger.logNullParam(Context.class, "get", "key");
61+
return null;
62+
}
5863
for (int i = 0; i < entries.length; i += 2) {
5964
if (entries[i] == key) {
6065
@SuppressWarnings("unchecked")
@@ -68,6 +73,14 @@ public <V> V get(ContextKey<V> key) {
6873
@Override
6974
@SuppressWarnings("ReferenceEquality")
7075
public <V> Context with(ContextKey<V> key, V value) {
76+
if (key == null) {
77+
ApiUsageLogger.logNullParam(Context.class, "with", "key");
78+
return this;
79+
}
80+
if (value == null) {
81+
ApiUsageLogger.logNullParam(Context.class, "with", "value");
82+
// Continue to preserve the preexisting behavior of storing null values.
83+
}
7184
for (int i = 0; i < entries.length; i += 2) {
7285
if (entries[i] == key) {
7386
if (entries[i + 1] == value) {

0 commit comments

Comments
 (0)