From 43f1ecf7ede49104c2aac1c9e5448b7cf7859ce7 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Wed, 17 Feb 2021 16:37:49 +0900 Subject: [PATCH] Remove support for schemaless URLs for gRPC exporters. (#2834) --- .../jaeger/JaegerGrpcSpanExporterBuilder.java | 25 +++++-------------- .../jaeger/JaegerGrpcSpanExporterTest.java | 11 +++++--- .../OtlpGrpcMetricExporterBuilder.java | 25 ++++++------------- .../metrics/OtlpGrpcMetricExporterTest.java | 7 ++++-- .../trace/OtlpGrpcSpanExporterBuilder.java | 25 +++++-------------- .../otlp/trace/OtlpGrpcSpanExporterTest.java | 14 +++++------ .../sdk/autoconfigure/JaegerConfigTest.java | 2 +- 7 files changed, 40 insertions(+), 69 deletions(-) diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java index c7ee329c6a..7feae5dc15 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java @@ -14,15 +14,10 @@ import java.net.URI; import java.net.URISyntaxException; import java.time.Duration; import java.util.concurrent.TimeUnit; -import java.util.logging.Level; -import java.util.logging.Logger; /** Builder utility for this exporter. */ public final class JaegerGrpcSpanExporterBuilder { - private static final Logger logger = - Logger.getLogger(JaegerGrpcSpanExporterBuilder.class.getName()); - private static final String DEFAULT_ENDPOINT_URL = "http://localhost:14250"; private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL); private static final long DEFAULT_TIMEOUT_SECS = 10; @@ -49,9 +44,6 @@ public final class JaegerGrpcSpanExporterBuilder { */ public JaegerGrpcSpanExporterBuilder setEndpoint(String endpoint) { requireNonNull(endpoint, "endpoint"); - if (!endpoint.contains("://")) { - endpoint = "http://" + endpoint; - } URI uri; try { @@ -59,18 +51,13 @@ public final class JaegerGrpcSpanExporterBuilder { } catch (URISyntaxException e) { throw new IllegalArgumentException("Invalid endpoint, must be a URL: " + endpoint, e); } - // TODO(anuraaga): Remove after announcing deprecation of schemaless URLs. - if (uri.getScheme() != null) { - if (!uri.getScheme().equals("http") && !uri.getScheme().equals("https")) { - throw new IllegalArgumentException("Invalid scheme, must be http or https: " + endpoint); - } - } else { - logger.log( - Level.WARNING, - "Endpoints must have a scheme of http:// or https://. Endpoints without schemes will " - + "not be permitted in a future version of the SDK.", - new Throwable()); + + if (uri.getScheme() == null + || (!uri.getScheme().equals("http") && !uri.getScheme().equals("https"))) { + throw new IllegalArgumentException( + "Invalid endpoint, must start with http:// or https://: " + uri); } + this.endpoint = uri; return this; } diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java index b4100dccb4..aefe6ff6fd 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java @@ -39,6 +39,7 @@ import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; import java.net.InetAddress; +import java.net.URISyntaxException; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -290,12 +291,16 @@ class JaegerGrpcSpanExporterTest { assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint(null)) .isInstanceOf(NullPointerException.class) .hasMessage("endpoint"); - assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("")) + assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("😺://localhost")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must be a URL: http://"); + .hasMessage("Invalid endpoint, must be a URL: 😺://localhost") + .hasCauseInstanceOf(URISyntaxException.class); + assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("localhost")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid endpoint, must start with http:// or https://: localhost"); assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("gopher://localhost")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid scheme, must be http or https: gopher://localhost"); + .hasMessage("Invalid endpoint, must start with http:// or https://: gopher://localhost"); } static class MockCollectorService extends CollectorServiceGrpc.CollectorServiceImplBase { diff --git a/exporters/otlp/metrics/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java b/exporters/otlp/metrics/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java index 6521fb536a..595f5d61ab 100644 --- a/exporters/otlp/metrics/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java +++ b/exporters/otlp/metrics/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java @@ -17,16 +17,11 @@ import java.net.URI; import java.net.URISyntaxException; import java.time.Duration; import java.util.concurrent.TimeUnit; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.annotation.Nullable; /** Builder utility for this exporter. */ public final class OtlpGrpcMetricExporterBuilder { - private static final Logger logger = - Logger.getLogger(OtlpGrpcMetricExporterBuilder.class.getName()); - private static final String DEFAULT_ENDPOINT_URL = "http://localhost:4317"; private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL); private static final long DEFAULT_TIMEOUT_SECS = 10; @@ -75,24 +70,20 @@ public final class OtlpGrpcMetricExporterBuilder { */ public OtlpGrpcMetricExporterBuilder setEndpoint(String endpoint) { requireNonNull(endpoint, "endpoint"); - final URI uri; + + URI uri; try { uri = new URI(endpoint); } catch (URISyntaxException e) { throw new IllegalArgumentException("Invalid endpoint, must be a URL: " + endpoint, e); } - // TODO(anuraaga): Remove after announcing deprecation of schemaless URLs. - if (uri.getScheme() != null) { - if (!uri.getScheme().equals("http") && !uri.getScheme().equals("https")) { - throw new IllegalArgumentException("Invalid scheme, must be http or https: " + endpoint); - } - } else { - logger.log( - Level.WARNING, - "Endpoints must have a scheme of http:// or https://. Endpoints without schemes will " - + "not be permitted in a future version of the SDK.", - new Throwable()); + + if (uri.getScheme() == null + || (!uri.getScheme().equals("http") && !uri.getScheme().equals("https"))) { + throw new IllegalArgumentException( + "Invalid endpoint, must start with http:// or https://: " + uri); } + this.endpoint = uri; return this; } diff --git a/exporters/otlp/metrics/src/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterTest.java b/exporters/otlp/metrics/src/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterTest.java index dd42eb6465..5b147997aa 100644 --- a/exporters/otlp/metrics/src/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterTest.java +++ b/exporters/otlp/metrics/src/test/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterTest.java @@ -87,9 +87,12 @@ class OtlpGrpcMetricExporterTest { .isInstanceOf(IllegalArgumentException.class) .hasMessage("Invalid endpoint, must be a URL: 😺://localhost") .hasCauseInstanceOf(URISyntaxException.class); - assertThatThrownBy(() -> OtlpGrpcMetricExporter.builder().setEndpoint("ftp://localhost")) + assertThatThrownBy(() -> OtlpGrpcMetricExporter.builder().setEndpoint("localhost")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid scheme, must be http or https: ftp://localhost"); + .hasMessage("Invalid endpoint, must start with http:// or https://: localhost"); + assertThatThrownBy(() -> OtlpGrpcMetricExporter.builder().setEndpoint("gopher://localhost")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid endpoint, must start with http:// or https://: gopher://localhost"); } @Test diff --git a/exporters/otlp/trace/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java b/exporters/otlp/trace/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java index 6d1d395d9b..aab2fc9449 100644 --- a/exporters/otlp/trace/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java +++ b/exporters/otlp/trace/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java @@ -20,17 +20,12 @@ import java.net.URI; import java.net.URISyntaxException; import java.time.Duration; import java.util.concurrent.TimeUnit; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.annotation.Nullable; import javax.net.ssl.SSLException; /** Builder utility for this exporter. */ public final class OtlpGrpcSpanExporterBuilder { - private static final Logger logger = - Logger.getLogger(OtlpGrpcSpanExporterBuilder.class.getName()); - private static final String DEFAULT_ENDPOINT_URL = "http://localhost:4317"; private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL); private static final long DEFAULT_TIMEOUT_SECS = 10; @@ -79,9 +74,6 @@ public final class OtlpGrpcSpanExporterBuilder { */ public OtlpGrpcSpanExporterBuilder setEndpoint(String endpoint) { requireNonNull(endpoint, "endpoint"); - if (!endpoint.contains("://")) { - endpoint = "http://" + endpoint; - } URI uri; try { @@ -89,18 +81,13 @@ public final class OtlpGrpcSpanExporterBuilder { } catch (URISyntaxException e) { throw new IllegalArgumentException("Invalid endpoint, must be a URL: " + endpoint, e); } - // TODO(anuraaga): Remove after announcing deprecation of schemaless URLs. - if (uri.getScheme() != null) { - if (!uri.getScheme().equals("http") && !uri.getScheme().equals("https")) { - throw new IllegalArgumentException("Invalid scheme, must be http or https: " + endpoint); - } - } else { - logger.log( - Level.WARNING, - "Endpoints must have a scheme of http:// or https://. Endpoints without schemes will " - + "not be permitted in a future version of the SDK.", - new Throwable()); + + if (uri.getScheme() == null + || (!uri.getScheme().equals("http") && !uri.getScheme().equals("https"))) { + throw new IllegalArgumentException( + "Invalid endpoint, must start with http:// or https://: " + uri); } + this.endpoint = uri; return this; } diff --git a/exporters/otlp/trace/src/test/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterTest.java b/exporters/otlp/trace/src/test/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterTest.java index e527cb236a..f5d3531f40 100644 --- a/exporters/otlp/trace/src/test/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterTest.java +++ b/exporters/otlp/trace/src/test/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterTest.java @@ -69,11 +69,6 @@ class OtlpGrpcSpanExporterTest { closer.close(); } - @Test - void legacyEndpointConfig() { - OtlpGrpcSpanExporter.builder().setEndpoint("localhost:4317"); - } - @Test @SuppressWarnings("PreferJavaTimeOverload") void invalidConfig() { @@ -90,12 +85,15 @@ class OtlpGrpcSpanExporterTest { assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint(null)) .isInstanceOf(NullPointerException.class) .hasMessage("endpoint"); - assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("")) + assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("😺://localhost")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must be a URL: http://"); + .hasMessage("Invalid endpoint, must be a URL: 😺://localhost"); + assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("localhost")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid endpoint, must start with http:// or https://: localhost"); assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("gopher://localhost")) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid scheme, must be http or https: gopher://localhost"); + .hasMessage("Invalid endpoint, must start with http:// or https://: gopher://localhost"); } @Test diff --git a/sdk-extensions/autoconfigure/src/testJaeger/java/io/opentelemetry/sdk/autoconfigure/JaegerConfigTest.java b/sdk-extensions/autoconfigure/src/testJaeger/java/io/opentelemetry/sdk/autoconfigure/JaegerConfigTest.java index f3c6baf25a..7c1b45805b 100644 --- a/sdk-extensions/autoconfigure/src/testJaeger/java/io/opentelemetry/sdk/autoconfigure/JaegerConfigTest.java +++ b/sdk-extensions/autoconfigure/src/testJaeger/java/io/opentelemetry/sdk/autoconfigure/JaegerConfigTest.java @@ -57,7 +57,7 @@ class JaegerConfigTest { @Test void configures() { - String endpoint = "localhost:" + server.httpPort(); + String endpoint = "http://localhost:" + server.httpPort(); System.setProperty("otel.exporter.jaeger.endpoint", endpoint);