2.x: fix set collectorUri() with URL with no port number adds port

tracing/tracing/src/main/java/io/helidon/tracing/TracerBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -162,7 +162,8 @@ default T collectorUri(URI uri) {
             result = result.collectorPath(uri.getPath());
         }
 
-        if (uri.getPort() > -1) {
+        /* Allow -1 as it means no port specified */
+        if (uri.getPort() >= -1) {
             result = result.collectorPort(uri.getPort());
         }
 

tracing/zipkin/src/test/java/io/helidon/tracing/zipkin/ZipkinTracerBuilderTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,12 +16,12 @@
 
 package io.helidon.tracing.zipkin;
 
+import java.net.URI;
 import java.util.List;
 
 import io.helidon.config.Config;
 import io.helidon.tracing.Tag;
 import io.helidon.tracing.TracerBuilder;
-
 import io.opentracing.Span;
 import io.opentracing.Tracer;
 import io.opentracing.noop.NoopTracer;
@@ -31,6 +31,7 @@
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -64,6 +65,29 @@ void testConfigDefaults() {
         assertThat(zBuilder.isEnabled(), is(ZipkinTracerBuilder.DEFAULT_ENABLED));
     }
 
+    @Test
+    void testConfigSuppressPort() {
+        /* Make sure if config sets port to -1 that we do not default it to something else */
+        TracerBuilder<?> builder = TracerBuilder.create(config.get("tracing.zipkin-defaults-suppress-port"));
+        ZipkinTracerBuilder zBuilder = (ZipkinTracerBuilder) builder;
+        assertThat(zBuilder.port(), is(-1));
+
+        Tracer tracer = zBuilder.build();
+        assertThat(tracer, notNullValue());
+    }
+
+    @Test
+    void testConfigSuppressPortUri() {
+        /* Create builder using Uri with no port number. Make sure we don't add a port number */
+        TracerBuilder<?> builder = TracerBuilder.create("unit-test-suppress-port-uri")
+                .collectorUri(URI.create("https://localhost/path"));
+        ZipkinTracerBuilder zBuilder = (ZipkinTracerBuilder) builder;
+        assertThat(zBuilder.port(), is(-1));
+
+        Tracer tracer = zBuilder.build();
+        assertThat(tracer, notNullValue());
+    }
+
     @Test
     void testConfigDisabled() {
         TracerBuilder<?> builder = TracerBuilder.create(config.get("tracing.zipkin-disabled"));

tracing/zipkin/src/test/resources/application.yaml

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2017, 2021 Oracle and/or its affiliates.
+# Copyright (c) 2017, 2022 Oracle and/or its affiliates.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -17,6 +17,9 @@
 tracing:
   zipkin-defaults:
     service: "helidon-service"
+  zipkin-defaults-suppress-port:
+    service: "helidon-service"
+    port: -1
   zipkin-disabled:
     enabled: false
     service: "helidon-service"