Skip to content

Commit

Permalink
Warn on potentially wrong endpoint configuration (#1928)
Browse files Browse the repository at this point in the history
* Warn on potentially wrong endpoint configuration

* spotless

* address review comments
  • Loading branch information
laurit authored Jul 10, 2024
1 parent b4404ea commit b98bf38
Show file tree
Hide file tree
Showing 4 changed files with 241 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
* Copyright Splunk Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.splunk.opentelemetry;

import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Map;
import java.util.function.Consumer;

class EndpointProtocolValidator {

private static final int OTLP_GRPC_PORT = 4317;
private static final int OTLP_HTTP_PROTOBUF_PORT = 4318;

private final Map<String, String> customized;
private final ConfigProperties config;
private final Consumer<String> warn;

private EndpointProtocolValidator(
Map<String, String> customized, ConfigProperties config, Consumer<String> warn) {
this.customized = customized;
this.config = config;
this.warn = warn;
}

/** Warn when the port number in OTLP endpoint doesn't agree with configured protocol. */
static void validate(
Map<String, String> customized, ConfigProperties config, Consumer<String> warn) {
EndpointProtocolValidator validator = new EndpointProtocolValidator(customized, config, warn);
validator.verifyEndpointProtocol();
}

private void verifyEndpointProtocol() {
String protocol = getString("otel.exporter.otlp.protocol", "http/protobuf");
String endpoint =
getString("otel.exporter.otlp.endpoint", "http://localhost:" + getPort(protocol));
if (!verifyEndpointProtocol(
protocol, endpoint, "otel.exporter.otlp.protocol", "otel.exporter.otlp.endpoint")) {
// there already was a mismatch between endpoint port and protocol,
// skip looking at signal specific endpoints
return;
}

for (String signal : new String[] {"traces", "metrics", "logs"}) {
verifySignalEndpointProtocol(signal, protocol, endpoint);
}
}

private boolean verifyEndpointProtocol(
String protocol, String endpoint, String protocolKey, String endpointKey) {
int port = getEndpointPort(endpoint);
if ("http/protobuf".equals(protocol)) {
if (port == OTLP_GRPC_PORT) {
warn.accept(protocolWarning("grpc", "http/protobuf", endpoint, protocolKey, endpointKey));
return false;
}
} else if ("grpc".equals(protocol)) {
if (port == OTLP_HTTP_PROTOBUF_PORT) {
warn.accept(protocolWarning("http/protobuf", "grpc", endpoint, protocolKey, endpointKey));
return false;
}
}
return true;
}

private static String protocolWarning(
String endpointProtocol,
String configuredProtocol,
String endpoint,
String protocolKey,
String endpointKey) {
return "The value for "
+ endpointKey
+ " ("
+ endpoint
+ ") appears to be a "
+ endpointProtocol
+ " endpoint (port "
+ getPort(endpointProtocol)
+ " is the default port for OTLP with "
+ endpointProtocol
+ " protocol) but value for "
+ protocolKey
+ " is "
+ configuredProtocol
+ ". This is likely to be a configuration error. You may need to change the endpoint port to "
+ getPort(configuredProtocol)
+ " (default port for OTLP with "
+ configuredProtocol
+ " protocol) or change the protocol to "
+ endpointProtocol
+ ".";
}

private static int getPort(String protocol) {
return "http/protobuf".equals(protocol) ? OTLP_HTTP_PROTOBUF_PORT : OTLP_GRPC_PORT;
}

private void verifySignalEndpointProtocol(
String signal, String defaultProtocol, String defaultEndpoint) {
String protocolKey = "otel.exporter.otlp." + signal + ".protocol";
String endpointKey = "otel.exporter.otlp." + signal + ".endpoint";
String protocol = getString(protocolKey, defaultProtocol);
String endpoint =
getString(endpointKey, addSignalEndpointSuffix(defaultEndpoint, signal, protocol));
verifyEndpointProtocol(protocol, endpoint, protocolKey, endpointKey);
}

private static String addSignalEndpointSuffix(String endpoint, String signal, String protocol) {
if ("http/protobuf".equals(protocol)) {
return endpoint + "/v1/" + signal;
}
return endpoint;
}

private int getEndpointPort(String endpoint) {
if (endpoint != null) {
try {
URL endpointUrl = new URL(endpoint);
return endpointUrl.getPort();
} catch (MalformedURLException ignore) {
// ignore
}
}
return -1;
}

private String getString(String key, String defaultValue) {
String value = customized.get(key);
if (value == null) {
value = config.getString(key);
}
return value == null ? defaultValue : value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ Map<String, String> customize(ConfigProperties config) {
customized.put("otel.instrumentation.logback-appender.enabled", "false");
}

EndpointProtocolValidator.validate(customized, config, logger::warning);

return customized;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright Splunk Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.splunk.opentelemetry;

import static com.splunk.opentelemetry.SplunkConfigurationTest.configuration;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;

class EndpointProtocolValidatorTest {

@Test
void testDefaultConfiguration() {
ConfigProperties config = configuration(Map::of);
List<String> messages = new ArrayList<>();
EndpointProtocolValidator.validate(Collections.emptyMap(), config, messages::add);
assertThat(messages).isEmpty();
}

@Test
void testGrpcEndpoint() {
ConfigProperties config =
configuration(() -> Map.of("otel.exporter.otlp.endpoint", "http://localhost:4317"));
List<String> messages = new ArrayList<>();
EndpointProtocolValidator.validate(Collections.emptyMap(), config, messages::add);
assertThat(messages).hasSize(1);
}

@Test
void testGrpcEndpointAndProtocol() {
ConfigProperties config =
configuration(
() ->
Map.of(
"otel.exporter.otlp.endpoint",
"http://localhost:4317",
"otel.exporter.otlp.protocol",
"grpc"));
List<String> messages = new ArrayList<>();
EndpointProtocolValidator.validate(Collections.emptyMap(), config, messages::add);
assertThat(messages).isEmpty();
}

@Test
void testUnknownEndpoint() {
ConfigProperties config =
configuration(() -> Map.of("otel.exporter.otlp.endpoint", "http://localhost:5000"));
List<String> messages = new ArrayList<>();
EndpointProtocolValidator.validate(Collections.emptyMap(), config, messages::add);
assertThat(messages).isEmpty();
}

@Test
void testSignalGrpcEndpoint() {
ConfigProperties config =
configuration(
() ->
Map.of(
"otel.exporter.otlp.traces.endpoint",
"http://localhost:4317",
"otel.exporter.otlp.metrics.endpoint",
"http://localhost:4317",
"otel.exporter.otlp.logs.endpoint",
"http://localhost:4317"));
List<String> messages = new ArrayList<>();
EndpointProtocolValidator.validate(Collections.emptyMap(), config, messages::add);
assertThat(messages).hasSize(3);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ private static ConfigProperties configuration() {
return configuration(Map::of);
}

private static ConfigProperties configuration(
Supplier<Map<String, String>> testPropertiesSupplier) {
static ConfigProperties configuration(Supplier<Map<String, String>> testPropertiesSupplier) {
AutoConfiguredOpenTelemetrySdk sdk =
AutoConfiguredOpenTelemetrySdk.builder()
// don't create the SDK
Expand Down

0 comments on commit b98bf38

Please sign in to comment.