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

Initialize appenders in the spring boot starter #8888

Merged
merged 4 commits into from
Jul 11, 2023
Merged

Initialize appenders in the spring boot starter #8888

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
284de93
Initialize appenders in the spring boot starter
Jul 6, 2023
0b3233d
code review comments, add tests
Jul 7, 2023
33abb10
remove printlns
Jul 7, 2023
5c660a4
Merge branch 'main' into spring-boot-appenders
laurit Jul 11, 2023
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
4 changes: 4 additions & 0 deletions instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ dependencies {
compileOnly("io.opentelemetry:opentelemetry-exporter-otlp")
compileOnly("io.opentelemetry:opentelemetry-exporter-zipkin")
compileOnly(project(":instrumentation-annotations"))
compileOnly(project(":instrumentation:log4j:log4j-appender-2.17:library"))
compileOnly("org.apache.logging.log4j:log4j-core:2.17.0")
compileOnly(project(":instrumentation:logback:logback-appender-1.0:library"))
compileOnly("ch.qos.logback:logback-classic:1.0.0")
Comment on lines +44 to +47
Copy link
Member

Choose a reason for hiding this comment

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

what do you think about pulling in the appender instrumentation by default, similar to the other library instrumentations?

Copy link
Member

Choose a reason for hiding this comment

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

created #8925 to track/discuss further, will merge this PR as is for the upcoming 1.28.0 release


compileOnly(project(":instrumentation:resources:library"))
annotationProcessor("com.google.auto.service:auto-service")
Expand Down
4 changes: 0 additions & 4 deletions ...io/opentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package io.opentelemetry.instrumentation.spring.autoconfigure;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
Expand Down Expand Up @@ -141,9 +140,6 @@ public OpenTelemetry openTelemetry(

ContextPropagators propagators = propagatorsProvider.getIfAvailable(ContextPropagators::noop);

// global is needed for logging appenders
GlobalOpenTelemetry.set(OpenTelemetrySdk.builder().setLoggerProvider(loggerProvider).build());
Copy link
Member

Choose a reason for hiding this comment

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

An OpenTelemtry instance is built twice: on line 145 and just after


return OpenTelemetrySdk.builder()
.setTracerProvider(tracerProvider)
.setMeterProvider(meterProvider)
Expand Down
53 changes: 53 additions & 0 deletions .../instrumentation/spring/autoconfigure/logging/OpenTelemetryAppenderAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure.logging;

import io.opentelemetry.api.OpenTelemetry;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.event.ApplicationPreparedEvent;
import org.springframework.context.ApplicationListener;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
@SuppressWarnings("OtelPrivateConstructorForUtilityClass")
@ConditionalOnBean(OpenTelemetry.class)
public class OpenTelemetryAppenderAutoConfiguration {
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved

@Bean
@ConditionalOnProperty(
prefix = "otel.springboot.log4j-appender",
name = "enabled",
matchIfMissing = true)
@ConditionalOnClass({
io.opentelemetry.instrumentation.log4j.appender.v2_17.OpenTelemetryAppender.class
})
ApplicationListener<ApplicationPreparedEvent> log4jOtelAppenderInitializer(
OpenTelemetry openTelemetry) {
return event -> {
io.opentelemetry.instrumentation.log4j.appender.v2_17.OpenTelemetryAppender.install(
openTelemetry);
};
}

@Bean
@ConditionalOnProperty(
Copy link
Member

Choose a reason for hiding this comment

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

Why not to enable the OpenTelemetry injection by default? Check on line 44 is not sufficient?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is enabled by default; in case the property is absent it still gets evaluated to true. I added it to make it possible to disable the appender auto configuration.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I was not rather explicit. My point is that I don't see the use case for which the user would like to disable the automatic injection of the OpenTelemetry instance into the OTel appender. I would prefer to remove the property if we don't have a use case in mind.

prefix = "otel.springboot.logback-appender",
name = "enabled",
matchIfMissing = true)
@ConditionalOnClass({
io.opentelemetry.instrumentation.logback.appender.v1_0.OpenTelemetryAppender.class
})
ApplicationListener<ApplicationPreparedEvent> logbackOtelAppenderInitializer(
OpenTelemetry openTelemetry) {
return event -> {
io.opentelemetry.instrumentation.logback.appender.v1_0.OpenTelemetryAppender.install(
openTelemetry);
};
}
}
7 changes: 0 additions & 7 deletions ...pentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import static io.opentelemetry.semconv.resource.attributes.ResourceAttributes.SERVICE_NAME;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.instrumentation.spring.autoconfigure.resources.OtelResourceAutoConfiguration;
Expand All @@ -17,7 +16,6 @@
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -37,11 +35,6 @@ public OpenTelemetry customOpenTelemetry() {

private final ApplicationContextRunner contextRunner = new ApplicationContextRunner();

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName(
"when Application Context contains OpenTelemetry bean should NOT initialize openTelemetry")
Expand Down
7 changes: 0 additions & 7 deletions ...emetry/instrumentation/spring/autoconfigure/aspects/TraceAspectAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -24,11 +22,6 @@ public class TraceAspectAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, TraceAspectAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when aspects are ENABLED should initialize WithSpanAspect bean")
void aspectsEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...tation/spring/autoconfigure/exporters/jaeger/JaegerSpanExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.jaeger.JaegerGrpcSpanExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -25,11 +23,6 @@ class JaegerSpanExporterAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, JaegerSpanExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when exporters are ENABLED should initialize JaegerGrpcSpanExporter bean")
void exportersEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...on/spring/autoconfigure/exporters/logging/LoggingMetricExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.logging.LoggingMetricExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
Expand All @@ -24,11 +22,6 @@ class LoggingMetricExporterAutoConfigurationTest {
OpenTelemetryAutoConfiguration.class,
LoggingMetricExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
void loggingEnabled() {
runner
Expand Down
7 changes: 0 additions & 7 deletions ...tion/spring/autoconfigure/exporters/logging/LoggingSpanExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.logging.LoggingSpanExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -26,11 +24,6 @@ class LoggingSpanExporterAutoConfigurationTest {
OpenTelemetryAutoConfiguration.class,
LoggingSpanExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when exporters are ENABLED should initialize LoggingSpanExporter bean")
void loggingEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...rumentation/spring/autoconfigure/exporters/otlp/OtlpLogExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.otlp.logs.OtlpGrpcLogRecordExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
Expand All @@ -23,11 +21,6 @@ class OtlpLogExporterAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, OtlpLoggerExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
void otlpEnabled() {
runner
Expand Down
7 changes: 0 additions & 7 deletions ...entation/spring/autoconfigure/exporters/otlp/OtlpMetricExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
Expand All @@ -23,11 +21,6 @@ class OtlpMetricExporterAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, OtlpMetricExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
void otlpEnabled() {
runner
Expand Down
7 changes: 0 additions & 7 deletions ...umentation/spring/autoconfigure/exporters/otlp/OtlpSpanExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -25,11 +23,6 @@ class OtlpSpanExporterAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, OtlpSpanExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when exporters are ENABLED should initialize OtlpGrpcSpanExporter bean")
void otlpEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...tation/spring/autoconfigure/exporters/zipkin/ZipkinSpanExporterAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.exporter.zipkin.ZipkinSpanExporter;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -25,11 +23,6 @@ class ZipkinSpanExporterAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, ZipkinSpanExporterAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when exporters are ENABLED should initialize ZipkinSpanExporter bean")
void exportersEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...tion/spring/autoconfigure/httpclients/resttemplate/RestTemplateAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -24,11 +22,6 @@ class RestTemplateAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, RestTemplateAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when httpclients are ENABLED should initialize RestTemplateInterceptor bean")
void httpClientsEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...umentation/spring/autoconfigure/httpclients/webclient/WebClientAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -24,11 +22,6 @@ class WebClientAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, WebClientAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when httpclients are ENABLED should initialize WebClientBeanPostProcessor bean")
void httpClientsEnabled() {
Expand Down
7 changes: 0 additions & 7 deletions ...try/instrumentation/spring/autoconfigure/metrics/MicrometerShimAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.micrometer.core.instrument.MeterRegistry;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.micrometer.v1_5.OpenTelemetryMeterRegistry;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -25,11 +23,6 @@ class MicrometerShimAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, MicrometerShimAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
void metricsEnabled() {
runner
Expand Down
7 changes: 0 additions & 7 deletions ...ry/instrumentation/spring/autoconfigure/propagators/PropagationAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
Expand All @@ -24,11 +22,6 @@ class PropagationAutoConfigurationTest {
AutoConfigurations.of(
OpenTelemetryAutoConfiguration.class, PropagationAutoConfiguration.class));

@BeforeEach
void resetGlobalLoggerProvider() {
GlobalOpenTelemetry.resetForTest();
}

@Test
@DisplayName("when propagation is ENABLED should initialize PropagationAutoConfiguration bean")
void shouldBeConfigured() {
Expand Down
Loading