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.

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

Add autoconfiguration wrapper artifact #2401

Merged
merged 22 commits into from
Jan 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.spi.OpenTelemetryFactory;
import io.opentelemetry.spi.trace.TracerProviderFactory;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import javax.annotation.Nullable;

/**
Expand All @@ -31,6 +33,9 @@
*/
public final class GlobalOpenTelemetry {
private static final Object mutex = new Object();

private static final OpenTelemetry NO_OP = DefaultOpenTelemetry.builder().build();

@Nullable private static volatile OpenTelemetry globalOpenTelemetry;

private GlobalOpenTelemetry() {}
Expand All @@ -48,12 +53,14 @@ public static OpenTelemetry get() {
if (globalOpenTelemetry == null) {
synchronized (mutex) {
if (globalOpenTelemetry == null) {
OpenTelemetryFactory openTelemetryFactory = Utils.loadSpi(OpenTelemetryFactory.class);
if (openTelemetryFactory != null) {
set(openTelemetryFactory.create());
} else {
set(DefaultOpenTelemetry.builder().build());

OpenTelemetry autoConfigured = maybeAutoConfigure();
if (autoConfigured != null) {
set(autoConfigured);
return autoConfigured;
}

return NO_OP;
}
}
}
Expand Down Expand Up @@ -115,4 +122,25 @@ public static Tracer getTracer(String instrumentationName, String instrumentatio
public static ContextPropagators getPropagators() {
return get().getPropagators();
}

@Nullable
private static OpenTelemetry maybeAutoConfigure() {
final Class<?> openTelemetrySdkAutoConfiguration;
try {
openTelemetrySdkAutoConfiguration =
Class.forName("io.opentelemetry.sdk.autoconfigure.OpenTelemetrySdkAutoConfiguration");
} catch (ClassNotFoundException e) {
return null;
}

try {
Method initialize = openTelemetrySdkAutoConfiguration.getMethod("initialize");
return (OpenTelemetry) initialize.invoke(null);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
throw new IllegalStateException(
"OpenTelemetrySdkAutoConfiguration detected on classpath "
+ "but could not invoke initialize method. This is a bug in OpenTelemetry.",
Copy link
Contributor

Choose a reason for hiding this comment

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

this could also be due to a restrictive security manager

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a public method, as far as I know there wouldn't be any situation that could be blocked by security manager, is there one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought security managers could block all reflection, but I wouldn't swear to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, a security manager can block all reflection, even for public methods, and even just calls to "getMethod", although I would suspect that's extremely rare these days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had no idea - do you think I should change the message? Wondering how realistic it is to disable public reflection vs adding noise to this code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just "This could be a bug in OpenTelemetry" ? Really, it doesn't matter all that much.

e);
}
}
}
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ subprojects {
jsr305 : "com.google.code.findbugs:jsr305:${findBugsJsr305Version}",
prometheus_client : "io.prometheus:simpleclient:${prometheusVersion}",
prometheus_client_common : "io.prometheus:simpleclient_common:${prometheusVersion}",
prometheus_client_httpserver: "io.prometheus:simpleclient_httpserver:${prometheusVersion}",
jkwatson marked this conversation as resolved.
Show resolved Hide resolved
protobuf : "com.google.protobuf:protobuf-java",
protobuf_util : "com.google.protobuf:protobuf-java-util",
zipkin_reporter : "io.zipkin.reporter2:zipkin-reporter",
Expand Down
5 changes: 2 additions & 3 deletions exporters/prometheus/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ description = 'OpenTelemetry Prometheus Exporter'
ext.moduleName = "io.opentelemetry.exporter.prometheus"

dependencies {
api project(':sdk:metrics')

implementation libraries.prometheus_client
api project(':sdk:metrics'),
libraries.prometheus_client

testImplementation libraries.prometheus_client_common,
libraries.guava
Expand Down
27 changes: 27 additions & 0 deletions sdk-extensions/autoconfigure/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
plugins {
id "java-library"
id "maven-publish"

id "ru.vyarus.animalsniffer"
}

description = 'OpenTelemetry SDK Auto-configuration'
ext.moduleName = "io.opentelemetry.sdk.autoconfigure"

dependencies {
api project(':sdk:all'),
project(':sdk:metrics')

implementation project(':extensions:trace-propagators')

compileOnly project(':exporters:jaeger')
compileOnly project(':exporters:jaeger-thrift')
compileOnly project(':exporters:logging')
compileOnly project(':exporters:otlp:all')
compileOnly project(':exporters:otlp:metrics')

compileOnly project(':exporters:prometheus')
compileOnly libraries.prometheus_client_httpserver

compileOnly project(':exporters:zipkin')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.autoconfigure;

import java.util.AbstractMap;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

class ConfigProperties {

static ConfigProperties get() {
return new ConfigProperties(System.getProperties(), System.getenv());
}

private final Map<String, String> config;
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be up above the static methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh - I'm going to have trouble getting used to fields being so far from the constructor :P

Copy link
Member

Choose a reason for hiding this comment

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

that's why static methods should go at the bottom of the class 😁


private ConfigProperties(Properties systemProperties, Map<String, String> environmentVariables) {
Map<String, String> config = new HashMap<>();
environmentVariables.forEach(
(name, value) -> {
config.put(name.toLowerCase(Locale.ROOT).replace('_', '.'), value);
});
systemProperties.forEach(
(key, value) -> {
config.put(((String) key).toLowerCase(Locale.ROOT), (String) value);
});

this.config = config;
}

@Nullable
String getString(String name) {
return config.get(name);
}

@Nullable
Integer getInt(String name) {
try {
// Null will throw NumberFormatException too.
return Integer.parseInt(config.get(name));
} catch (NumberFormatException ex) {
return null;
}
}

@Nullable
Long getLong(String name) {
try {
// Null will throw NumberFormatException too.
return Long.parseLong(config.get(name));
} catch (NumberFormatException ex) {
return null;
}
}

/**
* Get a double property from the config or {@code null} if it cannot be found or it has a wrong
* type.
*/
@Nullable
Double getDouble(String name) {
try {
return Double.parseDouble(config.get(name));
} catch (NumberFormatException | NullPointerException ex) {
return null;
}
}

List<String> getCommaSeparatedValues(String name) {
String value = config.get(name);
if (value == null) {
return Collections.emptyList();
}
return Arrays.stream(value.split(","))
.map(String::trim)
.filter(s -> !s.isEmpty())
.collect(Collectors.toList());
}

Map<String, String> getCommaSeparatedMap(String name) {
return getCommaSeparatedValues(name).stream()
.map(
keyValuePair ->
Arrays.stream(keyValuePair.split("=", 2))
.map(String::trim)
.filter(s -> !s.isEmpty())
.collect(Collectors.toList()))
Copy link
Contributor

Choose a reason for hiding this comment

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

These 4 lines of code could be extracted to a method like "filterBlanksAndNulls" and re-used above at line 96-99

.map(
splitKeyValuePairs ->
new AbstractMap.SimpleImmutableEntry<>(
splitKeyValuePairs.get(0), splitKeyValuePairs.get(1)))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}

boolean getBoolean(String name) {
return Boolean.parseBoolean(config.get(name));
}
}
Loading