From 375f244279265281c8fe91894522e5641ebcb7cb Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 7 Mar 2018 06:41:47 -0500 Subject: [PATCH] Added support for customization of HttpTracing with this change you can define beans that will get added / set to the `HttpTracing` bean fixes gh-886 --- .../main/asciidoc/spring-cloud-sleuth.adoc | 26 ++++- .../sleuth/instrument/web/ClientSampler.java | 45 ++++++++ .../sleuth/instrument/web/ServerSampler.java | 43 ++++++++ .../instrument/web/SkipPatternProvider.java | 4 +- .../web/SleuthHttpLegacyProperties.java | 36 ++++++ .../web/TraceHttpAutoConfiguration.java | 103 +++++++++++++++--- .../web/CompositeHttpSamplerTests.java | 81 ++++++++++++++ .../web/TraceFilterWebIntegrationTests.java | 26 +++++ 8 files changed, 343 insertions(+), 21 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ClientSampler.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ServerSampler.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpLegacyProperties.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/CompositeHttpSamplerTests.java diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 84b8ca2b3..2021a92eb 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -762,11 +762,31 @@ Running the preceding method with a value of `15` leads to setting a tag with a //=== Spring Integration -// TODO: Update this +=== HTTP -// === HTTP +If a customization of client / server parsing of the HTTP related spans is required, +just register a bean of type `brave.http.HttpClientParser` or +`brave.http.HttpServerParser`. If client /server sampling is required, just +register a bean of type `brave.http.HttpSampler` and name the bean + `sleuthClientSampler` for client sampler and `sleuthServerSampler` for server sampler. + For your convenience the `@ClientSampler` and `@ServerSampler` + annotations can be used to inject the proper beans or to + reference the bean names via their static String `NAME` fields. -// TODO: Update this +Check out Brave's code to see an example of how to make a path-based sampler +https://github.com/openzipkin/brave/tree/master/instrumentation/http#sampling-policy + +If you want to completely rewrite the `HttpTracing` bean you can use the `SkipPatternProvider` +interface to retrieve the URL `Pattern` for spans that should be not sampled. Below you can see +an example of usage of `SkipPatternProvider` inside a server side, `HttpSampler`. + +[source,java] +---- +@Configuration +class Config { +include::../../../../spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java[tags=custom_server_sampler,indent=2] +} +---- === `TraceFilter` diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ClientSampler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ClientSampler.java new file mode 100644 index 000000000..a66ef2294 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ClientSampler.java @@ -0,0 +1,45 @@ +/* + * Copyright 2013-2018 the original author or 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.beans.factory.annotation.Qualifier; + +/** + * Annotate a client {@link brave.http.HttpSampler} that hsould be + * injected to {@link brave.http.HttpTracing} + * + * @author Marcin Grzejszczak + * @since 2.0.0 + * @see Qualifier + */ +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.TYPE, ElementType.ANNOTATION_TYPE}) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +@Documented +@Qualifier(ClientSampler.NAME) +public @interface ClientSampler { + + String NAME = "sleuthClientSampler"; + +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ServerSampler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ServerSampler.java new file mode 100644 index 000000000..146956cb9 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ServerSampler.java @@ -0,0 +1,43 @@ +/* + * Copyright 2013-2018 the original author or 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.beans.factory.annotation.Qualifier; + +/** + * Annotate a server {@link brave.http.HttpSampler} that hsould be + * injected to {@link brave.http.HttpTracing} + * + * @author Marcin Grzejszczak + * @since 2.0.0 + * @see Qualifier + */ +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.TYPE, ElementType.ANNOTATION_TYPE}) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +@Documented +@Qualifier(ServerSampler.NAME) +public @interface ServerSampler { + String NAME = "sleuthServerSampler"; +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProvider.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProvider.java index 801f98871..7953d7496 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProvider.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SkipPatternProvider.java @@ -19,11 +19,11 @@ package org.springframework.cloud.sleuth.instrument.web; import java.util.regex.Pattern; /** - * Internal interface to describe patterns to skip tracing + * Provides a URL {@link Pattern} for spans that should be not sampled. * * @author Marcin Grzejszczak * @since 2.0.0 */ -interface SkipPatternProvider { +public interface SkipPatternProvider { Pattern skipPattern(); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpLegacyProperties.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpLegacyProperties.java new file mode 100644 index 000000000..db716c223 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpLegacyProperties.java @@ -0,0 +1,36 @@ +/* + * Copyright 2013-2018 the original author or 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * @author Marcin Grzejszczak + * @since 2.0.0 + */ +@ConfigurationProperties("spring.sleuth.http.legacy") +public class SleuthHttpLegacyProperties { + private boolean enabled; + + public boolean isEnabled() { + return this.enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java index 9858c9d39..466d16517 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java @@ -17,12 +17,17 @@ package org.springframework.cloud.sleuth.instrument.web; import brave.Tracing; +import brave.http.HttpAdapter; +import brave.http.HttpClientParser; +import brave.http.HttpSampler; +import brave.http.HttpServerParser; import brave.http.HttpTracing; -import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.sleuth.ErrorParser; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.context.annotation.Bean; @@ -39,28 +44,94 @@ import org.springframework.context.annotation.Configuration; @ConditionalOnBean(Tracing.class) @ConditionalOnProperty(name = "spring.sleuth.http.enabled", havingValue = "true", matchIfMissing = true) @AutoConfigureAfter(TraceWebAutoConfiguration.class) +@EnableConfigurationProperties(SleuthHttpLegacyProperties.class) public class TraceHttpAutoConfiguration { + @Autowired HttpClientParser clientParser; + @Autowired HttpServerParser serverParser; + @Autowired @ClientSampler HttpSampler clientSampler; + @Autowired(required = false) @ServerSampler HttpSampler serverSampler; + @Bean @ConditionalOnMissingBean // NOTE: stable bean name as might be used outside sleuth HttpTracing httpTracing( - @Value("${spring.sleuth.http.legacy.enabled:false}") boolean legacyEnabled, Tracing tracing, - TraceKeys traceKeys, - ErrorParser errorParser, - SkipPatternProvider provider - ) { - if (legacyEnabled) { - return HttpTracing.newBuilder(tracing) - .clientParser(new SleuthHttpClientParser(traceKeys)) - .serverParser(new SleuthHttpServerParser(traceKeys, errorParser)) - .serverSampler(new SleuthHttpSampler(provider)) - .build(); - } - return HttpTracing - .newBuilder(tracing) - .serverSampler(new SleuthHttpSampler(provider)) + SkipPatternProvider provider) { + HttpSampler serverSampler = combineUserProvidedSamplerWithSkipPatternSampler( + provider); + return HttpTracing.newBuilder(tracing) + .clientParser(this.clientParser) + .serverParser(this.serverParser) + .clientSampler(this.clientSampler) + .serverSampler(serverSampler) .build(); } + + private HttpSampler combineUserProvidedSamplerWithSkipPatternSampler( + SkipPatternProvider provider) { + HttpSampler serverSampler = this.serverSampler; + SleuthHttpSampler skipPatternSampler = new SleuthHttpSampler(provider); + if (serverSampler == null) { + return skipPatternSampler; + } + return new CompositeHttpSampler(skipPatternSampler, serverSampler); + } + + @Bean + @ConditionalOnProperty(name = "spring.sleuth.http.legacy.enabled", havingValue = "true") + HttpClientParser sleuthHttpClientParser(TraceKeys traceKeys) { + return new SleuthHttpClientParser(traceKeys); + } + + @Bean + @ConditionalOnProperty(name = "spring.sleuth.http.legacy.enabled", + havingValue = "false", matchIfMissing = true) + @ConditionalOnMissingBean + HttpClientParser httpClientParser() { + return new HttpClientParser(); + } + + @Bean + @ConditionalOnProperty(name = "spring.sleuth.http.legacy.enabled", havingValue = "true") + HttpServerParser sleuthHttpServerParser(TraceKeys traceKeys, ErrorParser errorParser) { + return new SleuthHttpServerParser(traceKeys, errorParser); + } + + @Bean + @ConditionalOnProperty(name = "spring.sleuth.http.legacy.enabled", + havingValue = "false", matchIfMissing = true) + @ConditionalOnMissingBean + HttpServerParser defaultHttpServerParser() { + return new HttpServerParser(); + } + + @Bean + @ConditionalOnMissingBean(name = ClientSampler.NAME) + HttpSampler sleuthClientSampler() { + return HttpSampler.TRACE_ID; + } +} + +class CompositeHttpSampler extends HttpSampler { + + private final HttpSampler left, right; + + CompositeHttpSampler(HttpSampler left, HttpSampler right) { + this.left = left; + this.right = right; + } + + @Override public Boolean trySample(HttpAdapter adapter, Req request) { + // If either decision is false, return false + Boolean leftDecision = this.left.trySample(adapter, request); + if (Boolean.FALSE.equals(leftDecision)) return false; + Boolean rightDecision = this.right.trySample(adapter, request); + if (Boolean.FALSE.equals(rightDecision)) return false; + // If either decision is null, return the other + if (leftDecision == null) return rightDecision; + if (rightDecision == null) return leftDecision; + // Neither are null and at least one is true + return leftDecision && rightDecision; + } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/CompositeHttpSamplerTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/CompositeHttpSamplerTests.java new file mode 100644 index 000000000..59a3567a9 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/CompositeHttpSamplerTests.java @@ -0,0 +1,81 @@ +/* + * Copyright 2013-2018 the original author or 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import brave.http.HttpAdapter; +import brave.http.HttpSampler; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.assertj.core.api.BDDAssertions.then; +import static org.mockito.BDDMockito.given; + +@RunWith(MockitoJUnitRunner.class) +public class CompositeHttpSamplerTests { + + @Mock HttpAdapter adapter; + @Mock HttpSampler left, right; + HttpSampler sampler; + Object request = new Object(); + + @Before + public void init(){ + this.sampler = new CompositeHttpSampler(left, right); + } + + @Test + public void should_return_null_on_both_null() { + given(this.left.trySample(this.adapter, this.request)).willReturn(null); + given(this.right.trySample(this.adapter, this.request)).willReturn(null); + + then(this.sampler.trySample(this.adapter, this.request)).isNull(); + } + + @Test + public void should_return_false_on_any_false() { + given(this.left.trySample(this.adapter, this.request)).willReturn(false); + given(this.right.trySample(this.adapter, this.request)).willReturn(null); + + then(this.sampler.trySample(this.adapter, this.request)).isFalse(); + + given(this.left.trySample(this.adapter, this.request)).willReturn(null); + given(this.right.trySample(this.adapter, this.request)).willReturn(false); + + then(this.sampler.trySample(this.adapter, this.request)).isFalse(); + + given(this.left.trySample(this.adapter, this.request)).willReturn(false); + given(this.right.trySample(this.adapter, this.request)).willReturn(true); + + then(this.sampler.trySample(this.adapter, this.request)).isFalse(); + + given(this.left.trySample(this.adapter, this.request)).willReturn(true); + given(this.right.trySample(this.adapter, this.request)).willReturn(false); + + then(this.sampler.trySample(this.adapter, this.request)).isFalse(); + } + + @Test + public void should_return_true_on_both_true() { + given(this.left.trySample(this.adapter, this.request)).willReturn(true); + given(this.right.trySample(this.adapter, this.request)).willReturn(true); + + then(this.sampler.trySample(this.adapter, this.request)).isTrue(); + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java index 7aca0177a..a41c0942a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java @@ -19,9 +19,12 @@ package org.springframework.cloud.sleuth.instrument.web; import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.regex.Pattern; import java.util.stream.Collectors; import brave.Tracing; +import brave.http.HttpAdapter; +import brave.http.HttpSampler; import brave.sampler.Sampler; import org.assertj.core.api.BDDAssertions; import org.junit.After; @@ -62,6 +65,7 @@ public class TraceFilterWebIntegrationTests { @Autowired Tracing tracer; @Autowired ArrayListSpanReporter accumulator; + @Autowired @ServerSampler HttpSampler sampler; @Autowired Environment environment; @Rule public OutputCapture capture = new OutputCapture(); @@ -115,6 +119,11 @@ public class TraceFilterWebIntegrationTests { then(this.accumulator.getSpans().get(0).tags()).containsEntry("http.status_code", "400"); } + @Test + public void should_inject_http_sampler() { + then(this.sampler).isNotNull(); + } + private int port() { return this.environment.getProperty("local.server.port", Integer.class); } @@ -135,6 +144,23 @@ public class TraceFilterWebIntegrationTests { return Sampler.ALWAYS_SAMPLE; } + // tag::custom_server_sampler[] + @Bean(name = ServerSampler.NAME) + HttpSampler myHttpSampler(SkipPatternProvider provider) { + Pattern pattern = provider.skipPattern(); + return new HttpSampler() { + + @Override public Boolean trySample(HttpAdapter adapter, Req request) { + String url = adapter.path(request); + boolean shouldSkip = pattern.matcher(url).matches(); + if (shouldSkip) { + return false; + } + return null; + } + }; + } + // end::custom_server_sampler[] @Bean RestTemplate restTemplate() { RestTemplate restTemplate = new RestTemplate();