- 
                Notifications
    
You must be signed in to change notification settings  - Fork 168
 
Load OTel SDK config from environment variables and system properties.… #1434
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
Changes from 6 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 | 
|---|---|---|
| 
          
            
          
           | 
    @@ -5,42 +5,121 @@ | |
| 
     | 
||
| 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.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.getResource(); | ||
| 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.getResource(); | ||
| 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"); | ||
| } | ||
| } | ||
| 
     | 
||
| 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(); | ||
| // } | ||
| /** 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"); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.