From a0b1ee829b7746a6b52fc53f35338dbb1f643b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Willi=20Sch=C3=B6nborn?= Date: Wed, 13 Nov 2019 12:01:07 +0100 Subject: [PATCH] Fixed -1 peer.port --- .../riptide/DefaultRequestArguments.java | 21 +++++----- .../opentracing/span/PeerSpanDecorator.java | 21 +++++++--- .../opentracing/OpenTracingPluginTest.java | 2 - .../span/PeerSpanDecoratorTest.java | 38 +++++++++++++++++++ 4 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/span/PeerSpanDecoratorTest.java diff --git a/riptide-core/src/main/java/org/zalando/riptide/DefaultRequestArguments.java b/riptide-core/src/main/java/org/zalando/riptide/DefaultRequestArguments.java index f6dcb37c8..c51286b39 100644 --- a/riptide-core/src/main/java/org/zalando/riptide/DefaultRequestArguments.java +++ b/riptide-core/src/main/java/org/zalando/riptide/DefaultRequestArguments.java @@ -5,8 +5,8 @@ import lombok.AllArgsConstructor; import lombok.Getter; import lombok.Singular; +import lombok.With; import lombok.experimental.FieldDefaults; -import lombok.experimental.Wither; import org.apiguardian.api.API; import org.organicdesign.fp.collections.BaseMap; import org.organicdesign.fp.collections.ImList; @@ -35,6 +35,7 @@ import static org.springframework.web.util.UriUtils.encodeQueryParam; import static org.zalando.fauxpas.FauxPas.throwingBiConsumer; import static org.zalando.fauxpas.FauxPas.throwingConsumer; +import static org.zalando.riptide.UrlResolution.RFC; @API(status = INTERNAL) @FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) @@ -42,19 +43,19 @@ final class DefaultRequestArguments implements RequestArguments { @Getter - @Wither + @With HttpMethod method; @Getter - @Wither + @With URI baseUrl; @Getter - @Wither + @With UrlResolution urlResolution; @Getter - @Wither + @With String uriTemplate; @Getter @@ -62,7 +63,7 @@ final class DefaultRequestArguments implements RequestArguments { ImList uriVariables; @Getter - @Wither + @With URI uri; BaseMap, Object> attributes; @@ -76,19 +77,19 @@ final class DefaultRequestArguments implements RequestArguments { BaseMap> headers; @Getter - @Wither + @With Object body; @Getter - @Wither + @With Entity entity; @Getter - @Wither + @With Route route; DefaultRequestArguments() { - this(null, null, null, null, PersistentVector.empty(), null, PersistentHashMap.empty(), + this(null, null, RFC, null, PersistentVector.empty(), null, PersistentHashMap.empty(), PersistentHashMap.empty(), PersistentTreeMap.empty(CASE_INSENSITIVE_ORDER), null, null, null); } diff --git a/riptide-opentracing/src/main/java/org/zalando/riptide/opentracing/span/PeerSpanDecorator.java b/riptide-opentracing/src/main/java/org/zalando/riptide/opentracing/span/PeerSpanDecorator.java index 75f16ee9f..cf2230e0f 100644 --- a/riptide-opentracing/src/main/java/org/zalando/riptide/opentracing/span/PeerSpanDecorator.java +++ b/riptide-opentracing/src/main/java/org/zalando/riptide/opentracing/span/PeerSpanDecorator.java @@ -1,12 +1,14 @@ package org.zalando.riptide.opentracing.span; import io.opentracing.Span; -import io.opentracing.tag.Tags; import org.zalando.riptide.RequestArguments; -import org.zalando.riptide.opentracing.ExtensionTags; import java.net.URI; +import static io.opentracing.tag.Tags.PEER_HOSTNAME; +import static io.opentracing.tag.Tags.PEER_PORT; +import static org.zalando.riptide.opentracing.ExtensionTags.PEER_ADDRESS; + /** * Sets the peer.hostname and peer.port span tags. * @@ -16,10 +18,17 @@ public final class PeerSpanDecorator implements SpanDecorator { @Override public void onRequest(final Span span, final RequestArguments arguments) { - final URI requestUri = arguments.getRequestUri(); - span.setTag(ExtensionTags.PEER_ADDRESS, requestUri.getHost() + ":" + requestUri.getPort()); - span.setTag(Tags.PEER_HOSTNAME, requestUri.getHost()); - span.setTag(Tags.PEER_PORT, requestUri.getPort()); + final URI uri = arguments.getRequestUri(); + final int port = uri.getPort(); + + span.setTag(PEER_HOSTNAME, uri.getHost()); + + if (port == -1) { + span.setTag(PEER_ADDRESS, uri.getHost()); + } else { + span.setTag(PEER_ADDRESS, uri.getHost() + ":" + port); + span.setTag(PEER_PORT, port); + } } } diff --git a/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/OpenTracingPluginTest.java b/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/OpenTracingPluginTest.java index 96a0e44f8..35e3b1ac8 100644 --- a/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/OpenTracingPluginTest.java +++ b/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/OpenTracingPluginTest.java @@ -29,13 +29,11 @@ import static com.github.restdriver.clientdriver.RestClientDriver.giveEmptyResponse; import static com.github.restdriver.clientdriver.RestClientDriver.giveResponse; import static com.github.restdriver.clientdriver.RestClientDriver.onRequestTo; -import static com.google.common.collect.Iterables.getOnlyElement; import static java.util.Collections.singletonMap; import static java.util.concurrent.Executors.newSingleThreadExecutor; import static java.util.concurrent.TimeUnit.SECONDS; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; diff --git a/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/span/PeerSpanDecoratorTest.java b/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/span/PeerSpanDecoratorTest.java new file mode 100644 index 000000000..d6979d2e2 --- /dev/null +++ b/riptide-opentracing/src/test/java/org/zalando/riptide/opentracing/span/PeerSpanDecoratorTest.java @@ -0,0 +1,38 @@ +package org.zalando.riptide.opentracing.span; + +import io.opentracing.mock.MockSpan; +import io.opentracing.mock.MockTracer; +import org.junit.jupiter.api.Test; +import org.zalando.riptide.RequestArguments; + +import java.net.URI; +import java.util.Map; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.not; + +final class PeerSpanDecoratorTest { + + private final SpanDecorator unit = new PeerSpanDecorator(); + + @Test + void shouldIgnoreAbsentPort() { + final MockTracer tracer = new MockTracer(); + final MockSpan span = tracer.buildSpan("test").start(); + + final RequestArguments arguments = RequestArguments.create() + .withBaseUrl(URI.create("http://localhost")) + .withUriTemplate("/test"); + + unit.onRequest(span, arguments); + + final Map tags = span.tags(); + + assertThat(tags, hasEntry("peer.hostname", "localhost")); + assertThat(tags, hasEntry("peer.address", "localhost")); + assertThat(tags, not(hasKey("peer.port"))); + } + +}