Initialize appenders in the spring boot starter #8888
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| /* | ||
| * 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.ApplicationReadyEvent; | ||
| 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
|
||
|
|
||
| @Configuration | ||
| @ConditionalOnProperty( | ||
| prefix = "otel.springboot.log4j-appender", | ||
| name = "enabled", | ||
| matchIfMissing = true) | ||
| @ConditionalOnClass({ | ||
| io.opentelemetry.instrumentation.log4j.appender.v2_17.OpenTelemetryAppender.class | ||
| }) | ||
| static class Log4jAppenderConfig { | ||
|
|
||
| @Bean | ||
| ApplicationListener<ApplicationReadyEvent> log4jOtelAppenderInitializer( | ||
| OpenTelemetry openTelemetry) { | ||
| return event -> { | ||
| io.opentelemetry.instrumentation.log4j.appender.v2_17.OpenTelemetryAppender.install( | ||
|
There was a problem hiding this comment. what do you think about adding the otel appender if it doesn't exist? One thing less for the user to do. Here's how I did it: https://github.com/grafana/grafana-opentelemetry-starter/blob/main/src/main/java/com/grafana/opentelemetry/Log4jConfig.java There was a problem hiding this comment. It would be great. We would have to provide a way to customize the OTel automatically added. Today, users can do this with an XML configuration in their Spring Boot projects. It could be done for example by providing a Spring bean of the OTel appender automatically added. The user could then customize this bean with a Spring post-processor. There was a problem hiding this comment.
Users can still add the appender (using XML) if they want to have some special settings.
sure, I don't mind 😄 There was a problem hiding this comment. I have created an issue for tracking and discussions: #8920 |
||
| openTelemetry); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| @Configuration | ||
| @ConditionalOnProperty( | ||
|
There was a problem hiding this comment. Why not to enable the OpenTelemetry injection by default? Check on line 44 is not sufficient? There was a problem hiding this comment. It is enabled by default; in case the property is absent it still gets evaluated to There was a problem hiding this comment. 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 | ||
| }) | ||
| static class LogbackAppenderConfig { | ||
|
|
||
| @Bean | ||
| ApplicationListener<ApplicationReadyEvent> logbackOtelAppenderInitializer( | ||
| OpenTelemetry openTelemetry) { | ||
| return event -> { | ||
| io.opentelemetry.instrumentation.logback.appender.v1_0.OpenTelemetryAppender.install( | ||
| openTelemetry); | ||
| }; | ||
| } | ||
| } | ||
| } | ||
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.
what do you think about pulling in the appender instrumentation by default, similar to the other library instrumentations?
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.
created #8925 to track/discuss further, will merge this PR as is for the upcoming 1.28.0 release