Skip to content

Commit

Permalink
Adds b3:0 header to avoid trace amplification when using proxies
Browse files Browse the repository at this point in the history
  • Loading branch information
Adrian Cole committed Sep 10, 2020
1 parent 63222d8 commit 54efe27
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ public final Builder toBuilder() {

Request newRequest(RequestBody body) throws IOException {
Request.Builder request = new Request.Builder().url(endpoint);
// Amplification can occur when the Zipkin endpoint is proxied, and the proxy is instrumented.
// This prevents that in proxies, such as Envoy, that understand B3 single format,
request.addHeader("b3", "0");
if (compressionEnabled) {
request.addHeader("Content-Encoding", "gzip");
Buffer gzipped = new Buffer();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2019 The OpenZipkin Authors
* Copyright 2016-2020 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -53,7 +53,7 @@ public class OkHttpSenderTest {
OkHttpSender sender =
OkHttpSender.newBuilder().endpoint(endpoint).compressionEnabled(false).build();

@Test public void badUrlIsAnIllegalArgument() throws Exception {
@Test public void badUrlIsAnIllegalArgument() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("invalid post url: ");

Expand Down Expand Up @@ -138,6 +138,15 @@ public class OkHttpSenderTest {
.isLessThan(requests.get(1).getBodySize());
}

@Test public void ensuresProxiesDontTrace() throws Exception {
server.enqueue(new MockResponse());

send(CLIENT_SPAN, CLIENT_SPAN).execute();

// If the Zipkin endpoint is proxied and instrumented, it will know "0" means don't trace.
assertThat(server.takeRequest().getHeader("b3")).isEqualTo("0");
}

@Test public void mediaTypeBasedOnSpanEncoding() throws Exception {
server.enqueue(new MockResponse());

Expand Down Expand Up @@ -254,7 +263,7 @@ public class OkHttpSenderTest {
}
}

@Test public void noExceptionWhenServerErrors() throws Exception {
@Test public void noExceptionWhenServerErrors() {
server.enqueue(new MockResponse().setResponseCode(500));

send().enqueue(new Callback<Void>() {
Expand All @@ -266,22 +275,22 @@ public class OkHttpSenderTest {
});
}

@Test public void outOfBandCancel() throws Exception {
@Test public void outOfBandCancel() {
HttpCall call = (HttpCall) send(CLIENT_SPAN, CLIENT_SPAN);
call.cancel();

assertThat(call.isCanceled()).isTrue();
}

@Test public void check_ok() throws Exception {
@Test public void check_ok() {
server.enqueue(new MockResponse());

assertThat(sender.check().ok()).isTrue();

assertThat(server.getRequestCount()).isEqualTo(1);
}

@Test public void check_fail() throws Exception {
@Test public void check_fail() {
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));

assertThat(sender.check().ok()).isFalse();
Expand All @@ -300,11 +309,11 @@ public class OkHttpSenderTest {
* tools, care should be taken to ensure the toString() output is a reasonable length and does not
* contain sensitive information.
*/
@Test public void toStringContainsOnlySenderTypeAndEndpoint() throws Exception {
@Test public void toStringContainsOnlySenderTypeAndEndpoint() {
assertThat(sender.toString()).isEqualTo("OkHttpSender{" + endpoint + "}");
}

@Test public void bugGuardCache() throws Exception {
@Test public void bugGuardCache() {
assertThat(sender.client.cache())
.withFailMessage("senders should not open a disk cache")
.isNull();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2019 The OpenZipkin Authors
* Copyright 2016-2020 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -209,6 +209,9 @@ void send(byte[] body, String mediaType) throws IOException {
connection.setConnectTimeout(connectTimeout);
connection.setReadTimeout(readTimeout);
connection.setRequestMethod("POST");
// Amplification can occur when the Zipkin endpoint is proxied, and the proxy is instrumented.
// This prevents that in proxies, such as Envoy, that understand B3 single format,
connection.addRequestProperty("b3", "0");
connection.addRequestProperty("Content-Type", mediaType);
if (compressionEnabled) {
connection.addRequestProperty("Content-Encoding", "gzip");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2019 The OpenZipkin Authors
* Copyright 2016-2020 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -45,15 +45,15 @@ public class URLConnectionSenderTest {
URLConnectionSender sender;
String endpoint = server.url("/api/v2/spans").toString();

@Before public void setUp() throws Exception {
@Before public void setUp() {
sender = URLConnectionSender.newBuilder()
.endpoint(endpoint)
.compressionEnabled(false)
.build();
}

@Test
public void badUrlIsAnIllegalArgument() throws Exception {
public void badUrlIsAnIllegalArgument() {
thrown.expect(IllegalArgumentException.class);
thrown.expectMessage("unknown protocol: htp");

Expand Down Expand Up @@ -121,6 +121,15 @@ public void badUrlIsAnIllegalArgument() throws Exception {
.isLessThan(requests.get(1).getBodySize());
}

@Test public void ensuresProxiesDontTrace() throws Exception {
server.enqueue(new MockResponse());

send(CLIENT_SPAN, CLIENT_SPAN).execute();

// If the Zipkin endpoint is proxied and instrumented, it will know "0" means don't trace.
assertThat(server.takeRequest().getHeader("b3")).isEqualTo("0");
}

@Test public void mediaTypeBasedOnSpanEncoding() throws Exception {
server.enqueue(new MockResponse());

Expand All @@ -131,7 +140,7 @@ public void badUrlIsAnIllegalArgument() throws Exception {
.isEqualTo("application/json");
}

@Test public void noExceptionWhenServerErrors() throws Exception {
@Test public void noExceptionWhenServerErrors() {
server.enqueue(new MockResponse().setResponseCode(500));

send().enqueue(new Callback<Void>() {
Expand All @@ -143,15 +152,15 @@ public void badUrlIsAnIllegalArgument() throws Exception {
});
}

@Test public void check_ok() throws Exception {
@Test public void check_ok() {
server.enqueue(new MockResponse());

assertThat(sender.check().ok()).isTrue();

assertThat(server.getRequestCount()).isEqualTo(1);
}

@Test public void check_fail() throws Exception {
@Test public void check_fail() {
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));

assertThat(sender.check().ok()).isFalse();
Expand All @@ -170,7 +179,7 @@ public void badUrlIsAnIllegalArgument() throws Exception {
* tools, care should be taken to ensure the toString() output is a reasonable length and does not
* contain sensitive information.
*/
@Test public void toStringContainsOnlySenderTypeAndEndpoint() throws Exception {
@Test public void toStringContainsOnlySenderTypeAndEndpoint() {
assertThat(sender.toString()).isEqualTo("URLConnectionSender{" + endpoint + "}");
}

Expand Down

0 comments on commit 54efe27

Please sign in to comment.