Remove support for schemaless URLs for gRPC exporters. (#2834)

This commit is contained in:
Anuraag Agrawal 2021-02-17 16:37:49 +09:00 committed by GitHub
parent 31a6da0bf5
commit 43f1ecf7ed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 40 additions and 69 deletions

View File

@ -14,15 +14,10 @@ import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.time.Duration; import java.time.Duration;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
/** Builder utility for this exporter. */ /** Builder utility for this exporter. */
public final class JaegerGrpcSpanExporterBuilder { 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 String DEFAULT_ENDPOINT_URL = "http://localhost:14250";
private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL); private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL);
private static final long DEFAULT_TIMEOUT_SECS = 10; private static final long DEFAULT_TIMEOUT_SECS = 10;
@ -49,9 +44,6 @@ public final class JaegerGrpcSpanExporterBuilder {
*/ */
public JaegerGrpcSpanExporterBuilder setEndpoint(String endpoint) { public JaegerGrpcSpanExporterBuilder setEndpoint(String endpoint) {
requireNonNull(endpoint, "endpoint"); requireNonNull(endpoint, "endpoint");
if (!endpoint.contains("://")) {
endpoint = "http://" + endpoint;
}
URI uri; URI uri;
try { try {
@ -59,18 +51,13 @@ public final class JaegerGrpcSpanExporterBuilder {
} catch (URISyntaxException e) { } catch (URISyntaxException e) {
throw new IllegalArgumentException("Invalid endpoint, must be a URL: " + endpoint, 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() == null
if (!uri.getScheme().equals("http") && !uri.getScheme().equals("https")) { || (!uri.getScheme().equals("http") && !uri.getScheme().equals("https"))) {
throw new IllegalArgumentException("Invalid scheme, must be http or https: " + endpoint); throw new IllegalArgumentException(
} "Invalid endpoint, must start with http:// or https://: " + uri);
} 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());
} }
this.endpoint = uri; this.endpoint = uri;
return this; return this;
} }

View File

@ -39,6 +39,7 @@ import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
import java.net.InetAddress; import java.net.InetAddress;
import java.net.URISyntaxException;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Optional; import java.util.Optional;
@ -290,12 +291,16 @@ class JaegerGrpcSpanExporterTest {
assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint(null)) assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint(null))
.isInstanceOf(NullPointerException.class) .isInstanceOf(NullPointerException.class)
.hasMessage("endpoint"); .hasMessage("endpoint");
assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("")) assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("😺://localhost"))
.isInstanceOf(IllegalArgumentException.class) .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")) assertThatThrownBy(() -> JaegerGrpcSpanExporter.builder().setEndpoint("gopher://localhost"))
.isInstanceOf(IllegalArgumentException.class) .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 { static class MockCollectorService extends CollectorServiceGrpc.CollectorServiceImplBase {

View File

@ -17,16 +17,11 @@ import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.time.Duration; import java.time.Duration;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable; import javax.annotation.Nullable;
/** Builder utility for this exporter. */ /** Builder utility for this exporter. */
public final class OtlpGrpcMetricExporterBuilder { 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 String DEFAULT_ENDPOINT_URL = "http://localhost:4317";
private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL); private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL);
private static final long DEFAULT_TIMEOUT_SECS = 10; private static final long DEFAULT_TIMEOUT_SECS = 10;
@ -75,24 +70,20 @@ public final class OtlpGrpcMetricExporterBuilder {
*/ */
public OtlpGrpcMetricExporterBuilder setEndpoint(String endpoint) { public OtlpGrpcMetricExporterBuilder setEndpoint(String endpoint) {
requireNonNull(endpoint, "endpoint"); requireNonNull(endpoint, "endpoint");
final URI uri;
URI uri;
try { try {
uri = new URI(endpoint); uri = new URI(endpoint);
} catch (URISyntaxException e) { } catch (URISyntaxException e) {
throw new IllegalArgumentException("Invalid endpoint, must be a URL: " + endpoint, 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() == null
if (!uri.getScheme().equals("http") && !uri.getScheme().equals("https")) { || (!uri.getScheme().equals("http") && !uri.getScheme().equals("https"))) {
throw new IllegalArgumentException("Invalid scheme, must be http or https: " + endpoint); throw new IllegalArgumentException(
} "Invalid endpoint, must start with http:// or https://: " + uri);
} 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());
} }
this.endpoint = uri; this.endpoint = uri;
return this; return this;
} }

View File

@ -87,9 +87,12 @@ class OtlpGrpcMetricExporterTest {
.isInstanceOf(IllegalArgumentException.class) .isInstanceOf(IllegalArgumentException.class)
.hasMessage("Invalid endpoint, must be a URL: 😺://localhost") .hasMessage("Invalid endpoint, must be a URL: 😺://localhost")
.hasCauseInstanceOf(URISyntaxException.class); .hasCauseInstanceOf(URISyntaxException.class);
assertThatThrownBy(() -> OtlpGrpcMetricExporter.builder().setEndpoint("ftp://localhost")) assertThatThrownBy(() -> OtlpGrpcMetricExporter.builder().setEndpoint("localhost"))
.isInstanceOf(IllegalArgumentException.class) .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 @Test

View File

@ -20,17 +20,12 @@ import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
import java.time.Duration; import java.time.Duration;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import javax.net.ssl.SSLException; import javax.net.ssl.SSLException;
/** Builder utility for this exporter. */ /** Builder utility for this exporter. */
public final class OtlpGrpcSpanExporterBuilder { 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 String DEFAULT_ENDPOINT_URL = "http://localhost:4317";
private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL); private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL);
private static final long DEFAULT_TIMEOUT_SECS = 10; private static final long DEFAULT_TIMEOUT_SECS = 10;
@ -79,9 +74,6 @@ public final class OtlpGrpcSpanExporterBuilder {
*/ */
public OtlpGrpcSpanExporterBuilder setEndpoint(String endpoint) { public OtlpGrpcSpanExporterBuilder setEndpoint(String endpoint) {
requireNonNull(endpoint, "endpoint"); requireNonNull(endpoint, "endpoint");
if (!endpoint.contains("://")) {
endpoint = "http://" + endpoint;
}
URI uri; URI uri;
try { try {
@ -89,18 +81,13 @@ public final class OtlpGrpcSpanExporterBuilder {
} catch (URISyntaxException e) { } catch (URISyntaxException e) {
throw new IllegalArgumentException("Invalid endpoint, must be a URL: " + endpoint, 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() == null
if (!uri.getScheme().equals("http") && !uri.getScheme().equals("https")) { || (!uri.getScheme().equals("http") && !uri.getScheme().equals("https"))) {
throw new IllegalArgumentException("Invalid scheme, must be http or https: " + endpoint); throw new IllegalArgumentException(
} "Invalid endpoint, must start with http:// or https://: " + uri);
} 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());
} }
this.endpoint = uri; this.endpoint = uri;
return this; return this;
} }

View File

@ -69,11 +69,6 @@ class OtlpGrpcSpanExporterTest {
closer.close(); closer.close();
} }
@Test
void legacyEndpointConfig() {
OtlpGrpcSpanExporter.builder().setEndpoint("localhost:4317");
}
@Test @Test
@SuppressWarnings("PreferJavaTimeOverload") @SuppressWarnings("PreferJavaTimeOverload")
void invalidConfig() { void invalidConfig() {
@ -90,12 +85,15 @@ class OtlpGrpcSpanExporterTest {
assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint(null)) assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint(null))
.isInstanceOf(NullPointerException.class) .isInstanceOf(NullPointerException.class)
.hasMessage("endpoint"); .hasMessage("endpoint");
assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("")) assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("😺://localhost"))
.isInstanceOf(IllegalArgumentException.class) .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")) assertThatThrownBy(() -> OtlpGrpcSpanExporter.builder().setEndpoint("gopher://localhost"))
.isInstanceOf(IllegalArgumentException.class) .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 @Test

View File

@ -57,7 +57,7 @@ class JaegerConfigTest {
@Test @Test
void configures() { void configures() {
String endpoint = "localhost:" + server.httpPort(); String endpoint = "http://localhost:" + server.httpPort();
System.setProperty("otel.exporter.jaeger.endpoint", endpoint); System.setProperty("otel.exporter.jaeger.endpoint", endpoint);