diff --git a/all/src/test/java/io/opentelemetry/all/NullGuardsTest.java b/all/src/test/java/io/opentelemetry/all/NullGuardsTest.java index 6c9276773e9..04392368eb7 100644 --- a/all/src/test/java/io/opentelemetry/all/NullGuardsTest.java +++ b/all/src/test/java/io/opentelemetry/all/NullGuardsTest.java @@ -10,6 +10,7 @@ import io.opentelemetry.api.common.KeyValue; import io.opentelemetry.api.common.Value; import io.opentelemetry.common.impl.ApiUsageLogger; +import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import java.io.File; @@ -27,7 +28,14 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.jar.JarEntry; @@ -104,24 +112,81 @@ class NullGuardsTest { * fallbacks in runtime-defaulted artifacts. */ private static final Map METHOD_STYLE_OVERRIDES = - Map.of( - overrideKey(GlobalOpenTelemetry.class, "set", void.class, OpenTelemetry.class), - GuardStyle.FAIL_FAST, - overrideKey(GlobalOpenTelemetry.class, "set", void.class, Supplier.class), - GuardStyle.FAIL_FAST, - overrideKey( + Map.ofEntries( + Map.entry( + overrideKey(GlobalOpenTelemetry.class, "set", void.class, OpenTelemetry.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(GlobalOpenTelemetry.class, "set", void.class, Supplier.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey( OpenTelemetry.class, "propagating", OpenTelemetry.class, ContextPropagators.class), - GuardStyle.FAIL_FAST, + GuardStyle.FAIL_FAST), + // Context wrappers are setup-time adapters. Fail fast rather than defer an NPE until use + // or substitute a noop that could discard application work or return an invalid result. + Map.entry( + overrideKey(Context.class, "taskWrapping", Executor.class, Executor.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey( + Context.class, "taskWrapping", ExecutorService.class, ExecutorService.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey( + Context.class, + "taskWrapping", + ScheduledExecutorService.class, + ScheduledExecutorService.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrap", Executor.class, Executor.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrap", ExecutorService.class, ExecutorService.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey( + Context.class, + "wrap", + ScheduledExecutorService.class, + ScheduledExecutorService.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrap", Runnable.class, Runnable.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrap", Callable.class, Callable.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrapFunction", Function.class, Function.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrapFunction", BiFunction.class, BiFunction.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrapConsumer", Consumer.class, Consumer.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrapConsumer", BiConsumer.class, BiConsumer.class), + GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Context.class, "wrapSupplier", Supplier.class, Supplier.class), + GuardStyle.FAIL_FAST), // Value.empty() cannot preserve these generic return types. We may reevaluate in future. - overrideKey(Value.class, "of", Value.class, String.class), GuardStyle.FAIL_FAST, - overrideKey(Value.class, "of", Value.class, byte[].class), GuardStyle.FAIL_FAST, - overrideKey(Value.class, "of", Value.class, Value[].class), GuardStyle.FAIL_FAST, - overrideKey(Value.class, "of", Value.class, List.class), GuardStyle.FAIL_FAST, - overrideKey(Value.class, "of", Value.class, KeyValue[].class), GuardStyle.FAIL_FAST, - overrideKey(Value.class, "of", Value.class, Map.class), GuardStyle.FAIL_FAST); + Map.entry( + overrideKey(Value.class, "of", Value.class, String.class), GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Value.class, "of", Value.class, byte[].class), GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Value.class, "of", Value.class, Value[].class), GuardStyle.FAIL_FAST), + Map.entry(overrideKey(Value.class, "of", Value.class, List.class), GuardStyle.FAIL_FAST), + Map.entry( + overrideKey(Value.class, "of", Value.class, KeyValue[].class), GuardStyle.FAIL_FAST), + Map.entry(overrideKey(Value.class, "of", Value.class, Map.class), GuardStyle.FAIL_FAST)); private static String overrideKey( Class owner, String name, Class returnType, Class... paramTypes) { diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java b/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java index 5fca80f2403..9a0a1beb0c3 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/SpanId.java @@ -55,8 +55,11 @@ public static String getInvalid() { * @return {@code true} if the span identifier is valid. */ public static boolean isValid(CharSequence spanId) { - return spanId != null - && spanId.length() == HEX_LENGTH + if (spanId == null) { + ApiUsageLogger.logNullParam(SpanId.class, "isValid", "spanId"); + return false; + } + return spanId.length() == HEX_LENGTH && !INVALID.contentEquals(spanId) && OtelEncodingUtils.isValidBase16String(spanId); } @@ -72,9 +75,13 @@ public static boolean isValid(CharSequence spanId) { * @return the lowercase hex (base16) representation of the {@code SpanId}. */ public static String fromBytes(byte[] spanIdBytes) { - if (spanIdBytes == null || spanIdBytes.length < BYTES_LENGTH) { - ApiUsageLogger.logUsageIssue(SpanId.class, "fromBytes", "spanIdBytes is null or too short"); - return INVALID; + if (spanIdBytes == null) { + ApiUsageLogger.logNullParam(SpanId.class, "fromBytes", "spanIdBytes"); + return getInvalid(); + } + if (spanIdBytes.length < BYTES_LENGTH) { + ApiUsageLogger.logUsageIssue(SpanId.class, "fromBytes", "spanIdBytes is too short"); + return getInvalid(); } char[] result = TemporaryBuffers.chars(HEX_LENGTH); OtelEncodingUtils.bytesToBase16(spanIdBytes, result, BYTES_LENGTH); diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/TraceFlags.java b/api/all/src/main/java/io/opentelemetry/api/trace/TraceFlags.java index 85074130b6a..c45921aa1b2 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/TraceFlags.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/TraceFlags.java @@ -5,6 +5,7 @@ package io.opentelemetry.api.trace; +import io.opentelemetry.common.impl.ApiUsageLogger; import javax.annotation.concurrent.Immutable; /** @@ -54,6 +55,10 @@ static TraceFlags getSampled() { * @return the {@link TraceFlags} converted from the given lowercase hex (base16) representation. */ static TraceFlags fromHex(CharSequence src, int srcOffset) { + if (src == null) { + ApiUsageLogger.logNullParam(TraceFlags.class, "fromHex", "src"); + return getDefault(); + } return ImmutableTraceFlags.fromHex(src, srcOffset); } diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java b/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java index a14f0636a39..1ea304a8990 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/TraceId.java @@ -59,8 +59,11 @@ public static String getInvalid() { * @return {@code true} if the {@code TraceId} is valid. */ public static boolean isValid(CharSequence traceId) { - return traceId != null - && traceId.length() == HEX_LENGTH + if (traceId == null) { + ApiUsageLogger.logNullParam(TraceId.class, "isValid", "traceId"); + return false; + } + return traceId.length() == HEX_LENGTH && !INVALID.contentEquals(traceId) && OtelEncodingUtils.isValidBase16String(traceId); } @@ -76,8 +79,12 @@ public static boolean isValid(CharSequence traceId) { * @return the lowercase hex (base16) representation of the {@code TraceId}. */ public static String fromBytes(byte[] traceIdBytes) { - if (traceIdBytes == null || traceIdBytes.length < BYTES_LENGTH) { - ApiUsageLogger.logUsageIssue(TraceId.class, "fromBytes", "traceIdBytes is null or too short"); + if (traceIdBytes == null) { + ApiUsageLogger.logNullParam(TraceId.class, "fromBytes", "traceIdBytes"); + return INVALID; + } + if (traceIdBytes.length < BYTES_LENGTH) { + ApiUsageLogger.logUsageIssue(TraceId.class, "fromBytes", "traceIdBytes is too short"); return INVALID; } char[] result = TemporaryBuffers.chars(HEX_LENGTH); diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/TracerProvider.java b/api/all/src/main/java/io/opentelemetry/api/trace/TracerProvider.java index 2ef9d12acfe..1c3dbb80407 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/TracerProvider.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/TracerProvider.java @@ -5,6 +5,7 @@ package io.opentelemetry.api.trace; +import io.opentelemetry.common.impl.ApiUsageLogger; import javax.annotation.concurrent.ThreadSafe; /** @@ -52,6 +53,11 @@ static TracerProvider noop() { * @since 1.4.0 */ default TracerBuilder tracerBuilder(String instrumentationScopeName) { + if (instrumentationScopeName == null) { + ApiUsageLogger.logNullParam( + TracerProvider.class, "tracerBuilder", "instrumentationScopeName"); + return DefaultTracerBuilder.getInstance(); + } return DefaultTracerBuilder.getInstance(); } } diff --git a/api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java b/api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java index efcd2476b2c..c7ab74be3ba 100644 --- a/api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java +++ b/api/all/src/main/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagator.java @@ -16,6 +16,7 @@ import io.opentelemetry.api.trace.TraceFlags; import io.opentelemetry.api.trace.TraceId; import io.opentelemetry.api.trace.TraceState; +import io.opentelemetry.common.impl.ApiUsageLogger; import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; @@ -94,7 +95,12 @@ public Collection fields() { @Override public void inject(Context context, @Nullable C carrier, TextMapSetter setter) { - if (context == null || setter == null) { + if (context == null) { + ApiUsageLogger.logNullParam(TextMapPropagator.class, "inject", "context"); + return; + } + if (setter == null) { + ApiUsageLogger.logNullParam(TextMapPropagator.class, "inject", "setter"); return; } @@ -132,9 +138,11 @@ public void inject(Context context, @Nullable C carrier, TextMapSetter se @Override public Context extract(Context context, @Nullable C carrier, TextMapGetter getter) { if (context == null) { + ApiUsageLogger.logNullParam(TextMapPropagator.class, "extract", "context"); return Context.root(); } if (getter == null) { + ApiUsageLogger.logNullParam(TextMapPropagator.class, "extract", "getter"); return context; } diff --git a/context/src/main/java/io/opentelemetry/context/ArrayBasedContext.java b/context/src/main/java/io/opentelemetry/context/ArrayBasedContext.java index 53f261caedb..d4274c60c8d 100644 --- a/context/src/main/java/io/opentelemetry/context/ArrayBasedContext.java +++ b/context/src/main/java/io/opentelemetry/context/ArrayBasedContext.java @@ -22,6 +22,7 @@ package io.opentelemetry.context; +import io.opentelemetry.common.impl.ApiUsageLogger; import java.util.Arrays; import javax.annotation.Nullable; @@ -55,6 +56,10 @@ private ArrayBasedContext(Object[] entries) { @Nullable @SuppressWarnings("ReferenceEquality") public V get(ContextKey key) { + if (key == null) { + ApiUsageLogger.logNullParam(Context.class, "get", "key"); + return null; + } for (int i = 0; i < entries.length; i += 2) { if (entries[i] == key) { @SuppressWarnings("unchecked") @@ -68,6 +73,14 @@ public V get(ContextKey key) { @Override @SuppressWarnings("ReferenceEquality") public Context with(ContextKey key, V value) { + if (key == null) { + ApiUsageLogger.logNullParam(Context.class, "with", "key"); + return this; + } + if (value == null) { + ApiUsageLogger.logNullParam(Context.class, "with", "value"); + // Continue to preserve the preexisting behavior of storing null values. + } for (int i = 0; i < entries.length; i += 2) { if (entries[i] == key) { if (entries[i + 1] == value) { diff --git a/context/src/main/java/io/opentelemetry/context/Context.java b/context/src/main/java/io/opentelemetry/context/Context.java index 5712c04b4a9..8eec2765928 100644 --- a/context/src/main/java/io/opentelemetry/context/Context.java +++ b/context/src/main/java/io/opentelemetry/context/Context.java @@ -23,6 +23,8 @@ package io.opentelemetry.context; import com.google.errorprone.annotations.MustBeClosed; +import io.opentelemetry.common.impl.ApiUsageLogger; +import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; @@ -119,6 +121,7 @@ static Context root() { * @since 1.1.0 */ static Executor taskWrapping(Executor executor) { + Objects.requireNonNull(executor, "executor"); return command -> executor.execute(Context.current().wrap(command)); } @@ -137,6 +140,7 @@ static Executor taskWrapping(Executor executor) { * @since 1.1.0 */ static ExecutorService taskWrapping(ExecutorService executorService) { + Objects.requireNonNull(executorService, "executorService"); if (executorService instanceof CurrentContextExecutorService) { return executorService; } @@ -162,6 +166,7 @@ static ExecutorService taskWrapping(ExecutorService executorService) { * @since 1.43.0 */ static ScheduledExecutorService taskWrapping(ScheduledExecutorService executorService) { + Objects.requireNonNull(executorService, "executorService"); if (executorService instanceof CurrentContextScheduledExecutorService) { return executorService; } @@ -201,6 +206,10 @@ static ScheduledExecutorService taskWrapping(ScheduledExecutorService executorSe /** Returns a new {@link Context} with the given {@link ImplicitContextKeyed} set. */ default Context with(ImplicitContextKeyed value) { + if (value == null) { + ApiUsageLogger.logNullParam(Context.class, "with", "value"); + return this; + } return value.storeInContext(this); } @@ -237,6 +246,7 @@ default Scope makeCurrent() { * and then invokes the input {@link Runnable}. */ default Runnable wrap(Runnable runnable) { + Objects.requireNonNull(runnable, "runnable"); return () -> { try (Scope ignored = makeCurrent()) { runnable.run(); @@ -249,6 +259,7 @@ default Runnable wrap(Runnable runnable) { * and then invokes the input {@link Callable}. */ default Callable wrap(Callable callable) { + Objects.requireNonNull(callable, "callable"); return () -> { try (Scope ignored = makeCurrent()) { return callable.call(); @@ -261,6 +272,7 @@ default Callable wrap(Callable callable) { * this the {@linkplain Context#current() current context} before each execution. */ default Executor wrap(Executor executor) { + Objects.requireNonNull(executor, "executor"); return command -> executor.execute(wrap(command)); } @@ -269,6 +281,7 @@ default Executor wrap(Executor executor) { * making this the {@linkplain Context#current() current context} before each execution. */ default ExecutorService wrap(ExecutorService executor) { + Objects.requireNonNull(executor, "executor"); if (executor instanceof ContextExecutorService) { return executor; } @@ -281,6 +294,7 @@ default ExecutorService wrap(ExecutorService executor) { * execution. */ default ScheduledExecutorService wrap(ScheduledExecutorService executor) { + Objects.requireNonNull(executor, "executor"); if (executor instanceof ContextScheduledExecutorService) { return executor; } @@ -292,6 +306,7 @@ default ScheduledExecutorService wrap(ScheduledExecutorService executor) { * and then invokes the input {@link Function}. */ default Function wrapFunction(Function function) { + Objects.requireNonNull(function, "function"); return t -> { try (Scope ignored = makeCurrent()) { return function.apply(t); @@ -304,6 +319,7 @@ default Function wrapFunction(Function function) { * and then invokes the input {@link BiFunction}. */ default BiFunction wrapFunction(BiFunction function) { + Objects.requireNonNull(function, "function"); return (t, u) -> { try (Scope ignored = makeCurrent()) { return function.apply(t, u); @@ -316,6 +332,7 @@ default BiFunction wrapFunction(BiFunction function) * and then invokes the input {@link Consumer}. */ default Consumer wrapConsumer(Consumer consumer) { + Objects.requireNonNull(consumer, "consumer"); return t -> { try (Scope ignored = makeCurrent()) { consumer.accept(t); @@ -328,6 +345,7 @@ default Consumer wrapConsumer(Consumer consumer) { * and then invokes the input {@link BiConsumer}. */ default BiConsumer wrapConsumer(BiConsumer consumer) { + Objects.requireNonNull(consumer, "consumer"); return (t, u) -> { try (Scope ignored = makeCurrent()) { consumer.accept(t, u); @@ -340,6 +358,7 @@ default BiConsumer wrapConsumer(BiConsumer consumer) { * and then invokes the input {@link Supplier}. */ default Supplier wrapSupplier(Supplier supplier) { + Objects.requireNonNull(supplier, "supplier"); return () -> { try (Scope ignored = makeCurrent()) { return supplier.get(); diff --git a/context/src/main/java/io/opentelemetry/context/ContextKey.java b/context/src/main/java/io/opentelemetry/context/ContextKey.java index 7357e515aeb..b7fbb179fb8 100644 --- a/context/src/main/java/io/opentelemetry/context/ContextKey.java +++ b/context/src/main/java/io/opentelemetry/context/ContextKey.java @@ -5,6 +5,8 @@ package io.opentelemetry.context; +import io.opentelemetry.common.impl.ApiUsageLogger; + /** * Key for indexing values of type {@code T} stored in a {@link Context}. {@link ContextKey} are * compared by reference, so it is expected that only one {@link ContextKey} is created for a @@ -39,6 +41,10 @@ public interface ContextKey { * keys. */ static ContextKey named(String name) { + if (name == null) { + ApiUsageLogger.logNullParam(ContextKey.class, "named", "name"); + return new DefaultContextKey<>(""); + } return new DefaultContextKey<>(name); } } diff --git a/context/src/main/java/io/opentelemetry/context/ContextStorage.java b/context/src/main/java/io/opentelemetry/context/ContextStorage.java index c68b5c9c65a..62458015c28 100644 --- a/context/src/main/java/io/opentelemetry/context/ContextStorage.java +++ b/context/src/main/java/io/opentelemetry/context/ContextStorage.java @@ -22,6 +22,7 @@ package io.opentelemetry.context; +import io.opentelemetry.common.impl.ApiUsageLogger; import java.util.function.Function; import javax.annotation.Nullable; @@ -86,6 +87,10 @@ static ContextStorage defaultStorage() { * main class. */ static void addWrapper(Function wrapper) { + if (wrapper == null) { + ApiUsageLogger.logNullParam(ContextStorage.class, "addWrapper", "wrapper"); + return; + } ContextStorageWrappers.addWrapper(wrapper); } diff --git a/context/src/main/java/io/opentelemetry/context/StrictContextStorage.java b/context/src/main/java/io/opentelemetry/context/StrictContextStorage.java index ac4731dc101..1d2d2c46866 100644 --- a/context/src/main/java/io/opentelemetry/context/StrictContextStorage.java +++ b/context/src/main/java/io/opentelemetry/context/StrictContextStorage.java @@ -22,6 +22,7 @@ import static java.lang.Thread.currentThread; +import io.opentelemetry.common.impl.ApiUsageLogger; import io.opentelemetry.context.internal.shaded.WeakConcurrentMap; import java.lang.ref.Reference; import java.util.Arrays; @@ -63,6 +64,11 @@ private StrictContextStorage(ContextStorage delegate) { @Override public Scope attach(Context context) { + if (context == null) { + ApiUsageLogger.logNullParam(ContextStorage.class, "attach", "context"); + return Scope.noop(); + } + Scope scope = delegate.attach(context); CallerStackTrace caller = new CallerStackTrace(context);