Merge pull request #821 from zalando/bugfix/negative-port

Fixed -1 peer.port

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,34 +35,35 @@
 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)
 @AllArgsConstructor
 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
     @Singular
     ImList<Object> uriVariables;
 
     @Getter
-    @Wither
+    @With
     URI uri;
 
     BaseMap<Attribute<?>, Object> attributes;
@@ -76,19 +77,19 @@ final class DefaultRequestArguments implements RequestArguments {
     BaseMap<String, List<String>> 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);
     }
 

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 <code>peer.hostname</code> and <code>peer.port</code> 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);
+        }
     }
 
 }

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;

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<String, Object> tags = span.tags();
+
+        assertThat(tags, hasEntry("peer.hostname", "localhost"));
+        assertThat(tags, hasEntry("peer.address", "localhost"));
+        assertThat(tags, not(hasKey("peer.port")));
+    }
+
+}