Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update autoconfigure to append signal path to otlp http endpoint if n… #3666

Merged
merged 6 commits into from
Oct 5, 2021
Merged

Update autoconfigure to append signal path to otlp http endpoint if n… #3666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b3a1b7e
Update autoconfigure to append signal path to otlp http endpoint if n…
jack-berg Sep 27, 2021
0793d5f
Use URI.resolve() instead of string concatenation
jack-berg Sep 27, 2021
58c8d2a
revert to string concatenation
jack-berg Sep 28, 2021
ce32b38
Add URL validation to OTLP endpoint
jack-berg Sep 28, 2021
1557491
Always append signal path to otlp http/protobuf endpoint
jack-berg Oct 4, 2021
3887ef4
Add changelog entry, clarify tests, append root path if not included …
jack-berg Oct 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ you must update your build configuration to also include the new `jaeger-proto`
artifact will not be included in a future 2.0 release of the SDK so it is recommended to instead
generated the protobuf classes in your own build.

### Auto-configuration (alpha)

- BREAKING CHANGE: The behavior of `otel.exporter.otlp.endpoint` has changed when the protocol
is `http/protobuf`. The new behavior is in line
with [recent changes](https://github.com/open-telemetry/opentelemetry-specification/pull/1975) to
the specification, which states that the signal path (e.g. `v1/traces` or `v1/metrics`) is
appended to the configured endpoint. Values for signal specific endpoint configuration (
e.g. `otel.exporter.otlp.traces.endpoint` and `otel.exporter.otlp.metrics.endpoint`) override the
generic endpoint configuration and are used as-is without modification.

## Version 1.6.0 (2021-09-13):

### API
Expand Down
6 changes: 3 additions & 3 deletions sdk-extensions/autoconfigure/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ The [OpenTelemetry Protocol (OTLP)](https://github.com/open-telemetry/openteleme
|------------------------------|-----------------------------|---------------------------------------------------------------------------|
| otel.traces.exporter=otlp (default) | OTEL_TRACES_EXPORTER=otlp | Select the OpenTelemetry exporter for tracing (default) |
| otel.metrics.exporter=otlp | OTEL_METRICS_EXPORTER=otlp | Select the OpenTelemetry exporter for metrics |
| otel.exporter.otlp.endpoint | OTEL_EXPORTER_OTLP_ENDPOINT | The OTLP traces and metrics endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. Default is `http://localhost:4317`. |
| otel.exporter.otlp.traces.endpoint | OTEL_EXPORTER_OTLP_TRACES_ENDPOINT | The OTLP traces endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. Default is `http://localhost:4317`. |
| otel.exporter.otlp.metrics.endpoint | OTEL_EXPORTER_OTLP_METRICS_ENDPOINT | The OTLP metrics endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. Default is `http://localhost:4317`. |
| otel.exporter.otlp.endpoint | OTEL_EXPORTER_OTLP_ENDPOINT | The OTLP traces and metrics endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. If protocol is `http/protobuf` the version and signal will be appended to the path (e.g. `v1/traces` or `v1/metrics`). Default is `http://localhost:4317` when protocol is `grpc`, and `http://localhost:4317/v1/{signal}` when protocol is `http/protobuf`. |
| otel.exporter.otlp.traces.endpoint | OTEL_EXPORTER_OTLP_TRACES_ENDPOINT | The OTLP traces endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. Default is `http://localhost:4317` when protocol is `grpc`, and `http://localhost:4317/v1/traces` when protocol is `http/protobuf`. |
| otel.exporter.otlp.metrics.endpoint | OTEL_EXPORTER_OTLP_METRICS_ENDPOINT | The OTLP metrics endpoint to connect to. Must be a URL with a scheme of either `http` or `https` based on the use of TLS. Default is `http://localhost:4317` when protocol is `grpc`, and `http://localhost:4317/v1/metrics` when protocol is `http/protobuf`. |
| otel.exporter.otlp.certificate | OTEL_EXPORTER_OTLP_CERTIFICATE | The path to the file containing trusted certificates to use when verifying an OTLP trace or metric server's TLS credentials. The file should contain one or more X.509 certificates in PEM format. By default the host platform's trusted root certificates are used. |
| otel.exporter.otlp.traces.certificate | OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE | The path to the file containing trusted certificates to use when verifying an OTLP trace server's TLS credentials. The file should contain one or more X.509 certificates in PEM format. By default the host platform's trusted root certificates are used. |
| otel.exporter.otlp.metrics.certificate | OTEL_EXPORTER_OTLP_METRICS_CERTIFICATE | The path to the file containing trusted certificates to use when verifying an OTLP metric server's TLS credentials. The file should contain one or more X.509 certificates in PEM format. By default the host platform's trusted root certificates are used. |
Expand Down
6 changes: 4 additions & 2 deletions ...nfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/MetricExporterConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
package io.opentelemetry.sdk.autoconfigure;

import static io.opentelemetry.sdk.autoconfigure.OtlpConfigUtil.DATA_TYPE_METRICS;
import static io.opentelemetry.sdk.autoconfigure.OtlpConfigUtil.PROTOCOL_GRPC;
import static io.opentelemetry.sdk.autoconfigure.OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;

import io.opentelemetry.exporter.logging.LoggingMetricExporter;
import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporter;
Expand Down Expand Up @@ -82,7 +84,7 @@ static MetricExporter configureOtlpMetrics(
String protocol = OtlpConfigUtil.getOtlpProtocol(DATA_TYPE_METRICS, config);

MetricExporter exporter;
if (protocol.equals("http/protobuf")) {
if (protocol.equals(PROTOCOL_HTTP_PROTOBUF)) {
try {
ClasspathUtil.checkClassExists(
"io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporter",
Expand All @@ -104,7 +106,7 @@ static MetricExporter configureOtlpMetrics(
builder::setTrustedCertificates);

exporter = builder.build();
} else if (protocol.equals("grpc")) {
} else if (protocol.equals(PROTOCOL_GRPC)) {
try {
ClasspathUtil.checkClassExists(
"io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter",
Expand Down
81 changes: 76 additions & 5 deletions ...nsions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/OtlpConfigUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,23 @@
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import javax.annotation.Nullable;

final class OtlpConfigUtil {

static final String DATA_TYPE_TRACES = "traces";
static final String DATA_TYPE_METRICS = "metrics";
static final String PROTOCOL_GRPC = "grpc";
static final String PROTOCOL_HTTP_PROTOBUF = "http/protobuf";

static String getOtlpProtocol(String dataType, ConfigProperties config) {
String protocol = config.getString("otel.exporter.otlp." + dataType + ".protocol");
Expand All @@ -32,7 +37,7 @@ static String getOtlpProtocol(String dataType, ConfigProperties config) {
if (protocol == null) {
protocol = config.getString("otel.experimental.exporter.otlp.protocol");
}
return (protocol == null) ? "grpc" : protocol;
return (protocol == null) ? PROTOCOL_GRPC : protocol;
}

static void configureOtlpExporterBuilder(
Expand All @@ -43,12 +48,28 @@ static void configureOtlpExporterBuilder(
Consumer<String> setCompression,
Consumer<Duration> setTimeout,
Consumer<byte[]> setTrustedCertificates) {
String endpoint = config.getString("otel.exporter.otlp." + dataType + ".endpoint");
if (endpoint == null) {
endpoint = config.getString("otel.exporter.otlp.endpoint");
String protocol = getOtlpProtocol(dataType, config);
boolean isHttpProtobuf = protocol.equals(PROTOCOL_HTTP_PROTOBUF);
URL endpoint =
validateEndpoint(
config.getString("otel.exporter.otlp." + dataType + ".endpoint"), isHttpProtobuf);
if (endpoint != null) {
if (endpoint.getPath().isEmpty()) {
endpoint = createUrl(endpoint, "/");
}
} else {
endpoint = validateEndpoint(config.getString("otel.exporter.otlp.endpoint"), isHttpProtobuf);
if (endpoint != null && isHttpProtobuf) {
String path = endpoint.getPath();
if (!path.endsWith("/")) {
path += "/";
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a little liberty from the spec here by appending a / before the signal path if it doesn't exist.

A strict interpretation would cause http://localhost:4317/foo to be transformed to http://localhost:4317/foov1/traces

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's OK. If someone needs a path without slash (or something entirely different), they can configure it with the per-signal configuration.
Still, it should probably also be fixed in the spec.

path += signalPath(dataType);
endpoint = createUrl(endpoint, path);
}
}
if (endpoint != null) {
setEndpoint.accept(endpoint);
setEndpoint.accept(endpoint.toString());
}

Map<String, String> headers = config.getMap("otel.exporter.otlp." + dataType + ".headers");
Expand Down Expand Up @@ -92,5 +113,55 @@ static void configureOtlpExporterBuilder(
}
}

private static URL createUrl(URL context, String spec) {
try {
return new URL(context, spec);
} catch (MalformedURLException e) {
throw new ConfigurationException("Unexpected exception creating URL.", e);
}
}

@Nullable
private static URL validateEndpoint(@Nullable String endpoint, boolean allowPath) {
if (endpoint == null) {
return null;
}
URL endpointUrl;
try {
endpointUrl = new URL(endpoint);
} catch (MalformedURLException e) {
throw new ConfigurationException("OTLP endpoint must be a valid URL: " + endpoint, e);
}
if (!endpointUrl.getProtocol().equals("http") && !endpointUrl.getProtocol().equals("https")) {
throw new ConfigurationException(
"OTLP endpoint scheme must be http or https: " + endpointUrl.getProtocol());
}
if (endpointUrl.getQuery() != null) {
throw new ConfigurationException(
"OTLP endpoint must not have a query string: " + endpointUrl.getQuery());
}
if (endpointUrl.getRef() != null) {
throw new ConfigurationException(
"OTLP endpoint must not have a fragment: " + endpointUrl.getRef());
}
if (!allowPath && (!endpointUrl.getPath().isEmpty() && !endpointUrl.getPath().equals("/"))) {
throw new ConfigurationException(
"OTLP endpoint must not have a path: " + endpointUrl.getPath());
}
return endpointUrl;
}

private static String signalPath(String dataType) {
switch (dataType) {
case DATA_TYPE_METRICS:
return "v1/metrics";
case DATA_TYPE_TRACES:
return "v1/traces";
default:
throw new IllegalArgumentException(
"Cannot determine signal path for unrecognized data type: " + dataType);
}
}

private OtlpConfigUtil() {}
}
6 changes: 4 additions & 2 deletions ...configure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
package io.opentelemetry.sdk.autoconfigure;

import static io.opentelemetry.sdk.autoconfigure.OtlpConfigUtil.DATA_TYPE_TRACES;
import static io.opentelemetry.sdk.autoconfigure.OtlpConfigUtil.PROTOCOL_GRPC;
import static io.opentelemetry.sdk.autoconfigure.OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.joining;
Expand Down Expand Up @@ -107,7 +109,7 @@ static SpanExporter configureExporter(
static SpanExporter configureOtlp(ConfigProperties config) {
String protocol = OtlpConfigUtil.getOtlpProtocol(DATA_TYPE_TRACES, config);

if (protocol.equals("http/protobuf")) {
if (protocol.equals(PROTOCOL_HTTP_PROTOBUF)) {
ClasspathUtil.checkClassExists(
"io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter",
"OTLP HTTP Trace Exporter",
Expand All @@ -124,7 +126,7 @@ static SpanExporter configureOtlp(ConfigProperties config) {
builder::setTrustedCertificates);

return builder.build();
} else if (protocol.equals("grpc")) {
} else if (protocol.equals(PROTOCOL_GRPC)) {
ClasspathUtil.checkClassExists(
"io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter",
"OTLP gRPC Trace Exporter",
Expand Down
Loading