From ab82f30623f0e577a3d3d84a6882bd762a352ecf Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 2 Apr 2019 14:10:36 +0200 Subject: [PATCH 1/2] Added a workaround for the mutability of native headers (#1323) Since for some reason, the native headers sometimes are immutable even though the accessor says that the headers are mutable, then we have to ensure their * mutability. We do so by first making a mutable copy of the native headers, then by removing the native headers from the headers map and replacing them with a mutable copy fixes #1184 --- .../messaging/MessageHeaderPropagation.java | 28 +++++++++++++++++-- .../client/HttpClientBeanPostProcessor.java | 3 +- src/checkstyle/checkstyle-suppressions.xml | 1 + 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagation.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagation.java index 2ac83145c..13ec0e0f8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagation.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/MessageHeaderPropagation.java @@ -83,7 +83,11 @@ enum MessageHeaderPropagation accessor.removeHeader(keyToRemove); if (accessor instanceof NativeMessageHeaderAccessor) { NativeMessageHeaderAccessor nativeAccessor = (NativeMessageHeaderAccessor) accessor; - nativeAccessor.removeNativeHeader(keyToRemove); + if (accessor.isMutable()) { + // 1184 native headers can be an immutable map + ensureNativeHeadersAreMutable(nativeAccessor) + .removeNativeHeader(keyToRemove); + } } else { Object nativeHeaders = accessor @@ -95,6 +99,26 @@ enum MessageHeaderPropagation } } + /** + * Since for some reason, the native headers sometimes are immutable even though the + * accessor says that the headers are mutable, then we have to ensure their + * mutability. We do so by first making a mutable copy of the native headers, then by + * removing the native headers from the headers map and replacing them with a mutable + * copy. Workaround for #1184 + * @param nativeAccessor accessor containing (or not) native headers + * @return modified accessor + */ + private static NativeMessageHeaderAccessor ensureNativeHeadersAreMutable( + NativeMessageHeaderAccessor nativeAccessor) { + Map> nativeHeaderMap = nativeAccessor.toNativeHeaderMap(); + nativeHeaderMap = nativeHeaderMap instanceof LinkedMultiValueMap ? nativeHeaderMap + : new LinkedMultiValueMap<>(nativeHeaderMap); + nativeAccessor.removeHeader(NativeMessageHeaderAccessor.NATIVE_HEADERS); + nativeAccessor.setHeader(NativeMessageHeaderAccessor.NATIVE_HEADERS, + nativeHeaderMap); + return nativeAccessor; + } + @Override public void put(MessageHeaderAccessor accessor, String key, String value) { try { @@ -116,7 +140,7 @@ enum MessageHeaderPropagation accessor.setHeader(key, value); if (accessor instanceof NativeMessageHeaderAccessor) { NativeMessageHeaderAccessor nativeAccessor = (NativeMessageHeaderAccessor) accessor; - nativeAccessor.setNativeHeader(key, value); + ensureNativeHeadersAreMutable(nativeAccessor).setNativeHeader(key, value); } else { Object nativeHeaders = accessor diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java index 9cf640e9a..096b4c9d8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/HttpClientBeanPostProcessor.java @@ -161,7 +161,8 @@ class HttpClientBeanPostProcessor implements BeanPostProcessor { AtomicReference reference = req.currentContext() .getOrDefault(AtomicReference.class, new AtomicReference()); Span span = handler().handleSend(injector(), req.requestHeaders(), req, - reference.get() == null ? handler().nextSpan(req) : (Span) reference.get()); + reference.get() == null ? handler().nextSpan(req) + : (Span) reference.get()); reference.set(span); } diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index 264dcd341..228a2c038 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -3,6 +3,7 @@ "-//Puppy Crawl//DTD Suppressions 1.1//EN" "https://www.puppycrawl.com/dtds/suppressions_1_1.dtd"> + From 3e1184e7f72b64ca99a202da200fc20d08e07f02 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 2 Apr 2019 14:37:37 +0200 Subject: [PATCH 2/2] Polish --- .mvn/wrapper/maven-wrapper.properties | 2 +- docs/src/main/asciidoc/ghpages.sh | 4 +- mvnw | 2 +- mvnw.cmd | 2 +- pom.xml | 6 + spring-cloud-sleuth-core/pom.xml | 5 + .../scheduling/TracingOnScheduledTests.java | 2 + .../resources/beans/applicationContext.xml | 164 +++++++++--------- 8 files changed, 100 insertions(+), 87 deletions(-) diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties index 66bfb1c18..406ac2077 100644 --- a/.mvn/wrapper/maven-wrapper.properties +++ b/.mvn/wrapper/maven-wrapper.properties @@ -5,7 +5,7 @@ # 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 +# https://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, diff --git a/docs/src/main/asciidoc/ghpages.sh b/docs/src/main/asciidoc/ghpages.sh index 4ff53f18e..ded935365 100755 --- a/docs/src/main/asciidoc/ghpages.sh +++ b/docs/src/main/asciidoc/ghpages.sh @@ -40,7 +40,7 @@ function check_if_anything_to_sync() { function retrieve_current_branch() { # Code getting the name of the current branch. For master we want to publish as we did until now - # http://stackoverflow.com/questions/1593051/how-to-programmatically-determine-the-current-checked-out-git-branch + # https://stackoverflow.com/questions/1593051/how-to-programmatically-determine-the-current-checked-out-git-branch # If there is a branch already passed will reuse it - otherwise will try to find it CURRENT_BRANCH=${BRANCH} if [[ -z "${CURRENT_BRANCH}" ]] ; then @@ -147,7 +147,7 @@ function copy_docs_for_current_version() { COMMIT_CHANGES="yes" else echo -e "Current branch is [${CURRENT_BRANCH}]" - # http://stackoverflow.com/questions/29300806/a-bash-script-to-check-if-a-string-is-present-in-a-comma-separated-list-of-strin + # https://stackoverflow.com/questions/29300806/a-bash-script-to-check-if-a-string-is-present-in-a-comma-separated-list-of-strin if [[ ",${WHITELISTED_BRANCHES_VALUE}," = *",${CURRENT_BRANCH},"* ]] ; then mkdir -p ${ROOT_FOLDER}/${CURRENT_BRANCH} echo -e "Branch [${CURRENT_BRANCH}] is whitelisted! Will copy the current docs to the [${CURRENT_BRANCH}] folder" diff --git a/mvnw b/mvnw index 25f750cb4..a08b219e3 100755 --- a/mvnw +++ b/mvnw @@ -8,7 +8,7 @@ # "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 +# https://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 diff --git a/mvnw.cmd b/mvnw.cmd index b0dc0e7e9..7ecd01d80 100644 --- a/mvnw.cmd +++ b/mvnw.cmd @@ -7,7 +7,7 @@ @REM "License"); you may not use this file except in compliance @REM with the License. You may obtain a copy of the License at @REM -@REM http://www.apache.org/licenses/LICENSE-2.0 +@REM https://www.apache.org/licenses/LICENSE-2.0 @REM @REM Unless required by applicable law or agreed to in writing, @REM software distributed under the License is distributed on an diff --git a/pom.xml b/pom.xml index 016a51174..9f80d2eda 100644 --- a/pom.xml +++ b/pom.xml @@ -244,6 +244,12 @@ 3.8.0 test + + net.jcip + jcip-annotations + 1.0 + test + diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index 4df8d6814..deb9b763c 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -340,6 +340,11 @@ 20.0 test + + net.jcip + jcip-annotations + test + diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java index b3faf4ee5..9a385aa81 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java @@ -22,6 +22,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import brave.Span; import brave.Tracing; import brave.sampler.Sampler; +import net.jcip.annotations.NotThreadSafe; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.Before; @@ -47,6 +48,7 @@ import static org.awaitility.Awaitility.await; @RunWith(SpringRunner.class) @SpringBootTest(classes = { ScheduledTestConfiguration.class }) @DirtiesContext +@NotThreadSafe public class TracingOnScheduledTests { @Autowired diff --git a/spring-cloud-sleuth-core/src/test/resources/beans/applicationContext.xml b/spring-cloud-sleuth-core/src/test/resources/beans/applicationContext.xml index e5400eddd..30df663a8 100644 --- a/spring-cloud-sleuth-core/src/test/resources/beans/applicationContext.xml +++ b/spring-cloud-sleuth-core/src/test/resources/beans/applicationContext.xml @@ -1,82 +1,82 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +