From e6fe273105bc5b8e714fdaa9f8612a71df9f3778 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 21 Feb 2024 16:06:02 -0800 Subject: [PATCH] Polish --- .../zipkin/ZipkinConnectionDetails.java | 1 - .../zipkin/ZipkinHttpClientSender.java | 2 +- .../tracing/zipkin/ZipkinProperties.java | 1 + .../zipkin/DefaultEncodingConfiguration.java | 2 +- .../zipkin/TestHttpEndpointSupplier.java | 47 +++++++++++++++++++ ...ConfigurationsBraveConfigurationTests.java | 28 +++++------ .../zipkin/ZipkinHttpClientSenderTests.java | 24 ++++------ .../zipkin/ZipkinRestTemplateSenderTests.java | 19 ++------ .../zipkin/ZipkinWebClientSenderTests.java | 26 +++------- 9 files changed, 81 insertions(+), 69 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/TestHttpEndpointSupplier.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConnectionDetails.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConnectionDetails.java index ed1eb7bc0a5..9d27cc70e7e 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConnectionDetails.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConnectionDetails.java @@ -22,7 +22,6 @@ import org.springframework.boot.autoconfigure.service.connection.ConnectionDetai /** * Details required to establish a connection to a Zipkin server. - * *

* Note: {@linkplain #getSpanEndpoint()} is only read once and passed to a bean of type * {@link HttpEndpointSupplier.Factory} which defaults to no-op (constant). diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSender.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSender.java index c1cc60631d6..2f982a54da2 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSender.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSender.java @@ -55,7 +55,7 @@ class ZipkinHttpClientSender extends HttpSender { .POST(BodyPublishers.ofByteArray(body)) .uri(endpoint) .timeout(this.readTimeout); - headers.forEach((key, values) -> values.forEach((value) -> request.header(key, value))); + headers.forEach((name, values) -> values.forEach((value) -> request.header(name, value))); try { HttpResponse response = this.httpClient.send(request.build(), BodyHandlers.discarding()); if (response.statusCode() / 100 != 2) { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinProperties.java index d525162ad93..9245edaeb6e 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinProperties.java @@ -90,6 +90,7 @@ public class ZipkinProperties { * JSON. */ JSON, + /** * Protocol Buffers v3. */ diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/DefaultEncodingConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/DefaultEncodingConfiguration.java index 7cf98ccd8c6..b1551b95cc7 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/DefaultEncodingConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/DefaultEncodingConfiguration.java @@ -30,7 +30,7 @@ class DefaultEncodingConfiguration { @Bean @ConditionalOnMissingBean - Encoding encoding() { + Encoding zipkinReporterEncoding() { return Encoding.JSON; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/TestHttpEndpointSupplier.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/TestHttpEndpointSupplier.java new file mode 100644 index 00000000000..27f20a07747 --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/TestHttpEndpointSupplier.java @@ -0,0 +1,47 @@ +/* + * Copyright 2012-2024 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.zipkin; + +import java.util.concurrent.atomic.AtomicInteger; + +import zipkin2.reporter.HttpEndpointSupplier; + +/** + * Test {@link HttpEndpointSupplier}. + * + * @author Moritz Halbritter + */ +class TestHttpEndpointSupplier implements HttpEndpointSupplier { + + private final String url; + + private final AtomicInteger suffix = new AtomicInteger(); + + TestHttpEndpointSupplier(String url) { + this.url = url; + } + + @Override + public String get() { + return this.url + "/" + this.suffix.incrementAndGet(); + } + + @Override + public void close() { + } + +} diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsBraveConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsBraveConfigurationTests.java index d1a6dd20ffe..e2dee923a24 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsBraveConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinConfigurationsBraveConfigurationTests.java @@ -72,7 +72,6 @@ class ZipkinConfigurationsBraveConfigurationTests { this.contextRunner.withClassLoader(new FilteredClassLoader("zipkin2.reporter.brave")) .withUserConfiguration(SenderConfiguration.class) .run((context) -> assertThat(context).doesNotHaveBean(AsyncZipkinSpanHandler.class)); - } @Test @@ -128,13 +127,7 @@ class ZipkinConfigurationsBraveConfigurationTests { this.contextRunner.withUserConfiguration(SenderConfiguration.class).run((context) -> { @SuppressWarnings("unchecked") BytesEncoder encoder = context.getBean(BytesEncoder.class); - - MutableSpan span = new MutableSpan(); - span.traceId("1"); - span.id("1"); - span.tag("error", "true"); - span.error(new RuntimeException("ice cream")); - + MutableSpan span = createTestSpan(); // default tag key name is "error", and doesn't overwrite assertThat(new String(encoder.encode(span), StandardCharsets.UTF_8)).isEqualTo( "{\"traceId\":\"0000000000000001\",\"id\":\"0000000000000001\",\"tags\":{\"error\":\"true\"}}"); @@ -147,19 +140,22 @@ class ZipkinConfigurationsBraveConfigurationTests { .run((context) -> { @SuppressWarnings("unchecked") BytesEncoder encoder = context.getBean(BytesEncoder.class); - - MutableSpan span = new MutableSpan(); - span.traceId("1"); - span.id("1"); - span.tag("error", "true"); - span.error(new RuntimeException("ice cream")); - + MutableSpan span = createTestSpan(); // The custom throwable parser doesn't use the key "error" we can see both assertThat(new String(encoder.encode(span), StandardCharsets.UTF_8)).isEqualTo( "{\"traceId\":\"0000000000000001\",\"id\":\"0000000000000001\",\"tags\":{\"error\":\"true\",\"exception\":\"ice cream\"}}"); }); } + private MutableSpan createTestSpan() { + MutableSpan span = new MutableSpan(); + span.traceId("1"); + span.id("1"); + span.tag("error", "true"); + span.error(new RuntimeException("ice cream")); + return span; + } + @Configuration(proxyBeanMethods = false) private static final class SenderConfiguration { @@ -185,7 +181,7 @@ class ZipkinConfigurationsBraveConfigurationTests { @Bean Tag throwableTag() { - return new Tag("exception") { + return new Tag<>("exception") { @Override protected String parseValue(Throwable throwable, TraceContext traceContext) { return throwable.getMessage(); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSenderTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSenderTests.java index 8a1f52b0c5c..bdcfa4884c9 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSenderTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinHttpClientSenderTests.java @@ -24,7 +24,6 @@ import java.util.Base64; import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import okhttp3.mockwebserver.MockResponse; @@ -57,7 +56,7 @@ class ZipkinHttpClientSenderTests extends ZipkinHttpSenderTests { private static MockWebServer mockBackEnd; - private static String zipkinUrl; + static String zipkinUrl; @BeforeAll static void beforeAll() throws IOException { @@ -130,22 +129,15 @@ class ZipkinHttpClientSenderTests extends ZipkinHttpSenderTests { void sendUsesDynamicEndpoint() throws Exception { mockBackEnd.enqueue(new MockResponse()); mockBackEnd.enqueue(new MockResponse()); - AtomicInteger suffix = new AtomicInteger(); - try (BytesMessageSender sender = createSender((e) -> new HttpEndpointSupplier() { - @Override - public String get() { - return zipkinUrl + "/" + suffix.incrementAndGet(); + try (TestHttpEndpointSupplier httpEndpointSupplier = new TestHttpEndpointSupplier(zipkinUrl)) { + try (BytesMessageSender sender = createSender((endpoint) -> httpEndpointSupplier, Encoding.JSON, + Duration.ofSeconds(10))) { + sender.send(Collections.emptyList()); + sender.send(Collections.emptyList()); } - - @Override - public void close() { - } - }, Encoding.JSON, Duration.ofSeconds(10))) { - sender.send(Collections.emptyList()); - sender.send(Collections.emptyList()); + assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/1"); + assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/2"); } - assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/1"); - assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/2"); } @Test diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinRestTemplateSenderTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinRestTemplateSenderTests.java index dff4fab42eb..7e891d7138c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinRestTemplateSenderTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinRestTemplateSenderTests.java @@ -21,7 +21,6 @@ import java.net.URI; import java.util.Base64; import java.util.Collections; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -97,7 +96,6 @@ class ZipkinRestTemplateSenderTests extends ZipkinHttpSenderTests { .andExpect(content().contentType("application/x-protobuf")) .andExpect(content().string("span1span2")) .andRespond(withStatus(HttpStatus.ACCEPTED)); - try (BytesMessageSender sender = createSender(Encoding.PROTO3)) { sender.send(List.of(toByteArray("span1"), toByteArray("span2"))); } @@ -111,20 +109,11 @@ class ZipkinRestTemplateSenderTests extends ZipkinHttpSenderTests { void sendUsesDynamicEndpoint() throws Exception { this.mockServer.expect(requestTo(ZIPKIN_URL + "/1")).andRespond(withStatus(HttpStatus.ACCEPTED)); this.mockServer.expect(requestTo(ZIPKIN_URL + "/2")).andRespond(withStatus(HttpStatus.ACCEPTED)); - - AtomicInteger suffix = new AtomicInteger(); - try (BytesMessageSender sender = createSender((e) -> new HttpEndpointSupplier() { - @Override - public String get() { - return ZIPKIN_URL + "/" + suffix.incrementAndGet(); + try (HttpEndpointSupplier httpEndpointSupplier = new TestHttpEndpointSupplier(ZIPKIN_URL)) { + try (BytesMessageSender sender = createSender((endpoint) -> httpEndpointSupplier, Encoding.JSON)) { + sender.send(Collections.emptyList()); + sender.send(Collections.emptyList()); } - - @Override - public void close() { - } - }, Encoding.JSON)) { - sender.send(Collections.emptyList()); - sender.send(Collections.emptyList()); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinWebClientSenderTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinWebClientSenderTests.java index 07028484b04..c917e9f346c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinWebClientSenderTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/tracing/zipkin/ZipkinWebClientSenderTests.java @@ -24,7 +24,6 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import okhttp3.mockwebserver.MockResponse; @@ -113,11 +112,9 @@ class ZipkinWebClientSenderTests extends ZipkinHttpSenderTests { void sendShouldSendSpansToZipkinInProto3() throws IOException, InterruptedException { mockBackEnd.enqueue(new MockResponse()); List encodedSpans = List.of(toByteArray("span1"), toByteArray("span2")); - try (BytesMessageSender sender = createSender(Encoding.PROTO3, Duration.ofSeconds(10))) { sender.send(encodedSpans); } - requestAssertions((request) -> { assertThat(request.getMethod()).isEqualTo("POST"); assertThat(request.getHeader("Content-Type")).isEqualTo("application/x-protobuf"); @@ -133,24 +130,15 @@ class ZipkinWebClientSenderTests extends ZipkinHttpSenderTests { void sendUsesDynamicEndpoint() throws Exception { mockBackEnd.enqueue(new MockResponse()); mockBackEnd.enqueue(new MockResponse()); - - AtomicInteger suffix = new AtomicInteger(); - try (BytesMessageSender sender = createSender((e) -> new HttpEndpointSupplier() { - @Override - public String get() { - return ZIPKIN_URL + "/" + suffix.incrementAndGet(); + try (HttpEndpointSupplier httpEndpointSupplier = new TestHttpEndpointSupplier(ZIPKIN_URL)) { + try (BytesMessageSender sender = createSender((endpoint) -> httpEndpointSupplier, Encoding.JSON, + Duration.ofSeconds(10))) { + sender.send(Collections.emptyList()); + sender.send(Collections.emptyList()); } - - @Override - public void close() { - } - }, Encoding.JSON, Duration.ofSeconds(10))) { - sender.send(Collections.emptyList()); - sender.send(Collections.emptyList()); + assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/1"); + assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/2"); } - - assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/1"); - assertThat(mockBackEnd.takeRequest().getPath()).endsWith("/2"); } @Test