From 0ce4dbd49ffc25b43eaae4caa8d3f7beaa529c1d Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Tue, 22 Oct 2024 11:28:55 +0200 Subject: [PATCH] Polish "Add property to control log exporting" See gh-42813 --- ...=> ConditionalOnEnabledLoggingExport.java} | 6 ++-- ...a => OnEnabledLoggingExportCondition.java} | 15 ++++---- .../otlp/OtlpLoggingAutoConfiguration.java | 2 -- .../otlp/OtlpLoggingConfigurations.java | 2 ++ ...itional-spring-configuration-metadata.json | 4 +-- ...OnEnabledLoggingExportConditionTests.java} | 34 +++++++++---------- .../OtlpLoggingAutoConfigurationTests.java | 22 +++++++++--- 7 files changed, 50 insertions(+), 35 deletions(-) rename spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/{ConditionalOnEnabledLogging.java => ConditionalOnEnabledLoggingExport.java} (93%) rename spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/{OnEnabledLoggingCondition.java => OnEnabledLoggingExportCondition.java} (84%) rename spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/{OnEnabledLoggingConditionTests.java => OnEnabledLoggingExportConditionTests.java} (75%) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/ConditionalOnEnabledLogging.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/ConditionalOnEnabledLoggingExport.java similarity index 93% rename from spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/ConditionalOnEnabledLogging.java rename to spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/ConditionalOnEnabledLoggingExport.java index 772a0356aca6..b32d8f9c4922 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/ConditionalOnEnabledLogging.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/ConditionalOnEnabledLoggingExport.java @@ -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..logging.export.enabled} property can be used to @@ -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. diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingCondition.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingExportCondition.java similarity index 84% rename from spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingCondition.java rename to spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingExportCondition.java index b73d32ae0931..eb328c816525 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingCondition.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingExportCondition.java @@ -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"; @@ -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 attributes = metadata.getAnnotationAttributes(ConditionalOnEnabledLogging.class.getName()); + Map attributes = metadata + .getAnnotationAttributes(ConditionalOnEnabledLoggingExport.class.getName()); if (attributes == null) { return null; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfiguration.java index e033650b4ba2..ec169dab024c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfiguration.java @@ -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; @@ -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 { } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingConfigurations.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingConfigurations.java index b2b1b300f9dd..14b491bd237d 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingConfigurations.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingConfigurations.java @@ -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; @@ -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 diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 478082038cbd..855ab8e56d45 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -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 }, { @@ -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", diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingConditionTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingExportConditionTests.java similarity index 75% rename from spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingConditionTests.java rename to spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingExportConditionTests.java index bd0d2ca98e12..1bca2e10715a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingConditionTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/OnEnabledLoggingExportConditionTests.java @@ -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() { @@ -122,7 +122,7 @@ private ConditionContext mockConditionContext(Map 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; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfigurationTests.java index 6066c16afbed..34a70bca0d26 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/logging/otlp/OtlpLoggingAutoConfigurationTests.java @@ -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