Skip to content

Commit

Permalink
Add tests and update behavior to verify propagators do not alter the … (
Browse files Browse the repository at this point in the history 
#3194)

* Add tests and update behavior to verify propagators do not alter the context when failing to parse inputs.

* Ad a couple tests around null contexts

* formatting
  • Loading branch information
@jkwatson
jkwatson authored May 6, 2021
1 parent 2a002f2 commit a8a85c6
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 133 deletions.
4 changes: 2 additions & 2 deletions api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C
return context;
}
if (baggageHeader.isEmpty()) {
return context.with(Baggage.empty());
return context;
}

BaggageBuilder baggageBuilder = Baggage.builder();
try {
extractEntries(baggageHeader, baggageBuilder);
} catch (RuntimeException e) {
return context.with(Baggage.empty());
return context;
}
return context.with(baggageBuilder.build());
}
Expand Down
9 changes: 3 additions & 6 deletions api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,20 @@ void extract_fullComplexities() {
assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage);
}

/**
* It would be cool if we could replace this with a fuzzer to generate tons of crud data, to make
* sure we don't blow up with it.
*/
@Test
void extract_invalidHeader() {
W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();

Context input = Context.current();
Context result =
propagator.extract(
Context.root(),
input,
ImmutableMap.of(
"baggage",
"key1= v;alsdf;-asdflkjasdf===asdlfkjadsf ,,a sdf9asdf-alue1; metadata-key = "
+ "value; othermetadata, key2 =value2 , key3 =\tvalue3 ; "),
getter);

assertThat(result).isSameAs(input);
assertThat(Baggage.fromContext(result)).isEqualTo(Baggage.empty());
}

Expand Down
79 changes: 31 additions & 48 deletions ...l/src/test/java/io/opentelemetry/api/trace/propagation/W3CTraceContextPropagatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,42 +298,42 @@ void extract_NotSampledContext_TraceStateWithSpaces() {
void extract_EmptyHeader() {
Map<String, String> invalidHeaders = new LinkedHashMap<>();
invalidHeaders.put(W3CTraceContextPropagator.TRACE_PARENT, "");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
void extract_invalidDelimiters() {
Map<String, String> carrier = new LinkedHashMap<>();
Context input = Context.current();
Context result;

carrier.put(
W3CTraceContextPropagator.TRACE_PARENT,
"01+" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00-02");
assertThat(
getSpanContext(w3cTraceContextPropagator.extract(Context.current(), carrier, getter)))
.isEqualTo(SpanContext.getInvalid());
result = w3cTraceContextPropagator.extract(input, carrier, getter);
assertThat(result).isSameAs(input);
assertThat(getSpanContext(result)).isEqualTo(SpanContext.getInvalid());

carrier.put(
W3CTraceContextPropagator.TRACE_PARENT,
"01-" + TRACE_ID_BASE16 + "+" + SPAN_ID_BASE16 + "-00-02");
assertThat(
getSpanContext(w3cTraceContextPropagator.extract(Context.current(), carrier, getter)))
.isEqualTo(SpanContext.getInvalid());
result = w3cTraceContextPropagator.extract(input, carrier, getter);
assertThat(result).isSameAs(input);
assertThat(getSpanContext(result)).isEqualTo(SpanContext.getInvalid());

carrier.put(
W3CTraceContextPropagator.TRACE_PARENT,
"01-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "+00-02");
assertThat(
getSpanContext(w3cTraceContextPropagator.extract(Context.current(), carrier, getter)))
.isEqualTo(SpanContext.getInvalid());
result = w3cTraceContextPropagator.extract(input, carrier, getter);
assertThat(result).isSameAs(input);
assertThat(getSpanContext(result)).isEqualTo(SpanContext.getInvalid());

carrier.put(
W3CTraceContextPropagator.TRACE_PARENT,
"01-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00+02");
assertThat(
getSpanContext(w3cTraceContextPropagator.extract(Context.current(), carrier, getter)))
.isEqualTo(SpanContext.getInvalid());
result = w3cTraceContextPropagator.extract(input, carrier, getter);
assertThat(result).isSameAs(input);
assertThat(getSpanContext(result)).isEqualTo(SpanContext.getInvalid());
}

@Test
Expand All @@ -342,10 +342,7 @@ void extract_InvalidTraceId() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + "abcdefghijklmnopabcdefghijklmnop" + "-" + SPAN_ID_BASE16 + "-01");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -354,10 +351,7 @@ void extract_InvalidTraceId_Size() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + TRACE_ID_BASE16 + "00-" + SPAN_ID_BASE16 + "-01");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -366,10 +360,7 @@ void extract_InvalidSpanId() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + TRACE_ID_BASE16 + "-" + "abcdefghijklmnop" + "-01");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -378,10 +369,7 @@ void extract_InvalidSpanId_Size() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "00-01");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -390,10 +378,7 @@ void extract_InvalidTraceFlags() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-gh");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -402,10 +387,14 @@ void extract_InvalidTraceFlags_Size() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-0100");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

private void verifyInvalidBehavior(Map<String, String> invalidHeaders) {
Context input = Context.current();
Context result = w3cTraceContextPropagator.extract(input, invalidHeaders, getter);
assertThat(result).isSameAs(input);
assertThat(getSpanContext(result)).isSameAs(SpanContext.getInvalid());
}

@Test
Expand Down Expand Up @@ -474,10 +463,7 @@ void extract_InvalidVersion_ff() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"ff-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-01");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -486,10 +472,7 @@ void extract_InvalidTraceparent_extraTrailing() {
invalidHeaders.put(
W3CTraceContextPropagator.TRACE_PARENT,
"00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00-01");
assertThat(
getSpanContext(
w3cTraceContextPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand Down
27 changes: 26 additions & 1 deletion context/src/test/java/io/opentelemetry/context/propagation/MultiTextMapPropagatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,19 @@ void inject_allDelegated() {
verify(propagator3).inject(context, carrier, setter);
}

@Test
void inject_nullContextAllDelegated() {
Map<String, String> carrier = new HashMap<>();
Context context = null;
TextMapSetter<Map<String, String>> setter = Map::put;

TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2, propagator3);
prop.inject(context, carrier, setter);
verify(propagator1).inject(context, carrier, setter);
verify(propagator2).inject(context, carrier, setter);
verify(propagator3).inject(context, carrier, setter);
}

@Test
void extract_noPropagators() {
Map<String, String> carrier = new HashMap<>();
Expand Down Expand Up @@ -121,7 +134,6 @@ void extract_found_all() {

@Test
void extract_notFound() {

Map<String, String> carrier = new HashMap<>();
Context context = mock(Context.class);
when(propagator1.extract(context, carrier, getter)).thenReturn(context);
Expand All @@ -132,4 +144,17 @@ void extract_notFound() {

assertThat(result).isSameAs(context);
}

@Test
void extract_nullContext() {
Map<String, String> carrier = new HashMap<>();
Context context = null;
when(propagator1.extract(context, carrier, getter)).thenReturn(context);
when(propagator2.extract(context, carrier, getter)).thenReturn(context);

TextMapPropagator prop = new MultiTextMapPropagator(propagator1, propagator2);
Context result = prop.extract(context, carrier, getter);

assertThat(result).isSameAs(context);
}
}
33 changes: 33 additions & 0 deletions context/src/test/java/io/opentelemetry/context/propagation/NoopTextMapPropagatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.context.Context;
import java.util.HashMap;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;

class NoopTextMapPropagatorTest {
Expand All @@ -15,4 +18,34 @@ class NoopTextMapPropagatorTest {
void noopFields() {
assertThat(TextMapPropagator.noop().fields()).isEmpty();
}

@Test
void extract_contextUnchanged() {
Context input = Context.current();
Context result =
TextMapPropagator.noop().extract(input, new HashMap<>(), new HashMapTextMapGetter());
assertThat(result).isSameAs(input);
}

@Test
void extract_nullContext() {
Context input = null;
Context result =
TextMapPropagator.noop().extract(input, new HashMap<>(), new HashMapTextMapGetter());
assertThat(result).isSameAs(input);
}

private static class HashMapTextMapGetter
implements TextMapGetter<HashMap<? extends Object, ? extends Object>> {
@Override
public Iterable<String> keys(HashMap<?, ?> carrier) {
return null;
}

@Nullable
@Override
public String get(@Nullable HashMap<?, ?> carrier, String key) {
return null;
}
}
}
5 changes: 3 additions & 2 deletions extensions/aws/src/main/java/io/opentelemetry/extension/aws/AwsXrayPropagator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,9 @@ private static <C> Context getContextFromHeader(
spanId,
isSampled ? TraceFlags.getSampled() : TraceFlags.getDefault(),
TraceState.getDefault());

context = context.with(Span.wrap(spanContext));
if (spanContext.isValid()) {
context = context.with(Span.wrap(spanContext));
}
if (baggage != null) {
context = context.with(baggage.build());
}
Expand Down
31 changes: 15 additions & 16 deletions extensions/aws/src/test/java/io/opentelemetry/extension/aws/AwsXrayPropagatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,7 @@ void extract_EmptyHeaderValue() {
Map<String, String> invalidHeaders = new LinkedHashMap<>();
invalidHeaders.put(TRACE_HEADER_KEY, "");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -284,8 +283,7 @@ void extract_InvalidTraceId() {
TRACE_HEADER_KEY,
"Root=abcdefghijklmnopabcdefghijklmnop;Parent=53995c3f42cd8ad8;Sampled=0");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -295,8 +293,7 @@ void extract_InvalidTraceId_Size() {
TRACE_HEADER_KEY,
"Root=1-8a3c60f7-d188f8fa79d48a391a778fa600;Parent=53995c3f42cd8ad8;Sampled=0");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -306,8 +303,7 @@ void extract_InvalidSpanId() {
TRACE_HEADER_KEY,
"Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=abcdefghijklmnop;Sampled=0");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -317,8 +313,7 @@ void extract_InvalidSpanId_Size() {
TRACE_HEADER_KEY,
"Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad800;Sampled=0");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -328,8 +323,7 @@ void extract_InvalidFlags() {
TRACE_HEADER_KEY,
"Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -339,8 +333,7 @@ void extract_InvalidFlags_Size() {
TRACE_HEADER_KEY,
"Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=10220");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

@Test
Expand All @@ -350,8 +343,14 @@ void extract_InvalidFlags_NonNumeric() {
TRACE_HEADER_KEY,
"Root=1-8a3c60f7-d188f8fa79d48a391a778fa6;Parent=53995c3f42cd8ad8;Sampled=a");

assertThat(getSpanContext(xrayPropagator.extract(Context.current(), invalidHeaders, getter)))
.isSameAs(SpanContext.getInvalid());
verifyInvalidBehavior(invalidHeaders);
}

private void verifyInvalidBehavior(Map<String, String> invalidHeaders) {
Context input = Context.current();
Context result = xrayPropagator.extract(input, invalidHeaders, getter);
assertThat(result).isSameAs(input);
assertThat(getSpanContext(result)).isSameAs(SpanContext.getInvalid());
}

@Test
Expand Down
Loading

0 comments on commit a8a85c6

Please sign in to comment.