-
Notifications
You must be signed in to change notification settings - Fork 130
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
Load OTel SDK config from environment variables and system properties.… #1434
Changes from all commits
ee61c7b
ecb469b
a89bd63
cc3490c
139c096
6846a02
e3316a5
45f8efe
4f75414
70eef3a
f5c4825
e5cb582
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.maven; | ||
|
||
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; | ||
import io.opentelemetry.sdk.resources.Resource; | ||
import java.lang.reflect.InvocationTargetException; | ||
import java.lang.reflect.Method; | ||
|
||
/** Utility class to use the {@link AutoConfiguredOpenTelemetrySdk}. */ | ||
public class AutoConfigureUtil2 { | ||
|
||
private AutoConfigureUtil2() {} | ||
|
||
/** | ||
* Returns the {@link Resource} that was autoconfigured. | ||
* | ||
* <p>Inspired by {@link | ||
* io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil#getConfig(AutoConfiguredOpenTelemetrySdk)} | ||
*/ | ||
public static Resource getResource( | ||
AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) { | ||
try { | ||
Method method = AutoConfiguredOpenTelemetrySdk.class.getDeclaredMethod("getResource"); | ||
method.setAccessible(true); | ||
return (Resource) method.invoke(autoConfiguredOpenTelemetrySdk); | ||
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { | ||
throw new IllegalStateException( | ||
"Error calling getResource on AutoConfiguredOpenTelemetrySdk", e); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.maven; | ||
|
||
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; | ||
import java.lang.reflect.Method; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class AutoConfigureUtil2Test { | ||
|
||
/** | ||
* Verify the reflection call works with the current version of AutoConfiguredOpenTelemetrySdk. | ||
* | ||
* @throws NoSuchMethodException if the method does not exist | ||
*/ | ||
@Test | ||
void test_getResource() throws NoSuchMethodException { | ||
Method method = AutoConfiguredOpenTelemetrySdk.class.getDeclaredMethod("getResource"); | ||
method.setAccessible(true); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,42 +5,132 @@ | |
|
||
package io.opentelemetry.maven; | ||
|
||
import org.junit.jupiter.api.Disabled; | ||
import static io.opentelemetry.api.common.AttributeKey.stringKey; | ||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; | ||
import io.opentelemetry.sdk.resources.Resource; | ||
import org.junit.jupiter.api.AfterAll; | ||
import org.junit.jupiter.api.Test; | ||
|
||
/** | ||
* Note: if otel-java-contrib bumps to Java 11+, we could use junit-pioneer's | ||
* {@code @SetSystemProperty} and {@code @ClearSystemProperty} but no bump is planned for now. | ||
*/ | ||
public class OpenTelemetrySdkServiceTest { | ||
|
||
/** Verify default `service.name` */ | ||
/** Verify default config */ | ||
@Test | ||
@Disabled | ||
public void testDefaultConfiguration() { | ||
testConfiguration("maven"); | ||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
System.clearProperty("otel.service.name"); | ||
System.clearProperty("otel.resource.attributes"); | ||
cyrille-leclerc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { | ||
|
||
Resource resource = openTelemetrySdkService.resource; | ||
assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("maven"); | ||
|
||
ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); | ||
assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); | ||
assertThat(configProperties.getString("otel.traces.exporter")).isEqualTo("none"); | ||
assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); | ||
assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); | ||
} | ||
} | ||
|
||
/** Verify overwritten `service.name` */ | ||
/** Verify overwritten `service.name`,`key1` and `key2` */ | ||
@Test | ||
@Disabled | ||
public void testOverwrittenConfiguration() { | ||
public void testOverwrittenResourceAttributes() { | ||
System.setProperty("otel.service.name", "my-maven"); | ||
try { | ||
testConfiguration("my-maven"); | ||
System.setProperty("otel.resource.attributes", "key1=val1,key2=val2"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be a bit better to not rely on global state (here in system properties) as we have to take care of the cleanup here. What I think could be slightly better is to add an argument to the The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we postpone this improvement? I lack time to clean this up unfortunately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can definitely postpone that to later improvement. Just to keep it safe once this test class has executed adding an
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great suggestion. Please review below. |
||
|
||
try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { | ||
|
||
Resource resource = openTelemetrySdkService.resource; | ||
assertThat(resource.getAttribute(stringKey("service.name"))).isEqualTo("my-maven"); | ||
assertThat(resource.getAttribute(stringKey("key1"))).isEqualTo("val1"); | ||
assertThat(resource.getAttribute(stringKey("key2"))).isEqualTo("val2"); | ||
|
||
} finally { | ||
System.clearProperty("otel.service.name"); | ||
System.clearProperty("otel.resource.attributes"); | ||
} | ||
} | ||
|
||
/** Verify defining `otel.exporter.otlp.endpoint` works */ | ||
@Test | ||
public void testOverwrittenExporterConfiguration_1() { | ||
System.setProperty("otel.exporter.otlp.endpoint", "https://example.com:4317"); | ||
|
||
try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { | ||
|
||
ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); | ||
assertThat(configProperties.getString("otel.exporter.otlp.endpoint")) | ||
.isEqualTo("https://example.com:4317"); | ||
assertThat(configProperties.getString("otel.traces.exporter")).isNull(); | ||
assertThat(configProperties.getString("otel.metrics.exporter")).isNull(); | ||
assertThat(configProperties.getString("otel.logs.exporter")).isNull(); | ||
|
||
} finally { | ||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
} | ||
} | ||
|
||
/** Verify defining `otel.exporter.otlp.traces.endpoint` works */ | ||
@Test | ||
public void testOverwrittenExporterConfiguration_2() { | ||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
System.clearProperty("otel.traces.exporter"); | ||
System.setProperty("otel.exporter.otlp.traces.endpoint", "https://example.com:4317/"); | ||
|
||
try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { | ||
|
||
ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); | ||
assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); | ||
assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) | ||
.isEqualTo("https://example.com:4317/"); | ||
assertThat(configProperties.getString("otel.traces.exporter")).isNull(); | ||
assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); | ||
assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); | ||
|
||
} finally { | ||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
System.clearProperty("otel.traces.exporter"); | ||
System.clearProperty("otel.exporter.otlp.traces.endpoint"); | ||
} | ||
} | ||
|
||
/** Verify defining `otel.exporter.otlp.traces.endpoint` and `otel.traces.exporter` works */ | ||
@Test | ||
public void testOverwrittenExporterConfiguration_3() { | ||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
System.setProperty("otel.traces.exporter", "otlp"); | ||
System.setProperty("otel.exporter.otlp.traces.endpoint", "https://example.com:4317/"); | ||
|
||
try (OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService()) { | ||
|
||
ConfigProperties configProperties = openTelemetrySdkService.getConfigProperties(); | ||
assertThat(configProperties.getString("otel.exporter.otlp.endpoint")).isNull(); | ||
assertThat(configProperties.getString("otel.exporter.otlp.traces.endpoint")) | ||
.isEqualTo("https://example.com:4317/"); | ||
assertThat(configProperties.getString("otel.traces.exporter")).isEqualTo("otlp"); | ||
assertThat(configProperties.getString("otel.metrics.exporter")).isEqualTo("none"); | ||
assertThat(configProperties.getString("otel.logs.exporter")).isEqualTo("none"); | ||
|
||
} finally { | ||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
System.clearProperty("otel.exporter.otlp.traces.endpoint"); | ||
System.clearProperty("otel.exporter.otlp.traces.protocol"); | ||
} | ||
} | ||
|
||
void testConfiguration(String expectedServiceName) { | ||
// OpenTelemetrySdkService openTelemetrySdkService = new OpenTelemetrySdkService(); | ||
// openTelemetrySdkService.initialize(); | ||
// try { | ||
// Resource resource = | ||
// openTelemetrySdkService.autoConfiguredOpenTelemetrySdk.getResource(); | ||
// assertThat(resource.getAttribute(ResourceAttributes.SERVICE_NAME)) | ||
// .isEqualTo(expectedServiceName); | ||
// } finally { | ||
// openTelemetrySdkService.dispose(); | ||
// GlobalOpenTelemetry.resetForTest(); | ||
// GlobalEventEmitterProvider.resetForTest(); | ||
// } | ||
@AfterAll | ||
static void afterAll() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SylvainJuge please review |
||
System.clearProperty("otel.exporter.otlp.endpoint"); | ||
System.clearProperty("otel.exporter.otlp.traces.endpoint"); | ||
System.clearProperty("otel.exporter.otlp.traces.protocol"); | ||
System.clearProperty("otel.resource.attributes"); | ||
System.clearProperty("otel.service.name"); | ||
System.clearProperty("otel.traces.exporter"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor] can you elaborate a bit why do we call this method and what it allows to do here ? In addition, is there a follow-up action to make this method publicly accessible in the SDK if there are other similar use-cases ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch. Comment added in he code of the
OpenTelemetrySdkService
. The use cases are:service.name
andservice.namespace