Don't throw on null in propagator implementations. (#2904)

* Don't throw on null in propagator implementations. * Baggage too
open-telemetry · Feb 24, 2021 · 18df74e · 18df74e
1 parent b393a58
commit 18df74e
Show file tree

Hide file tree

Showing 17 changed files with 301 additions and 41 deletions.
diff --git a/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java b/api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java
@@ -41,6 +41,9 @@ public Collection<String> fields() {
 
   @Override
   public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
+    if (context == null || setter == null) {
+      return;
+    }
     Baggage baggage = Baggage.fromContext(context);
     if (baggage.isEmpty()) {
       return;
@@ -63,6 +66,13 @@ public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
 
   @Override
   public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
+    if (context == null) {
+      return Context.root();
+    }
+    if (getter == null) {
+      return context;
+    }
+
     String baggageHeader = getter.get(carrier, FIELD);
     if (baggageHeader == null) {
       return context;

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
@@ -23,7 +23,6 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Objects;
 import java.util.Set;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
@@ -100,8 +99,9 @@ public Collection<String> fields() {
 
   @Override
   public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
-    Objects.requireNonNull(context, "context");
-    Objects.requireNonNull(setter, "setter");
+    if (context == null || setter == null) {
+      return;
+    }
 
     SpanContext spanContext = Span.fromContext(context).getSpanContext();
     if (!spanContext.isValid()) {
@@ -148,8 +148,12 @@ public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> se
 
   @Override
   public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
-    Objects.requireNonNull(context, "context");
-    Objects.requireNonNull(getter, "getter");
+    if (context == null) {
+      return Context.root();
+    }
+    if (getter == null) {
+      return context;
+    }
 
     SpanContext spanContext = extractImpl(carrier, getter);
     if (!spanContext.isValid()) {

diff --git a/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java b/api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java
@@ -13,7 +13,9 @@
 import io.opentelemetry.api.baggage.BaggageEntryMetadata;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
@@ -154,6 +156,19 @@ void extract_invalidHeader() {
     assertThat(Baggage.fromContext(result)).isEqualTo(Baggage.empty());
   }
 
+  @Test
+  void extract_nullContext() {
+    assertThat(W3CBaggagePropagator.getInstance().extract(null, Collections.emptyMap(), getter))
+        .isSameAs(Context.root());
+  }
+
+  @Test
+  void extract_nullGetter() {
+    Context context = Context.current().with(Baggage.builder().put("cat", "meow").build());
+    assertThat(W3CBaggagePropagator.getInstance().extract(context, Collections.emptyMap(), null))
+        .isSameAs(context);
+  }
+
   @Test
   void inject_noBaggage() {
     W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();
@@ -186,4 +201,19 @@ void inject() {
             singletonMap(
                 "baggage", "meta=meta-value;somemetadata; someother=foo,nometa=nometa-value"));
   }
+
+  @Test
+  void inject_nullContext() {
+    Map<String, String> carrier = new LinkedHashMap<>();
+    W3CBaggagePropagator.getInstance().inject(null, carrier, Map::put);
+    assertThat(carrier).isEmpty();
+  }
+
+  @Test
+  void inject_nullSetter() {
+    Map<String, String> carrier = new LinkedHashMap<>();
+    Context context = Context.current().with(Baggage.builder().put("cat", "meow").build());
+    W3CBaggagePropagator.getInstance().inject(context, carrier, null);
+    assertThat(carrier).isEmpty();
+  }
 }
diff --git a/...l/src/test/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagatorTest.java b/...l/src/test/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagatorTest.java
@@ -161,6 +161,25 @@ void inject_NotSampledContext_WithTraceState() {
             entry(W3CTraceContextPropagator.TRACE_STATE, TRACESTATE_NOT_DEFAULT_ENCODING));
   }
 
+  @Test
+  void inject_nullContext() {
+    Map<String, String> carrier = new LinkedHashMap<>();
+    w3cTraceContextPropagator.inject(null, carrier, setter);
+    assertThat(carrier).isEmpty();
+  }
+
+  @Test
+  void inject_nullSetter() {
+    Map<String, String> carrier = new LinkedHashMap<>();
+    Context context =
+        withSpanContext(
+            SpanContext.create(
+                TRACE_ID_BASE16, SPAN_ID_BASE16, TraceFlags.getDefault(), TRACE_STATE),
+            Context.current());
+    w3cTraceContextPropagator.inject(context, carrier, null);
+    assertThat(carrier).isEmpty();
+  }
+
   @Test
   void extract_Nothing() {
     // Context remains untouched.
@@ -491,4 +510,21 @@ void extract_emptyCarrier() {
                 w3cTraceContextPropagator.extract(Context.current(), emptyHeaders, getter)))
         .isSameAs(SpanContext.getInvalid());
   }
+
+  @Test
+  void extract_nullContext() {
+    assertThat(w3cTraceContextPropagator.extract(null, Collections.emptyMap(), getter))
+        .isSameAs(Context.root());
+  }
+
+  @Test
+  void extract_nullGetter() {
+    Context context =
+        withSpanContext(
+            SpanContext.create(
+                TRACE_ID_BASE16, SPAN_ID_BASE16, TraceFlags.getDefault(), TraceState.getDefault()),
+            Context.current());
+    assertThat(w3cTraceContextPropagator.extract(context, Collections.emptyMap(), null))
+        .isSameAs(context);
+  }
 }
diff --git a/extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java b/extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java
@@ -17,7 +17,6 @@
 import io.opentelemetry.context.propagation.TextMapSetter;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Objects;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
@@ -82,8 +81,12 @@ public Collection<String> fields() {
 
   @Override
   public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
-    Objects.requireNonNull(context, "context");
-    Objects.requireNonNull(setter, "setter");
+    if (context == null) {
+      return;
+    }
+    if (setter == null) {
+      return;
+    }
 
     Span span = Span.fromContext(context);
     if (!span.getSpanContext().isValid()) {
@@ -120,7 +123,12 @@ public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> se
 
   @Override
   public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter) {
-    Objects.requireNonNull(getter, "getter");
+    if (context == null) {
+      return Context.root();
+    }
+    if (getter == null) {
+      return context;
+    }
 
     SpanContext spanContext = getSpanContextFromHeader(carrier, getter);
     if (!spanContext.isValid()) {
@@ -130,7 +138,8 @@ public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C
     return context.with(Span.wrap(spanContext));
   }
 
-  private static <C> SpanContext getSpanContextFromHeader(C carrier, TextMapGetter<C> getter) {
+  private static <C> SpanContext getSpanContextFromHeader(
+      @Nullable C carrier, TextMapGetter<C> getter) {
     String traceHeader = getter.get(carrier, TRACE_HEADER_KEY);
     if (traceHeader == null || traceHeader.isEmpty()) {
       return SpanContext.getInvalid();

diff --git a/extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java b/extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java
@@ -95,6 +95,24 @@ void inject_WithTraceState() {
             "Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=0");
   }
 
+  @Test
+  void inject_nullContext() {
+    Map<String, String> carrier = new LinkedHashMap<>();
+    xrayPropagator.inject(null, carrier, setter);
+    assertThat(carrier).isEmpty();
+  }
+
+  @Test
+  void inject_nullSetter() {
+    Map<String, String> carrier = new LinkedHashMap<>();
+    Context context =
+        withSpanContext(
+            SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault()),
+            Context.current());
+    xrayPropagator.inject(context, carrier, null);
+    assertThat(carrier).isEmpty();
+  }
+
   @Test
   void extract_Nothing() {
     // Context remains untouched.
@@ -243,6 +261,21 @@ void extract_InvalidFlags_NonNumeric() {
         .isSameAs(SpanContext.getInvalid());
   }
 
+  @Test
+  void extract_nullContext() {
+    assertThat(xrayPropagator.extract(null, Collections.emptyMap(), getter))
+        .isSameAs(Context.root());
+  }
+
+  @Test
+  void extract_nullGetter() {
+    Context context =
+        withSpanContext(
+            SpanContext.create(TRACE_ID, SPAN_ID, TraceFlags.getDefault(), TraceState.getDefault()),
+            Context.current());
+    assertThat(xrayPropagator.extract(context, Collections.emptyMap(), null)).isSameAs(context);
+  }
+
   private static Context withSpanContext(SpanContext spanContext, Context context) {
     return context.with(Span.wrap(spanContext));
   }

diff --git a/...ors/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractor.java b/...ors/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractor.java
@@ -8,10 +8,11 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
 import java.util.Optional;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 @Immutable
 interface B3PropagatorExtractor {
 
-  <C> Optional<Context> extract(Context context, C carrier, TextMapGetter<C> getter);
+  <C> Optional<Context> extract(Context context, @Nullable C carrier, TextMapGetter<C> getter);
 }
diff --git a/...va/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorMultipleHeaders.java b/...va/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorMultipleHeaders.java
@@ -8,9 +8,9 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 @Immutable
@@ -19,14 +19,19 @@ final class B3PropagatorExtractorMultipleHeaders implements B3PropagatorExtracto
       Logger.getLogger(B3PropagatorExtractorMultipleHeaders.class.getName());
 
   @Override
-  public <C> Optional<Context> extract(Context context, C carrier, TextMapGetter<C> getter) {
-    Objects.requireNonNull(carrier, "carrier");
-    Objects.requireNonNull(getter, "getter");
+  public <C> Optional<Context> extract(
+      Context context, @Nullable C carrier, TextMapGetter<C> getter) {
+    if (context == null) {
+      return Optional.of(Context.root());
+    }
+    if (getter == null) {
+      return Optional.of(context);
+    }
     return extractSpanContextFromMultipleHeaders(context, carrier, getter);
   }
 
   private static <C> Optional<Context> extractSpanContextFromMultipleHeaders(
-      Context context, C carrier, TextMapGetter<C> getter) {
+      Context context, @Nullable C carrier, TextMapGetter<C> getter) {
     String traceId = getter.get(carrier, B3Propagator.TRACE_ID_HEADER);
     if (!Common.isTraceIdValid(traceId)) {
       logger.fine(

diff --git a/.../java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorSingleHeader.java b/.../java/io/opentelemetry/extension/trace/propagation/B3PropagatorExtractorSingleHeader.java
@@ -8,9 +8,9 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapGetter;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 @Immutable
@@ -19,14 +19,19 @@ final class B3PropagatorExtractorSingleHeader implements B3PropagatorExtractor {
       Logger.getLogger(B3PropagatorExtractorSingleHeader.class.getName());
 
   @Override
-  public <C> Optional<Context> extract(Context context, C carrier, TextMapGetter<C> getter) {
-    Objects.requireNonNull(carrier, "carrier");
-    Objects.requireNonNull(getter, "getter");
+  public <C> Optional<Context> extract(
+      Context context, @Nullable C carrier, TextMapGetter<C> getter) {
+    if (context == null) {
+      return Optional.of(Context.root());
+    }
+    if (getter == null) {
+      return Optional.of(context);
+    }
     return extractSpanContextFromSingleHeader(context, carrier, getter);
   }
 
   private static <C> Optional<Context> extractSpanContextFromSingleHeader(
-      Context context, C carrier, TextMapGetter<C> getter) {
+      Context context, @Nullable C carrier, TextMapGetter<C> getter) {
     String value = getter.get(carrier, B3Propagator.COMBINED_HEADER);
     if (StringUtils.isNullOrEmpty(value)) {
       return Optional.empty();

diff --git a/...tors/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorInjector.java b/...tors/src/main/java/io/opentelemetry/extension/trace/propagation/B3PropagatorInjector.java
@@ -7,9 +7,10 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapSetter;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 @Immutable
 interface B3PropagatorInjector {
-  <C> void inject(Context context, C carrier, TextMapSetter<C> setter);
+  <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter);
 }
diff --git a/...ava/io/opentelemetry/extension/trace/propagation/B3PropagatorInjectorMultipleHeaders.java b/...ava/io/opentelemetry/extension/trace/propagation/B3PropagatorInjectorMultipleHeaders.java
@@ -9,15 +9,19 @@
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapSetter;
-import java.util.Objects;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 @Immutable
 final class B3PropagatorInjectorMultipleHeaders implements B3PropagatorInjector {
   @Override
-  public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
-    Objects.requireNonNull(context, "context");
-    Objects.requireNonNull(setter, "setter");
+  public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
+    if (context == null) {
+      return;
+    }
+    if (setter == null) {
+      return;
+    }
 
     SpanContext spanContext = Span.fromContext(context).getSpanContext();
     if (!spanContext.isValid()) {

diff --git a/...n/java/io/opentelemetry/extension/trace/propagation/B3PropagatorInjectorSingleHeader.java b/...n/java/io/opentelemetry/extension/trace/propagation/B3PropagatorInjectorSingleHeader.java
@@ -11,7 +11,7 @@
 import io.opentelemetry.api.trace.TraceId;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.propagation.TextMapSetter;
-import java.util.Objects;
+import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 
 @Immutable
@@ -26,9 +26,13 @@ final class B3PropagatorInjectorSingleHeader implements B3PropagatorInjector {
   private static final int COMBINED_HEADER_SIZE = SAMPLED_FLAG_OFFSET + SAMPLED_FLAG_SIZE;
 
   @Override
-  public <C> void inject(Context context, C carrier, TextMapSetter<C> setter) {
-    Objects.requireNonNull(context, "context");
-    Objects.requireNonNull(setter, "setter");
+  public <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter) {
+    if (context == null) {
+      return;
+    }
+    if (setter == null) {
+      return;
+    }
 
     SpanContext spanContext = Span.fromContext(context).getSpanContext();
     if (!spanContext.isValid()) {