From 61739bd9175d5756fbcf24564ff1e15c9510edab Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Thu, 31 Aug 2023 09:51:41 +0200 Subject: [PATCH] Prevent propagation for local fields when using W3C propagation Closes gh-37109 --- .../tracing/BraveAutoConfiguration.java | 30 ++++++-- .../tracing/CompositePropagationFactory.java | 28 +++++-- .../tracing/LocalBaggageFields.java | 77 +++++++++++++++++++ .../tracing/LocalBaggageFieldsTests.java | 63 +++++++++++++++ 4 files changed, 187 insertions(+), 11 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFields.java create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFieldsTests.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.java index a52d6167e1..4a3291a887 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/BraveAutoConfiguration.java @@ -37,7 +37,9 @@ import brave.handler.SpanHandler; import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.ScopeDecorator; import brave.propagation.CurrentTraceContextCustomizer; +import brave.propagation.Propagation; import brave.propagation.Propagation.Factory; +import brave.propagation.Propagation.KeyFactory; import brave.propagation.ThreadLocalCurrentTraceContext; import brave.sampler.Sampler; import io.micrometer.tracing.brave.bridge.BraveBaggageManager; @@ -168,7 +170,7 @@ public class BraveAutoConfiguration { @Bean @ConditionalOnMissingBean Factory propagationFactory(TracingProperties properties) { - return CompositePropagationFactory.create(properties.getPropagation(), null); + return CompositePropagationFactory.create(properties.getPropagation()); } } @@ -187,13 +189,31 @@ public class BraveAutoConfiguration { @ConditionalOnMissingBean BaggagePropagation.FactoryBuilder propagationFactoryBuilder( ObjectProvider baggagePropagationCustomizers) { - CompositePropagationFactory factory = CompositePropagationFactory - .create(this.tracingProperties.getPropagation(), BRAVE_BAGGAGE_MANAGER); - FactoryBuilder builder = BaggagePropagation.newFactoryBuilder(factory); - baggagePropagationCustomizers.orderedStream().forEach((customizer) -> customizer.customize(builder)); + // There's a chicken-and-egg problem here: to create a builder, we need a + // factory. But the CompositePropagationFactory needs data from the builder. + // We create a throw-away builder with a throw-away factory, and then copy the + // config to the real builder + FactoryBuilder throwAwayBuilder = BaggagePropagation.newFactoryBuilder(createThrowAwayFactory()); + baggagePropagationCustomizers.orderedStream() + .forEach((customizer) -> customizer.customize(throwAwayBuilder)); + CompositePropagationFactory propagationFactory = CompositePropagationFactory.create( + this.tracingProperties.getPropagation(), BRAVE_BAGGAGE_MANAGER, + LocalBaggageFields.extractFrom(throwAwayBuilder)); + FactoryBuilder builder = BaggagePropagation.newFactoryBuilder(propagationFactory); + throwAwayBuilder.configs().forEach(builder::add); return builder; } + @SuppressWarnings("deprecation") + private Factory createThrowAwayFactory() { + return new Factory() { + @Override + public Propagation create(KeyFactory keyFactory) { + return null; + } + }; + } + @Bean @Order(0) BaggagePropagationCustomizer remoteFieldsBaggagePropagationCustomizer() { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactory.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactory.java index 0ec3db30c2..5d7722554c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactory.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/CompositePropagationFactory.java @@ -17,7 +17,6 @@ package org.springframework.boot.actuate.autoconfigure.tracing; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.function.Predicate; import java.util.stream.Stream; @@ -85,15 +84,27 @@ class CompositePropagationFactory extends Propagation.Factory { .orElse(context); } + /** + * Creates a new {@link CompositePropagationFactory}, which uses the given + * {@code injectionTypes} for injection and {@code extractionTypes} for extraction. + * @param properties the propagation properties + * @return the {@link CompositePropagationFactory} + */ + static CompositePropagationFactory create(TracingProperties.Propagation properties) { + return create(properties, null, null); + } + /** * Creates a new {@link CompositePropagationFactory}, which uses the given * {@code injectionTypes} for injection and {@code extractionTypes} for extraction. * @param properties the propagation properties * @param baggageManager the baggage manager to use, or {@code null} + * @param localFields the local fields, or {@code null} * @return the {@link CompositePropagationFactory} */ - static CompositePropagationFactory create(TracingProperties.Propagation properties, BaggageManager baggageManager) { - PropagationFactoryMapper mapper = new PropagationFactoryMapper(baggageManager); + static CompositePropagationFactory create(TracingProperties.Propagation properties, BaggageManager baggageManager, + LocalBaggageFields localFields) { + PropagationFactoryMapper mapper = new PropagationFactoryMapper(baggageManager, localFields); List injectors = properties.getEffectiveProducedTypes().stream().map(mapper::map).toList(); List extractors = properties.getEffectiveConsumedTypes().stream().map(mapper::map).toList(); return new CompositePropagationFactory(injectors, extractors); @@ -107,8 +118,11 @@ class CompositePropagationFactory extends Propagation.Factory { private final BaggageManager baggageManager; - PropagationFactoryMapper(BaggageManager baggageManager) { + private final LocalBaggageFields localFields; + + PropagationFactoryMapper(BaggageManager baggageManager, LocalBaggageFields localFields) { this.baggageManager = baggageManager; + this.localFields = (localFields != null) ? localFields : LocalBaggageFields.empty(); } Propagation.Factory map(PropagationType type) { @@ -140,8 +154,10 @@ class CompositePropagationFactory extends Propagation.Factory { * @return the W3C propagation factory */ private Propagation.Factory w3c() { - return (this.baggageManager != null) ? new W3CPropagation(this.baggageManager, Collections.emptyList()) - : new W3CPropagation(); + if (this.baggageManager == null) { + return new W3CPropagation(); + } + return new W3CPropagation(this.baggageManager, this.localFields.asList()); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFields.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFields.java new file mode 100644 index 0000000000..dfd3f495ae --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFields.java @@ -0,0 +1,77 @@ +/* + * Copyright 2012-2023 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 + * + * 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, + * 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.boot.actuate.autoconfigure.tracing; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import brave.baggage.BaggagePropagation; +import brave.baggage.BaggagePropagationConfig; +import brave.baggage.BaggagePropagationConfig.SingleBaggageField; + +import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; + +/** + * Local baggage fields. + * + * @author Moritz Halbritter + */ +class LocalBaggageFields { + + private final List fields; + + LocalBaggageFields(List fields) { + Assert.notNull(fields, "fields must not be null"); + this.fields = fields; + } + + /** + * Returns the local fields as a list. + * @return the list + */ + List asList() { + return Collections.unmodifiableList(this.fields); + } + + /** + * Extracts the local fields from the given propagation factory builder. + * @param builder the propagation factory builder to extract the local fields from + * @return the local fields + */ + static LocalBaggageFields extractFrom(BaggagePropagation.FactoryBuilder builder) { + List localFields = new ArrayList<>(); + for (BaggagePropagationConfig config : builder.configs()) { + if (config instanceof SingleBaggageField field) { + if (CollectionUtils.isEmpty(field.keyNames())) { + localFields.add(field.field().name()); + } + } + } + return new LocalBaggageFields(localFields); + } + + /** + * Creates empty local fields. + * @return the empty local fields + */ + static LocalBaggageFields empty() { + return new LocalBaggageFields(Collections.emptyList()); + } + +} diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFieldsTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFieldsTests.java new file mode 100644 index 0000000000..6f6e99d87d --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/LocalBaggageFieldsTests.java @@ -0,0 +1,63 @@ +/* + * Copyright 2012-2023 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 + * + * 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, + * 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.boot.actuate.autoconfigure.tracing; + +import brave.baggage.BaggageField; +import brave.baggage.BaggagePropagation; +import brave.baggage.BaggagePropagation.FactoryBuilder; +import brave.baggage.BaggagePropagationConfig; +import brave.propagation.Propagation; +import brave.propagation.Propagation.Factory; +import brave.propagation.Propagation.KeyFactory; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link LocalBaggageFields}. + * + * @author Moritz Halbritter + */ +class LocalBaggageFieldsTests { + + @Test + void extractFromBuilder() { + FactoryBuilder builder = createBuilder(); + builder.add(BaggagePropagationConfig.SingleBaggageField.remote(BaggageField.create("remote-field-1"))); + builder.add(BaggagePropagationConfig.SingleBaggageField.remote(BaggageField.create("remote-field-2"))); + builder.add(BaggagePropagationConfig.SingleBaggageField.local(BaggageField.create("local-field-1"))); + builder.add(BaggagePropagationConfig.SingleBaggageField.local(BaggageField.create("local-field-2"))); + LocalBaggageFields fields = LocalBaggageFields.extractFrom(builder); + assertThat(fields.asList()).containsExactlyInAnyOrder("local-field-1", "local-field-2"); + } + + @Test + void empty() { + assertThat(LocalBaggageFields.empty().asList()).isEmpty(); + } + + @SuppressWarnings("deprecation") + private static FactoryBuilder createBuilder() { + return BaggagePropagation.newFactoryBuilder(new Factory() { + @Override + public Propagation create(KeyFactory keyFactory) { + return null; + } + }); + } + +}