Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Getter.Keys() with Jaeger Baggage support. #1549

Merged
merged 15 commits into from
Nov 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ public class HttpTraceContextExtractBenchmark {
private final HttpTraceContext httpTraceContext = HttpTraceContext.getInstance();
private final Getter<Map<String, String>> getter =
new Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
import edu.berkeley.cs.jqf.fuzz.Fuzz;
import edu.berkeley.cs.jqf.fuzz.JQF;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator.Getter;
import java.util.Map;
import javax.annotation.Nullable;
import org.junit.runner.RunWith;

@RunWith(JQF.class)
Expand All @@ -22,7 +24,21 @@ public class W3CBaggagePropagatorFuzzTest {
@Fuzz
public void safeForRandomInputs(String baggage) {
Context context =
baggagePropagator.extract(Context.root(), ImmutableMap.of("baggage", baggage), Map::get);
baggagePropagator.extract(
Context.root(),
ImmutableMap.of("baggage", baggage),
new Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Nullable
@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
}
});
assertThat(context).isNotNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,28 @@
import io.opentelemetry.api.baggage.Baggage;
import io.opentelemetry.api.baggage.EntryMetadata;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator.Getter;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;

class W3CBaggagePropagatorTest {

private static final Getter<Map<String, String>> getter =
new Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Nullable
@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
}
};

@Test
void fields() {
assertThat(W3CBaggagePropagator.getInstance().fields()).containsExactly("baggage");
Expand All @@ -27,8 +43,7 @@ void fields() {
void extract_noBaggageHeader() {
W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();

Context result =
propagator.extract(Context.root(), ImmutableMap.<String, String>of(), Map::get);
Context result = propagator.extract(Context.root(), ImmutableMap.<String, String>of(), getter);

assertThat(result).isEqualTo(Context.root());
}
Expand All @@ -37,8 +52,7 @@ void extract_noBaggageHeader() {
void extract_emptyBaggageHeader() {
W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();

Context result =
propagator.extract(Context.root(), ImmutableMap.of("baggage", ""), ImmutableMap::get);
Context result = propagator.extract(Context.root(), ImmutableMap.of("baggage", ""), getter);

assertThat(Baggage.fromContext(result)).isEqualTo(Baggage.empty());
}
Expand All @@ -48,8 +62,7 @@ void extract_singleEntry() {
W3CBaggagePropagator propagator = W3CBaggagePropagator.getInstance();

Context result =
propagator.extract(
Context.root(), ImmutableMap.of("baggage", "key=value"), ImmutableMap::get);
propagator.extract(Context.root(), ImmutableMap.of("baggage", "key=value"), getter);

Baggage expectedBaggage = Baggage.builder().put("key", "value").build();
assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage);
Expand All @@ -61,9 +74,7 @@ void extract_multiEntry() {

Context result =
propagator.extract(
Context.root(),
ImmutableMap.of("baggage", "key1=value1,key2=value2"),
ImmutableMap::get);
Context.root(), ImmutableMap.of("baggage", "key1=value1,key2=value2"), getter);

Baggage expectedBaggage = Baggage.builder().put("key1", "value1").put("key2", "value2").build();
assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage);
Expand All @@ -75,7 +86,7 @@ void extract_duplicateKeys() {

Context result =
propagator.extract(
Context.root(), ImmutableMap.of("baggage", "key=value1,key=value2"), ImmutableMap::get);
Context.root(), ImmutableMap.of("baggage", "key=value1,key=value2"), getter);

Baggage expectedBaggage = Baggage.builder().put("key", "value2").build();
assertThat(Baggage.fromContext(result)).isEqualTo(expectedBaggage);
Expand All @@ -89,7 +100,7 @@ void extract_withMetadata() {
propagator.extract(
Context.root(),
ImmutableMap.of("baggage", "key=value;metadata-key=value;othermetadata"),
ImmutableMap::get);
getter);

Baggage expectedBaggage =
Baggage.builder()
Expand All @@ -109,7 +120,7 @@ void extract_fullComplexities() {
"baggage",
"key1= value1; metadata-key = value; othermetadata, "
+ "key2 =value2 , key3 =\tvalue3 ; "),
ImmutableMap::get);
getter);

Baggage expectedBaggage =
Baggage.builder()
Expand All @@ -135,7 +146,7 @@ void extract_invalidHeader() {
"baggage",
"key1= v;alsdf;-asdflkjasdf===asdlfkjadsf ,,a sdf9asdf-alue1; metadata-key = "
+ "value; othermetadata, key2 =value2 , key3 =\tvalue3 ; "),
ImmutableMap::get);
getter);

assertThat(Baggage.fromContext(result)).isEqualTo(Baggage.empty());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
import edu.berkeley.cs.jqf.fuzz.Fuzz;
import edu.berkeley.cs.jqf.fuzz.JQF;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator.Getter;
import java.util.Map;
import javax.annotation.Nullable;
import org.junit.runner.RunWith;

@RunWith(JQF.class)
Expand All @@ -25,7 +27,19 @@ public void safeForRandomInputs(String traceParentHeader, String traceStateHeade
httpTraceContext.extract(
Context.root(),
ImmutableMap.of("traceparent", traceParentHeader, "tracestate", traceStateHeader),
Map::get);
new Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Nullable
@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
}
});

assertThat(context).isNotNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;

/** Unit tests for {@link HttpTraceContext}. */
Expand All @@ -39,7 +40,19 @@ class HttpTraceContextTest {
private static final String TRACEPARENT_HEADER_NOT_SAMPLED =
"00-" + TRACE_ID_BASE16 + "-" + SPAN_ID_BASE16 + "-00";
private static final Setter<Map<String, String>> setter = Map::put;
private static final Getter<Map<String, String>> getter = Map::get;
private static final Getter<Map<String, String>> getter =
new Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Nullable
@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
}
};
// Encoding preserves the order which is the reverse order of adding.
private static final String TRACESTATE_NOT_DEFAULT_ENCODING = "bar=baz,foo=bar";
private static final String TRACESTATE_NOT_DEFAULT_ENCODING_WITH_SPACES =
Expand Down Expand Up @@ -151,7 +164,7 @@ void extract_Nothing() {
// Context remains untouched.
assertThat(
httpTraceContext.extract(
Context.current(), Collections.<String, String>emptyMap(), Map::get))
Context.current(), Collections.<String, String>emptyMap(), getter))
.isSameAs(Context.current());
}

Expand All @@ -169,9 +182,7 @@ void extract_SampledContext() {
void extract_NullCarrier() {
Map<String, String> carrier = new LinkedHashMap<>();
carrier.put(TRACE_PARENT, TRACEPARENT_HEADER_SAMPLED);
assertThat(
getSpanContext(
httpTraceContext.extract(Context.current(), null, (c, k) -> carrier.get(k))))
assertThat(getSpanContext(httpTraceContext.extract(Context.current(), carrier, getter)))
.isEqualTo(
SpanContext.createFromRemoteParent(
TRACE_ID_BASE16, SPAN_ID_BASE16, SAMPLED_TRACE_OPTIONS, TRACE_STATE_DEFAULT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ interface Setter<C> {
*/
interface Getter<C> {

/**
* Returns all the keys in the given carrier.
*
* @param carrier carrier of propagation fields, such as an http request.
* @since 0.10.0
*/
Iterable<String> keys(C carrier);

/**
* Returns the first value of the given propagation {@code key} or returns {@code null}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ private MapSetter() {}
private static final class MapGetter implements TextMapPropagator.Getter<Map<String, String>> {
private static final MapGetter INSTANCE = new MapGetter();

@Override
public Iterable<String> keys(Map<String, String> map) {
return map.keySet();
}

@Override
public String get(Map<String, String> map, String key) {
return map.get(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ public static class JaegerContextExtractBenchmark extends AbstractContextExtract

private final TextMapPropagator.Getter<Map<String, String>> getter =
new TextMapPropagator.Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
Expand Down Expand Up @@ -137,6 +142,11 @@ public static class JaegerUrlEncodedContextExtractBenchmark

private final TextMapPropagator.Getter<Map<String, String>> getter =
new TextMapPropagator.Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
Expand Down Expand Up @@ -180,6 +190,11 @@ public static class B3SingleHeaderContextExtractBenchmark

private final TextMapPropagator.Getter<Map<String, String>> getter =
new TextMapPropagator.Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
Expand Down Expand Up @@ -226,6 +241,11 @@ private static Map<String, String> createHeaders(

private final TextMapPropagator.Getter<Map<String, String>> getter =
new TextMapPropagator.Getter<Map<String, String>>() {
@Override
public Iterable<String> keys(Map<String, String> carrier) {
return carrier.keySet();
}

@Override
public String get(Map<String, String> carrier, String key) {
return carrier.get(key);
Expand Down
Loading