diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/otlp/OtlpMetricsPropertiesConfigAdapter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/otlp/OtlpMetricsPropertiesConfigAdapter.java index dbbcd6efe244..8ad40855835f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/otlp/OtlpMetricsPropertiesConfigAdapter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/otlp/OtlpMetricsPropertiesConfigAdapter.java @@ -17,6 +17,7 @@ package org.springframework.boot.actuate.autoconfigure.metrics.export.otlp; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -28,7 +29,6 @@ import org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryProperties; import org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryResourceAttributes; import org.springframework.core.env.Environment; -import org.springframework.util.StringUtils; /** * Adapter to convert {@link OtlpMetricsProperties} to an {@link OtlpConfig}. @@ -40,11 +40,6 @@ class OtlpMetricsPropertiesConfigAdapter extends StepRegistryPropertiesConfigAdapter implements OtlpConfig { - /** - * Default value for application name if {@code spring.application.name} is not set. - */ - private static final String DEFAULT_APPLICATION_NAME = "unknown_service"; - private final OpenTelemetryProperties openTelemetryProperties; private final OtlpMetricsConnectionDetails connectionDetails; @@ -77,21 +72,10 @@ public AggregationTemporality aggregationTemporality() { @Override public Map resourceAttributes() { - Map attributes = new OpenTelemetryResourceAttributes( - this.openTelemetryProperties.getResourceAttributes()) - .asMap(); - attributes.computeIfAbsent("service.name", (key) -> getApplicationName()); - attributes.computeIfAbsent("service.group", (key) -> getApplicationGroup()); - return Collections.unmodifiableMap(attributes); - } - - private String getApplicationName() { - return this.environment.getProperty("spring.application.name", DEFAULT_APPLICATION_NAME); - } - - private String getApplicationGroup() { - String applicationGroup = this.environment.getProperty("spring.application.group"); - return (StringUtils.hasLength(applicationGroup)) ? applicationGroup : null; + Map resourceAttributes = new LinkedHashMap<>(); + new OpenTelemetryResourceAttributes(this.environment, this.openTelemetryProperties.getResourceAttributes()) + .applyTo(resourceAttributes::put); + return Collections.unmodifiableMap(resourceAttributes); } @Override diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryAutoConfiguration.java index 173b76ca83bb..c19a7c8a9579 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryAutoConfiguration.java @@ -16,8 +16,6 @@ package org.springframework.boot.actuate.autoconfigure.opentelemetry; -import java.util.Map; - import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.sdk.OpenTelemetrySdk; @@ -36,7 +34,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.core.env.Environment; -import org.springframework.util.StringUtils; /** * {@link EnableAutoConfiguration Auto-configuration} for OpenTelemetry. @@ -49,11 +46,6 @@ @EnableConfigurationProperties(OpenTelemetryProperties.class) public class OpenTelemetryAutoConfiguration { - /** - * Default value for application name if {@code spring.application.name} is not set. - */ - private static final String DEFAULT_APPLICATION_NAME = "unknown_service"; - @Bean @ConditionalOnMissingBean(OpenTelemetry.class) OpenTelemetrySdk openTelemetry(ObjectProvider tracerProvider, @@ -76,21 +68,8 @@ Resource openTelemetryResource(Environment environment, OpenTelemetryProperties private Resource toResource(Environment environment, OpenTelemetryProperties properties) { ResourceBuilder builder = Resource.builder(); - Map attributes = new OpenTelemetryResourceAttributes(properties.getResourceAttributes()) - .asMap(); - attributes.computeIfAbsent("service.name", (key) -> getApplicationName(environment)); - attributes.computeIfAbsent("service.group", (key) -> getApplicationGroup(environment)); - attributes.forEach(builder::put); + new OpenTelemetryResourceAttributes(environment, properties.getResourceAttributes()).applyTo(builder::put); return builder.build(); } - private String getApplicationName(Environment environment) { - return environment.getProperty("spring.application.name", DEFAULT_APPLICATION_NAME); - } - - private String getApplicationGroup(Environment environment) { - String applicationGroup = environment.getProperty("spring.application.group"); - return (StringUtils.hasLength(applicationGroup)) ? applicationGroup : null; - } - } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributes.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributes.java index d6ba172e0dcc..52c66a44856e 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributes.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributes.java @@ -21,16 +21,19 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.BiConsumer; import java.util.function.Function; +import org.springframework.core.env.Environment; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** - * OpenTelemetryResourceAttributes retrieves information from the + * {@link OpenTelemetryResourceAttributes} retrieves information from the * {@code OTEL_RESOURCE_ATTRIBUTES} and {@code OTEL_SERVICE_NAME} environment variables - * and merges it with the resource attributes provided by the user. - *

- * User-provided resource attributes take precedence. + * and merges it with the resource attributes provided by the user. User-provided resource + * attributes take precedence. Additionally, {@code spring.application.*} related + * properties can be applied as defaults. *

* OpenTelemetry * Resource Specification @@ -40,47 +43,72 @@ */ public final class OpenTelemetryResourceAttributes { + /** + * Default value for service name if {@code service.name} is not set. + */ + private static final String DEFAULT_SERVICE_NAME = "unknown_service"; + + private final Environment environment; + private final Map resourceAttributes; private final Function getEnv; /** * Creates a new instance of {@link OpenTelemetryResourceAttributes}. + * @param environment the environment * @param resourceAttributes user provided resource attributes to be used */ - public OpenTelemetryResourceAttributes(Map resourceAttributes) { - this(resourceAttributes, null); + public OpenTelemetryResourceAttributes(Environment environment, Map resourceAttributes) { + this(environment, resourceAttributes, null); } /** * Creates a new {@link OpenTelemetryResourceAttributes} instance. + * @param environment the environment * @param resourceAttributes user provided resource attributes to be used * @param getEnv a function to retrieve environment variables by name */ - OpenTelemetryResourceAttributes(Map resourceAttributes, Function getEnv) { + OpenTelemetryResourceAttributes(Environment environment, Map resourceAttributes, + Function getEnv) { + Assert.notNull(environment, "'environment' must not be null"); + this.environment = environment; this.resourceAttributes = (resourceAttributes != null) ? resourceAttributes : Collections.emptyMap(); this.getEnv = (getEnv != null) ? getEnv : System::getenv; } /** - * Returns resource attributes by combining attributes from environment variables and - * user-defined resource attributes. The final resource contains all attributes from - * both sources. + * Applies resource attributes to the provided BiConsumer after being combined from + * environment variables and user-defined resource attributes. *

* If a key exists in both environment variables and user-defined resources, the value * from the user-defined resource takes precedence, even if it is empty. *

- * Null keys and values are ignored. - * @return the resource attributes + * Additionally, {@code spring.application.name} or {@code unknown_service} will be + * used as the default for {@code service.name}, and {@code spring.application.group} + * will serve as the default for {@code service.group}. + * @param consumer the {@link BiConsumer} to apply */ - public Map asMap() { + public void applyTo(BiConsumer consumer) { + Assert.notNull(consumer, "'consumer' must not be null"); Map attributes = getResourceAttributesFromEnv(); this.resourceAttributes.forEach((name, value) -> { - if (name != null && value != null) { + if (StringUtils.hasLength(name) && value != null) { attributes.put(name, value); } }); - return attributes; + attributes.computeIfAbsent("service.name", (k) -> getApplicationName()); + attributes.computeIfAbsent("service.group", (k) -> getApplicationGroup()); + attributes.forEach(consumer); + } + + private String getApplicationName() { + return this.environment.getProperty("spring.application.name", DEFAULT_SERVICE_NAME); + } + + private String getApplicationGroup() { + String applicationGroup = this.environment.getProperty("spring.application.group"); + return (StringUtils.hasLength(applicationGroup)) ? applicationGroup : null; } /** @@ -122,7 +150,7 @@ private String getEnv(String name) { * @param value value to decode * @return the decoded string */ - public static String decode(String value) { + private static String decode(String value) { if (value.indexOf('%') < 0) { return value; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributesTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributesTests.java index 799ea7bc2605..973d2a40ac95 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributesTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributesTests.java @@ -27,6 +27,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.springframework.mock.env.MockEnvironment; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -41,6 +43,8 @@ class OpenTelemetryResourceAttributesTests { private static final PercentEscaper escaper = PercentEscaper.create(); + private final MockEnvironment environment = new MockEnvironment(); + private final Map environmentVariables = new LinkedHashMap<>(); private final Map resourceAttributes = new LinkedHashMap<>(); @@ -48,7 +52,6 @@ class OpenTelemetryResourceAttributesTests { @BeforeAll static void beforeAll() { long seed = new Random().nextLong(); - System.out.println("Seed: " + seed); random = new Random(seed); } @@ -56,40 +59,37 @@ static void beforeAll() { void otelServiceNameShouldTakePrecedenceOverOtelResourceAttributes() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=ignored"); this.environmentVariables.put("OTEL_SERVICE_NAME", "otel-service"); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).hasSize(1).containsEntry("service.name", "otel-service"); + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", "otel-service"); } @Test void otelServiceNameWhenEmptyShouldTakePrecedenceOverOtelResourceAttributes() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=ignored"); this.environmentVariables.put("OTEL_SERVICE_NAME", ""); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).hasSize(1).containsEntry("service.name", ""); + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", ""); } @Test - void otelResourceAttributesShouldBeUsed() { + void otelResourceAttributes() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", ", ,,key1=value1,key2= value2, key3=value3,key4=,=value5,key6,=,key7=spring+boot,key8=ś"); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).hasSize(6) + assertThat(getAttributes()).hasSize(7) .containsEntry("key1", "value1") .containsEntry("key2", "value2") .containsEntry("key3", "value3") .containsEntry("key4", "") .containsEntry("key7", "spring+boot") - .containsEntry("key8", "ś"); + .containsEntry("key8", "ś") + .containsEntry("service.name", "unknown_service"); } @Test - void resourceAttributesShouldBeMergedWithEnvironmentVariables() { + void resourceAttributesShouldBeMergedWithEnvironmentVariablesAndTakePrecedence() { this.resourceAttributes.put("service.group", "custom-group"); this.resourceAttributes.put("key2", ""); this.environmentVariables.put("OTEL_SERVICE_NAME", "custom-service"); this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key1=value1,key2=value2"); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).hasSize(4) + assertThat(getAttributes()).hasSize(4) .containsEntry("service.name", "custom-service") .containsEntry("service.group", "custom-group") .containsEntry("key1", "value1") @@ -97,27 +97,20 @@ void resourceAttributesShouldBeMergedWithEnvironmentVariables() { } @Test - void resourceAttributesWithNullKeyOrValueShouldBeIgnored() { - this.resourceAttributes.put("service.group", null); - this.resourceAttributes.put("service.name", null); - this.resourceAttributes.put(null, "value"); - this.environmentVariables.put("OTEL_SERVICE_NAME", "custom-service"); - this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key1=value1,key2=value2"); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).hasSize(3) - .containsEntry("service.name", "custom-service") - .containsEntry("key1", "value1") - .containsEntry("key2", "value2"); + void invalidResourceAttributesShouldBeIgnored() { + this.resourceAttributes.put("", "empty-key"); + this.resourceAttributes.put(null, "null-key"); + this.resourceAttributes.put("null-value", null); + this.resourceAttributes.put("empty-value", ""); + assertThat(getAttributes()).hasSize(2) + .containsEntry("service.name", "unknown_service") + .containsEntry("empty-value", ""); } @Test @SuppressWarnings("unchecked") - void systemGetEnvShouldBeUsedAsDefaultEnvFunctionAndResourceAttributesAreEmpty() { - OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(null); - assertThat(attributes).extracting("resourceAttributes") - .asInstanceOf(InstanceOfAssertFactories.MAP) - .isNotNull() - .isEmpty(); + void systemGetEnvShouldBeUsedAsDefaultEnvFunction() { + OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(this.environment, null); Function getEnv = assertThat(attributes).extracting("getEnv") .asInstanceOf(InstanceOfAssertFactories.type(Function.class)) .actual(); @@ -125,38 +118,98 @@ void systemGetEnvShouldBeUsedAsDefaultEnvFunctionAndResourceAttributesAreEmpty() } @Test - void shouldDecodeOtelResourceAttributeValues() { + void otelResourceAttributeValuesShouldBePercentDecoded() { Stream.generate(this::generateRandomString).limit(10000).forEach((value) -> { - String key = "key"; - this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", key + "=" + escaper.escape(value)); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).hasSize(1).containsEntry(key, value); + this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=" + escaper.escape(value)); + assertThat(getAttributes()).hasSize(2) + .containsEntry("service.name", "unknown_service") + .containsEntry("key", value); }); } @Test - void shouldThrowIllegalArgumentExceptionWhenDecodingPercentIllegalHexChar() { + void illegalArgumentExceptionShouldBeThrownWhenDecodingIllegalHexCharPercentEncodedValue() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=abc%ß"); - assertThatIllegalArgumentException().isThrownBy(() -> getAttributes().asMap()) + assertThatIllegalArgumentException().isThrownBy(this::getAttributes) .withMessage("Failed to decode percent-encoded characters at index 3 in the value: 'abc%ß'"); } @Test - void shouldUseReplacementCharWhenDecodingNonUtf8Character() { + void replacementCharShouldBeUsedWhenDecodingNonUtf8Character() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=%a3%3e"); - OpenTelemetryResourceAttributes attributes = getAttributes(); - assertThat(attributes.asMap()).containsEntry("key", "\ufffd>"); + assertThat(getAttributes()).containsEntry("key", "\ufffd>"); } @Test - void shouldThrowIllegalArgumentExceptionWhenDecodingPercent() { + void illegalArgumentExceptionShouldBeThrownWhenDecodingInvalidPercentEncodedValue() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=%"); - assertThatIllegalArgumentException().isThrownBy(() -> getAttributes().asMap()) + assertThatIllegalArgumentException().isThrownBy(this::getAttributes) .withMessage("Failed to decode percent-encoded characters at index 0 in the value: '%'"); } - private OpenTelemetryResourceAttributes getAttributes() { - return new OpenTelemetryResourceAttributes(this.resourceAttributes, this.environmentVariables::get); + @Test + void unknownServiceShouldBeUsedAsDefaultServiceName() { + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", "unknown_service"); + } + + @Test + void springApplicationGroupNameShouldBeUsedAsDefaultServiceGroup() { + this.environment.setProperty("spring.application.group", "spring-boot"); + assertThat(getAttributes()).hasSize(2) + .containsEntry("service.name", "unknown_service") + .containsEntry("service.group", "spring-boot"); + } + + @Test + void springApplicationNameShouldBeUsedAsDefaultServiceName() { + this.environment.setProperty("spring.application.name", "spring-boot-app"); + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", "spring-boot-app"); + } + + @Test + void resourceAttributesShouldTakePrecedenceOverSpringApplicationName() { + this.resourceAttributes.put("service.name", "spring-boot"); + this.environment.setProperty("spring.application.name", "spring-boot-app"); + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", "spring-boot"); + } + + @Test + void otelResourceAttributesShouldTakePrecedenceOverSpringApplicationName() { + this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=spring-boot"); + this.environment.setProperty("spring.application.name", "spring-boot-app"); + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", "spring-boot"); + } + + @Test + void otelServiceNameShouldTakePrecedenceOverSpringApplicationName() { + this.environmentVariables.put("OTEL_SERVICE_NAME", "spring-boot"); + this.environment.setProperty("spring.application.name", "spring-boot-app"); + assertThat(getAttributes()).hasSize(1).containsEntry("service.name", "spring-boot"); + } + + @Test + void resourceAttributesShouldTakePrecedenceOverSpringApplicationGroupName() { + this.resourceAttributes.put("service.group", "spring-boot-app"); + this.environment.setProperty("spring.application.group", "spring-boot"); + assertThat(getAttributes()).hasSize(2) + .containsEntry("service.name", "unknown_service") + .containsEntry("service.group", "spring-boot-app"); + } + + @Test + void otelResourceAttributesShouldTakePrecedenceOverSpringApplicationGroupName() { + this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "service.group=spring-boot"); + this.environment.setProperty("spring.application.group", "spring-boot-app"); + assertThat(getAttributes()).hasSize(2) + .containsEntry("service.name", "unknown_service") + .containsEntry("service.group", "spring-boot"); + } + + private Map getAttributes() { + Map attributes = new LinkedHashMap<>(); + new OpenTelemetryResourceAttributes(this.environment, this.resourceAttributes, this.environmentVariables::get) + .applyTo(attributes::put); + return attributes; } private String generateRandomString() {