From 06e3f37afd0198b3e23997073853aae4ca05973e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 11 Sep 2024 13:56:25 -0700 Subject: [PATCH] Polish --- .../opentelemetry/otlp/package-info.java | 2 +- .../reference/pages/features/logging.adoc | 2 + .../ConfigurationPropertiesBeanRegistrar.java | 9 ++- ...tendedLogFormatStructuredLogFormatter.java | 69 +++++++++++-------- ...tendedLogFormatStructuredLogFormatter.java | 64 ++++++++++------- .../logging/logback/StructuredLogEncoder.java | 34 ++++++--- .../GraylogExtendedLogFormatService.java | 2 +- ...dLogFormatStructuredLogFormatterTests.java | 1 - 8 files changed, 109 insertions(+), 74 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/otlp/package-info.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/otlp/package-info.java index 519a94919d..ecdcd3f16b 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/otlp/package-info.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/otlp/package-info.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-docs/src/docs/antora/modules/reference/pages/features/logging.adoc b/spring-boot-project/spring-boot-docs/src/docs/antora/modules/reference/pages/features/logging.adoc index fca2cdee10..0dba5146f2 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/antora/modules/reference/pages/features/logging.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/antora/modules/reference/pages/features/logging.adoc @@ -454,6 +454,7 @@ To enable structured logging, set the property configprop:logging.structured.for [[features.logging.structured.ecs]] === Elastic Common Schema + https://www.elastic.co/guide/en/ecs/8.11/ecs-reference.html[Elastic Common Schema] is a JSON based logging format. To enable the Elastic Common Schema log format, set the appropriate `format` property to `ecs`: @@ -499,6 +500,7 @@ NOTE: configprop:logging.structured.ecs.service.version[] will default to config [[features.logging.structured.gelf]] === Graylog Extended Log Format (GELF) + https://go2docs.graylog.org/current/getting_in_log_data/gelf.html[Graylog Extended Log Format] is a JSON based logging format for the Graylog log analytics platform. To enable the Graylog Extended Log Format, set the appropriate `format` property to `gelf`: diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java index 82c3a8bcbe..ed5add358a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java @@ -102,12 +102,11 @@ final class ConfigurationPropertiesBeanRegistrar { static BeanDefinitionHolder applyScopedProxyMode(ScopeMetadata metadata, BeanDefinitionHolder definition, BeanDefinitionRegistry registry) { - ScopedProxyMode scopedProxyMode = metadata.getScopedProxyMode(); - if (scopedProxyMode.equals(ScopedProxyMode.NO)) { - return definition; + ScopedProxyMode mode = metadata.getScopedProxyMode(); + if (mode != ScopedProxyMode.NO) { + return ScopedProxyUtils.createScopedProxy(definition, registry, mode == ScopedProxyMode.TARGET_CLASS); } - boolean proxyTargetClass = scopedProxyMode.equals(ScopedProxyMode.TARGET_CLASS); - return ScopedProxyUtils.createScopedProxy(definition, registry, proxyTargetClass); + return definition; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/GraylogExtendedLogFormatStructuredLogFormatter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/GraylogExtendedLogFormatStructuredLogFormatter.java index 8a31b50b99..5138b50353 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/GraylogExtendedLogFormatStructuredLogFormatter.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/GraylogExtendedLogFormatStructuredLogFormatter.java @@ -32,6 +32,7 @@ import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.util.ReadOnlyStringMap; import org.springframework.boot.json.JsonWriter; +import org.springframework.boot.json.JsonWriter.Members; import org.springframework.boot.json.JsonWriter.WritableJson; import org.springframework.boot.logging.structured.CommonStructuredLogFormat; import org.springframework.boot.logging.structured.GraylogExtendedLogFormatService; @@ -41,6 +42,7 @@ import org.springframework.core.env.Environment; import org.springframework.core.log.LogMessage; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; /** * Log4j2 {@link StructuredLogFormatter} for @@ -60,12 +62,6 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure */ private static final Pattern FIELD_NAME_VALID_PATTERN = Pattern.compile("^[\\w.\\-]*$"); - /** - * Every field been sent and prefixed with an underscore "_" will be treated as an - * additional field. - */ - private static final String ADDITIONAL_FIELD_PREFIX = "_"; - /** * Libraries SHOULD not allow to send id as additional field ("_id"). Graylog server * nodes omit this field automatically. @@ -78,9 +74,8 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure private static void jsonMembers(Environment environment, JsonWriter.Members members) { members.add("version", "1.1"); - // note: a blank message will lead to a Graylog error as of Graylog v6.0.x. We are - // ignoring this here. - members.add("short_message", LogEvent::getMessage).as(Message::getFormattedMessage); + members.add("short_message", LogEvent::getMessage) + .as(GraylogExtendedLogFormatStructuredLogFormatter::getMessageText); members.add("timestamp", LogEvent::getInstant) .as(GraylogExtendedLogFormatStructuredLogFormatter::formatTimeStamp); members.add("level", GraylogExtendedLogFormatStructuredLogFormatter::convertLevel); @@ -92,25 +87,23 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure members.add("_log_logger", LogEvent::getLoggerName); members.from(LogEvent::getContextData) .whenNot(ReadOnlyStringMap::isEmpty) - .usingPairs((contextData, pairs) -> contextData - .forEach((key, value) -> createAdditionalField(key, value, pairs))); - members.add().whenNotNull(LogEvent::getThrownProxy).usingMembers((eventMembers) -> { - eventMembers.add("full_message", - GraylogExtendedLogFormatStructuredLogFormatter::formatFullMessageWithThrowable); - eventMembers.add("_error_type", (event) -> event.getThrownProxy().getThrowable()) - .whenNotNull() - .as(ObjectUtils::nullSafeClassName); - eventMembers.add("_error_stack_trace", (event) -> event.getThrownProxy().getExtendedStackTraceAsString()); - eventMembers.add("_error_message", (event) -> event.getThrownProxy().getMessage()); - }); + .usingPairs(GraylogExtendedLogFormatStructuredLogFormatter::createAdditionalFields); + members.add() + .whenNotNull(LogEvent::getThrownProxy) + .usingMembers(GraylogExtendedLogFormatStructuredLogFormatter::throwableMembers); + } + + private static String getMessageText(Message message) { + // Always return text as a blank message will lead to a error as of Graylog v6 + String formattedMessage = message.getFormattedMessage(); + return (!StringUtils.hasText(formattedMessage)) ? "(blank)" : formattedMessage; } /** * GELF requires "seconds since UNIX epoch with optional decimal places for * milliseconds". To comply with this requirement, we format a POSIX timestamp * with millisecond precision as e.g. "1725459730385" -> "1725459730.385" - * @param timeStamp the timestamp of the log message. Note it is not the standard Java - * `Instant` type but {@link org.apache.logging.log4j.core.time} + * @param timeStamp the timestamp of the log message. * @return the timestamp formatted as string with millisecond precision */ private static WritableJson formatTimeStamp(Instant timeStamp) { @@ -127,23 +120,39 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure return Severity.getSeverity(event.getLevel()).getCode(); } + private static void throwableMembers(Members members) { + members.add("full_message", GraylogExtendedLogFormatStructuredLogFormatter::formatFullMessageWithThrowable); + members.add("_error_type", (event) -> event.getThrownProxy().getThrowable()) + .whenNotNull() + .as(ObjectUtils::nullSafeClassName); + members.add("_error_stack_trace", (event) -> event.getThrownProxy().getExtendedStackTraceAsString()); + members.add("_error_message", (event) -> event.getThrownProxy().getMessage()); + } + private static String formatFullMessageWithThrowable(LogEvent event) { return event.getMessage().getFormattedMessage() + "\n\n" + event.getThrownProxy().getExtendedStackTraceAsString(); } - private static void createAdditionalField(String fieldName, Object value, BiConsumer pairs) { - Assert.notNull(fieldName, "fieldName must not be null"); - if (!FIELD_NAME_VALID_PATTERN.matcher(fieldName).matches()) { - logger.warn(LogMessage.format("'%s' is not a valid field name according to GELF standard", fieldName)); + private static void createAdditionalFields(ReadOnlyStringMap contextData, BiConsumer pairs) { + contextData.forEach((name, value) -> createAdditionalField(name, value, pairs)); + } + + private static void createAdditionalField(String name, Object value, BiConsumer pairs) { + Assert.notNull(name, "fieldName must not be null"); + if (!FIELD_NAME_VALID_PATTERN.matcher(name).matches()) { + logger.warn(LogMessage.format("'%s' is not a valid field name according to GELF standard", name)); return; } - if (ADDITIONAL_FIELD_ILLEGAL_KEYS.contains(fieldName)) { - logger.warn(LogMessage.format("'%s' is an illegal field name according to GELF standard", fieldName)); + if (ADDITIONAL_FIELD_ILLEGAL_KEYS.contains(name)) { + logger.warn(LogMessage.format("'%s' is an illegal field name according to GELF standard", name)); return; } - String key = (fieldName.startsWith(ADDITIONAL_FIELD_PREFIX)) ? fieldName : ADDITIONAL_FIELD_PREFIX + fieldName; - pairs.accept(key, value); + pairs.accept(asAdditionalFieldName(name), value); + } + + private static Object asAdditionalFieldName(String name) { + return (!name.startsWith("_")) ? "_" + name : name; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatter.java index 4d9df6a814..b2ddd304d1 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatter.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatter.java @@ -17,6 +17,7 @@ package org.springframework.boot.logging.logback; import java.math.BigDecimal; +import java.util.List; import java.util.Objects; import java.util.Set; import java.util.function.BiConsumer; @@ -28,8 +29,10 @@ import ch.qos.logback.classic.spi.IThrowableProxy; import ch.qos.logback.classic.util.LevelToSyslogSeverity; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.slf4j.event.KeyValuePair; import org.springframework.boot.json.JsonWriter; +import org.springframework.boot.json.JsonWriter.Members; import org.springframework.boot.json.JsonWriter.WritableJson; import org.springframework.boot.logging.structured.CommonStructuredLogFormat; import org.springframework.boot.logging.structured.GraylogExtendedLogFormatService; @@ -39,6 +42,7 @@ import org.springframework.core.env.Environment; import org.springframework.core.log.LogMessage; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; /** * Logback {@link StructuredLogFormatter} for @@ -58,12 +62,6 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure */ private static final Pattern FIELD_NAME_VALID_PATTERN = Pattern.compile("^[\\w.\\-]*$"); - /** - * Every field been sent and prefixed with an underscore "_" will be treated as an - * additional field. - */ - private static final String ADDITIONAL_FIELD_PREFIX = "_"; - /** * Libraries SHOULD not allow to send id as additional field ("_id"). Graylog server * nodes omit this field automatically. @@ -78,9 +76,8 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure private static void jsonMembers(Environment environment, ThrowableProxyConverter throwableProxyConverter, JsonWriter.Members members) { members.add("version", "1.1"); - // note: a blank message will lead to a Graylog error as of Graylog v6.0.x. We are - // ignoring this here. - members.add("short_message", ILoggingEvent::getFormattedMessage); + members.add("short_message", ILoggingEvent::getFormattedMessage) + .as(GraylogExtendedLogFormatStructuredLogFormatter::getMessageText); members.add("timestamp", ILoggingEvent::getTimeStamp) .as(GraylogExtendedLogFormatStructuredLogFormatter::formatTimeStamp); members.add("level", LevelToSyslogSeverity::convert); @@ -95,15 +92,15 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure .usingPairs((mdc, pairs) -> mdc.forEach((key, value) -> createAdditionalField(key, value, pairs))); members.from(ILoggingEvent::getKeyValuePairs) .when((keyValuePairs) -> !CollectionUtils.isEmpty(keyValuePairs)) - .usingPairs((keyValuePairs, pairs) -> keyValuePairs - .forEach((keyValuePair) -> createAdditionalField(keyValuePair.key, keyValuePair.value, pairs))); - members.add().whenNotNull(ILoggingEvent::getThrowableProxy).usingMembers((throwableMembers) -> { - throwableMembers.add("full_message", - (event) -> formatFullMessageWithThrowable(throwableProxyConverter, event)); - throwableMembers.add("_error_type", ILoggingEvent::getThrowableProxy).as(IThrowableProxy::getClassName); - throwableMembers.add("_error_stack_trace", throwableProxyConverter::convert); - throwableMembers.add("_error_message", ILoggingEvent::getThrowableProxy).as(IThrowableProxy::getMessage); - }); + .usingPairs(GraylogExtendedLogFormatStructuredLogFormatter::createAdditionalField); + members.add() + .whenNotNull(ILoggingEvent::getThrowableProxy) + .usingMembers((throwableMembers) -> throwableMembers(throwableMembers, throwableProxyConverter)); + } + + private static String getMessageText(String formattedMessage) { + // Always return text as a blank message will lead to a error as of Graylog v6 + return (!StringUtils.hasText(formattedMessage)) ? "(blank)" : formattedMessage; } /** @@ -117,23 +114,38 @@ class GraylogExtendedLogFormatStructuredLogFormatter extends JsonWriterStructure return (out) -> out.append(new BigDecimal(timeStamp).movePointLeft(3).toPlainString()); } + private static void throwableMembers(Members members, + ThrowableProxyConverter throwableProxyConverter) { + members.add("full_message", (event) -> formatFullMessageWithThrowable(throwableProxyConverter, event)); + members.add("_error_type", ILoggingEvent::getThrowableProxy).as(IThrowableProxy::getClassName); + members.add("_error_stack_trace", throwableProxyConverter::convert); + members.add("_error_message", ILoggingEvent::getThrowableProxy).as(IThrowableProxy::getMessage); + } + private static String formatFullMessageWithThrowable(ThrowableProxyConverter throwableProxyConverter, ILoggingEvent event) { return event.getFormattedMessage() + "\n\n" + throwableProxyConverter.convert(event); } - private static void createAdditionalField(String key, Object value, BiConsumer pairs) { - Assert.notNull(key, "fieldName must not be null"); - if (!FIELD_NAME_VALID_PATTERN.matcher(key).matches()) { - logger.warn(LogMessage.format("'%s' is not a valid field name according to GELF standard", key)); + private static void createAdditionalField(List keyValuePairs, BiConsumer pairs) { + keyValuePairs.forEach((keyValuePair) -> createAdditionalField(keyValuePair.key, keyValuePair.value, pairs)); + } + + private static void createAdditionalField(String name, Object value, BiConsumer pairs) { + Assert.notNull(name, "fieldName must not be null"); + if (!FIELD_NAME_VALID_PATTERN.matcher(name).matches()) { + logger.warn(LogMessage.format("'%s' is not a valid field name according to GELF standard", name)); return; } - if (ADDITIONAL_FIELD_ILLEGAL_KEYS.contains(key)) { - logger.warn(LogMessage.format("'%s' is an illegal field name according to GELF standard", key)); + if (ADDITIONAL_FIELD_ILLEGAL_KEYS.contains(name)) { + logger.warn(LogMessage.format("'%s' is an illegal field name according to GELF standard", name)); return; } - String keyWithPrefix = (key.startsWith(ADDITIONAL_FIELD_PREFIX)) ? key : ADDITIONAL_FIELD_PREFIX + key; - pairs.accept(keyWithPrefix, value); + pairs.accept(asAdditionalFieldName(name), value); + } + + private static Object asAdditionalFieldName(String name) { + return (!name.startsWith("_")) ? "_" + name : name; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/StructuredLogEncoder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/StructuredLogEncoder.java index 5b3c9d964e..746251e1e0 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/StructuredLogEncoder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/StructuredLogEncoder.java @@ -28,6 +28,7 @@ import org.springframework.boot.logging.structured.CommonStructuredLogFormat; import org.springframework.boot.logging.structured.StructuredLogFormatter; import org.springframework.boot.logging.structured.StructuredLogFormatterFactory; import org.springframework.boot.logging.structured.StructuredLogFormatterFactory.CommonFormatters; +import org.springframework.boot.util.Instantiator; import org.springframework.boot.util.Instantiator.AvailableParameters; import org.springframework.core.env.Environment; import org.springframework.util.Assert; @@ -79,16 +80,29 @@ public class StructuredLogEncoder extends EncoderBase { } private void addCommonFormatters(CommonFormatters commonFormatters) { - commonFormatters.add(CommonStructuredLogFormat.ELASTIC_COMMON_SCHEMA, - (instantiator) -> new ElasticCommonSchemaStructuredLogFormatter(instantiator.getArg(Environment.class), - instantiator.getArg(ThrowableProxyConverter.class))); - commonFormatters - .add(CommonStructuredLogFormat.GRAYLOG_EXTENDED_LOG_FORMAT, - (instantiator) -> new GraylogExtendedLogFormatStructuredLogFormatter( - instantiator.getArg(Environment.class), - instantiator.getArg(ThrowableProxyConverter.class))); - commonFormatters.add(CommonStructuredLogFormat.LOGSTASH, (instantiator) -> new LogstashStructuredLogFormatter( - instantiator.getArg(ThrowableProxyConverter.class))); + commonFormatters.add(CommonStructuredLogFormat.ELASTIC_COMMON_SCHEMA, this::createEcsFormatter); + commonFormatters.add(CommonStructuredLogFormat.GRAYLOG_EXTENDED_LOG_FORMAT, this::createGraylogFormatter); + commonFormatters.add(CommonStructuredLogFormat.LOGSTASH, this::createLogstashFormatter); + } + + private StructuredLogFormatter createEcsFormatter( + Instantiator> instantiator) { + Environment environment = instantiator.getArg(Environment.class); + ThrowableProxyConverter throwableProxyConverter = instantiator.getArg(ThrowableProxyConverter.class); + return new ElasticCommonSchemaStructuredLogFormatter(environment, throwableProxyConverter); + } + + private StructuredLogFormatter createGraylogFormatter( + Instantiator> instantiator) { + Environment environment = instantiator.getArg(Environment.class); + ThrowableProxyConverter throwableProxyConverter = instantiator.getArg(ThrowableProxyConverter.class); + return new GraylogExtendedLogFormatStructuredLogFormatter(environment, throwableProxyConverter); + } + + private StructuredLogFormatter createLogstashFormatter( + Instantiator> instantiator) { + ThrowableProxyConverter throwableProxyConverter = instantiator.getArg(ThrowableProxyConverter.class); + return new LogstashStructuredLogFormatter(throwableProxyConverter); } @Override diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/structured/GraylogExtendedLogFormatService.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/structured/GraylogExtendedLogFormatService.java index bbe854a1a9..8e10f4831e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/structured/GraylogExtendedLogFormatService.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/structured/GraylogExtendedLogFormatService.java @@ -48,7 +48,6 @@ public record GraylogExtendedLogFormatService(String name, String version) { * @param members the members to add to */ public void jsonMembers(JsonWriter.Members members) { - // note "host" is a field name prescribed by GELF members.add("host", this::name).whenHasLength(); members.add("_service_version", this::version).whenHasLength(); } @@ -65,4 +64,5 @@ public record GraylogExtendedLogFormatService(String name, String version) { .orElse(NONE) .withDefaults(environment); } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatterTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatterTests.java index 39ab27c638..2b2c039d73 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatterTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/GraylogExtendedLogFormatStructuredLogFormatterTests.java @@ -134,7 +134,6 @@ class GraylogExtendedLogFormatStructuredLogFormatterTests extends AbstractStruct .formatted()); assertThat(deserialized) .containsAllEntriesOf(map("_error_type", "java.lang.RuntimeException", "_error_message", "Boom")); - assertThat(stackTrace).startsWith( "java.lang.RuntimeException: Boom%n\tat org.springframework.boot.logging.logback.GraylogExtendedLogFormatStructuredLogFormatterTests.shouldFormatException" .formatted());