Skip to content

Commit

Permalink
Polish "Add property to control log exporting"
Browse files Browse the repository at this point in the history
  • Loading branch information
mhalbritter committed Oct 22, 2024
1 parent e9b3b97 commit 0ce4dbd
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.springframework.context.annotation.Conditional;

/**
* {@link Conditional @Conditional} that checks whether logging exporter is enabled. It
* {@link Conditional @Conditional} that checks whether logging export is enabled. It
* matches if the value of the {@code management.logging.export.enabled} property is
* {@code true} or if it is not configured. If the {@link #value() logging exporter name}
* is set, the {@code management.<name>.logging.export.enabled} property can be used to
Expand All @@ -39,8 +39,8 @@
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.TYPE, ElementType.METHOD })
@Documented
@Conditional(OnEnabledLoggingCondition.class)
public @interface ConditionalOnEnabledLogging {
@Conditional(OnEnabledLoggingExportCondition.class)
public @interface ConditionalOnEnabledLoggingExport {

/**
* Name of the logging exporter.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
*
* @author Moritz Halbritter
* @author Dmytro Nosan
* @see ConditionalOnEnabledLogging
* @see ConditionalOnEnabledLoggingExport
*/
class OnEnabledLoggingCondition extends SpringBootCondition {
class OnEnabledLoggingExportCondition extends SpringBootCondition {

private static final String GLOBAL_PROPERTY = "management.logging.export.enabled";

Expand All @@ -46,22 +46,23 @@ public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeM
.getProperty(EXPORTER_PROPERTY.formatted(loggingExporter), Boolean.class);
if (exporterLoggingEnabled != null) {
return new ConditionOutcome(exporterLoggingEnabled,
ConditionMessage.forCondition(ConditionalOnEnabledLogging.class)
ConditionMessage.forCondition(ConditionalOnEnabledLoggingExport.class)
.because(EXPORTER_PROPERTY.formatted(loggingExporter) + " is " + exporterLoggingEnabled));
}
}
Boolean globalLoggingEnabled = context.getEnvironment().getProperty(GLOBAL_PROPERTY, Boolean.class);
if (globalLoggingEnabled != null) {
return new ConditionOutcome(globalLoggingEnabled,
ConditionMessage.forCondition(ConditionalOnEnabledLogging.class)
ConditionMessage.forCondition(ConditionalOnEnabledLoggingExport.class)
.because(GLOBAL_PROPERTY + " is " + globalLoggingEnabled));
}
return ConditionOutcome.match(ConditionMessage.forCondition(ConditionalOnEnabledLogging.class)
.because("logging is enabled by default"));
return ConditionOutcome.match(ConditionMessage.forCondition(ConditionalOnEnabledLoggingExport.class)
.because("is enabled by default"));
}

private static String getExporterName(AnnotatedTypeMetadata metadata) {
Map<String, Object> attributes = metadata.getAnnotationAttributes(ConditionalOnEnabledLogging.class.getName());
Map<String, Object> attributes = metadata
.getAnnotationAttributes(ConditionalOnEnabledLoggingExport.class.getName());
if (attributes == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import io.opentelemetry.exporter.otlp.http.logs.OtlpHttpLogRecordExporter;
import io.opentelemetry.sdk.logs.SdkLoggerProvider;

import org.springframework.boot.actuate.autoconfigure.logging.ConditionalOnEnabledLogging;
import org.springframework.boot.autoconfigure.AutoConfiguration;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
Expand All @@ -37,7 +36,6 @@
@ConditionalOnClass({ SdkLoggerProvider.class, OpenTelemetry.class, OtlpHttpLogRecordExporter.class })
@EnableConfigurationProperties(OtlpLoggingProperties.class)
@Import({ OtlpLoggingConfigurations.ConnectionDetails.class, OtlpLoggingConfigurations.Exporters.class })
@ConditionalOnEnabledLogging("otlp")
public class OtlpLoggingAutoConfiguration {

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.opentelemetry.exporter.otlp.logs.OtlpGrpcLogRecordExporter;
import io.opentelemetry.exporter.otlp.logs.OtlpGrpcLogRecordExporterBuilder;

import org.springframework.boot.actuate.autoconfigure.logging.ConditionalOnEnabledLoggingExport;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
Expand Down Expand Up @@ -76,6 +77,7 @@ public String getUrl(Transport transport) {
@Configuration(proxyBeanMethods = false)
@ConditionalOnMissingBean({ OtlpGrpcLogRecordExporter.class, OtlpHttpLogRecordExporter.class })
@ConditionalOnBean(OtlpLoggingConnectionDetails.class)
@ConditionalOnEnabledLoggingExport("otlp")
static class Exporters {

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@
{
"name": "management.logging.export.enabled",
"type": "java.lang.Boolean",
"description": "Whether the auto-configuration for exporting log record data is enabled",
"description": "Whether auto-configuration of logging is enabled to export logs.",
"defaultValue": true
},
{
Expand Down Expand Up @@ -2076,7 +2076,7 @@
{
"name": "management.otlp.logging.export.enabled",
"type": "java.lang.Boolean",
"description": "Whether auto-configuration for exporting OTLP log records is enabled."
"description": "Whether auto-configuration of logging is enabled to export OTLP logs."
},
{
"name": "management.otlp.tracing.export.enabled",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,81 +31,81 @@
import static org.mockito.Mockito.mock;

/**
* Tests for {@link OnEnabledLoggingCondition}.
* Tests for {@link OnEnabledLoggingExportCondition}.
*
* @author Moritz Halbritter
* @author Dmytro Nosan
*/
class OnEnabledLoggingConditionTests {
class OnEnabledLoggingExportConditionTests {

@Test
void shouldMatchIfNoPropertyIsSet() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(mockConditionContext(), mockMetadata(""));
assertThat(outcome.isMatch()).isTrue();
assertThat(outcome.getMessage()).isEqualTo("@ConditionalOnEnabledLogging logging is enabled by default");
assertThat(outcome.getMessage()).isEqualTo("@ConditionalOnEnabledLoggingExport is enabled by default");
}

@Test
void shouldNotMatchIfGlobalPropertyIsFalse() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(
mockConditionContext(Map.of("management.logging.export.enabled", "false")), mockMetadata(""));
assertThat(outcome.isMatch()).isFalse();
assertThat(outcome.getMessage())
.isEqualTo("@ConditionalOnEnabledLogging management.logging.export.enabled is false");
.isEqualTo("@ConditionalOnEnabledLoggingExport management.logging.export.enabled is false");
}

@Test
void shouldMatchIfGlobalPropertyIsTrue() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(
mockConditionContext(Map.of("management.logging.export.enabled", "true")), mockMetadata(""));
assertThat(outcome.isMatch()).isTrue();
assertThat(outcome.getMessage())
.isEqualTo("@ConditionalOnEnabledLogging management.logging.export.enabled is true");
.isEqualTo("@ConditionalOnEnabledLoggingExport management.logging.export.enabled is true");
}

@Test
void shouldNotMatchIfExporterPropertyIsFalse() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(
mockConditionContext(Map.of("management.otlp.logging.export.enabled", "false")), mockMetadata("otlp"));
assertThat(outcome.isMatch()).isFalse();
assertThat(outcome.getMessage())
.isEqualTo("@ConditionalOnEnabledLogging management.otlp.logging.export.enabled is false");
.isEqualTo("@ConditionalOnEnabledLoggingExport management.otlp.logging.export.enabled is false");
}

@Test
void shouldMatchIfExporterPropertyIsTrue() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(
mockConditionContext(Map.of("management.otlp.logging.export.enabled", "true")), mockMetadata("otlp"));
assertThat(outcome.isMatch()).isTrue();
assertThat(outcome.getMessage())
.isEqualTo("@ConditionalOnEnabledLogging management.otlp.logging.export.enabled is true");
.isEqualTo("@ConditionalOnEnabledLoggingExport management.otlp.logging.export.enabled is true");
}

@Test
void exporterPropertyShouldOverrideGlobalPropertyIfTrue() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(mockConditionContext(
Map.of("management.logging.enabled", "false", "management.otlp.logging.export.enabled", "true")),
mockMetadata("otlp"));
assertThat(outcome.isMatch()).isTrue();
assertThat(outcome.getMessage())
.isEqualTo("@ConditionalOnEnabledLogging management.otlp.logging.export.enabled is true");
.isEqualTo("@ConditionalOnEnabledLoggingExport management.otlp.logging.export.enabled is true");
}

@Test
void exporterPropertyShouldOverrideGlobalPropertyIfFalse() {
OnEnabledLoggingCondition condition = new OnEnabledLoggingCondition();
OnEnabledLoggingExportCondition condition = new OnEnabledLoggingExportCondition();
ConditionOutcome outcome = condition.getMatchOutcome(mockConditionContext(
Map.of("management.logging.enabled", "true", "management.otlp.logging.export.enabled", "false")),
mockMetadata("otlp"));
assertThat(outcome.isMatch()).isFalse();
assertThat(outcome.getMessage())
.isEqualTo("@ConditionalOnEnabledLogging management.otlp.logging.export.enabled is false");
.isEqualTo("@ConditionalOnEnabledLoggingExport management.otlp.logging.export.enabled is false");
}

private ConditionContext mockConditionContext() {
Expand All @@ -122,7 +122,7 @@ private ConditionContext mockConditionContext(Map<String, String> properties) {

private AnnotatedTypeMetadata mockMetadata(String exporter) {
AnnotatedTypeMetadata metadata = mock(AnnotatedTypeMetadata.class);
given(metadata.getAnnotationAttributes(ConditionalOnEnabledLogging.class.getName()))
given(metadata.getAnnotationAttributes(ConditionalOnEnabledLoggingExport.class.getName()))
.willReturn(Map.of("value", exporter));
return metadata;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,26 @@ void shouldNotSupplyBeansIfDependencyIsMissing(String packageName) {
});
}

@Test
void shouldBackOffWhenLoggingExportPropertyIsNotEnabled() {
this.contextRunner
.withPropertyValues("management.logging.export.enabled=false",
"management.otlp.logging.endpoint=http://localhost:4318/v1/logs")
.run((context) -> {
assertThat(context).hasSingleBean(OtlpLoggingConnectionDetails.class);
assertThat(context).doesNotHaveBean(LogRecordExporter.class);
});
}

@Test
void shouldBackOffWhenOtlpLoggingExportPropertyIsNotEnabled() {
this.contextRunner.withPropertyValues("management.otlp.logging.export.enabled=false").run((context) -> {
assertThat(context).doesNotHaveBean(OtlpLoggingConnectionDetails.class);
assertThat(context).doesNotHaveBean(LogRecordExporter.class);
});
this.contextRunner
.withPropertyValues("management.otlp.logging.export.enabled=false",
"management.otlp.logging.endpoint=http://localhost:4318/v1/logs")
.run((context) -> {
assertThat(context).hasSingleBean(OtlpLoggingConnectionDetails.class);
assertThat(context).doesNotHaveBean(LogRecordExporter.class);
});
}

@Test
Expand Down

0 comments on commit 0ce4dbd

Please sign in to comment.