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..76becebcd14d 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 @@ -77,12 +77,11 @@ 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); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes.fromEnv(); + attributes.putAll(this.openTelemetryProperties.getResourceAttributes()); + attributes.putIfAbsent("service.name", this::getApplicationName); + attributes.putIfAbsent("service.group", this::getApplicationGroup); + return Collections.unmodifiableMap(attributes.asMap()); } private String getApplicationName() { 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..cc714a59a9d4 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; @@ -76,11 +74,11 @@ 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); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes.fromEnv(); + attributes.putAll(properties.getResourceAttributes()); + attributes.putIfAbsent("service.name", () -> getApplicationName(environment)); + attributes.putIfAbsent("service.group", () -> getApplicationGroup(environment)); + attributes.asMap().forEach(builder::put); return builder.build(); } 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..8cb5192da410 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 @@ -18,100 +18,135 @@ import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.function.Function; +import java.util.function.Supplier; +import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; +import org.springframework.util.function.SupplierUtils; /** - * 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. + * {@link OpenTelemetryResourceAttributes} for managing OpenTelemetry resource attributes + * and provides convenient API to construct, modify, and merge attributes from different + * sources. *

* OpenTelemetry * Resource Specification * * @author Dmytro Nosan * @since 3.5.0 + * @see #fromEnv() */ public final class OpenTelemetryResourceAttributes { - private final Map resourceAttributes; - - private final Function getEnv; + private final Map attributes = new LinkedHashMap<>(); /** - * Creates a new instance of {@link OpenTelemetryResourceAttributes}. - * @param resourceAttributes user provided resource attributes to be used + * Return Resource attributes as a Map. + * @return the resource attributes as key-value pairs */ - public OpenTelemetryResourceAttributes(Map resourceAttributes) { - this(resourceAttributes, null); + public Map asMap() { + return new LinkedHashMap<>(this.attributes); } /** - * Creates a new {@link OpenTelemetryResourceAttributes} instance. - * @param resourceAttributes user provided resource attributes to be used - * @param getEnv a function to retrieve environment variables by name + * Adds a name-value pair to the resource attributes. Both the name and value will be + * trimmed. + * @param name the attribute name to add, must not be null or empty + * @param value the attribute value to add, must not be null */ - OpenTelemetryResourceAttributes(Map resourceAttributes, Function getEnv) { - this.resourceAttributes = (resourceAttributes != null) ? resourceAttributes : Collections.emptyMap(); - this.getEnv = (getEnv != null) ? getEnv : System::getenv; + public void put(String name, String value) { + if (StringUtils.hasText(name) && value != null) { + this.attributes.put(name.trim(), value.trim()); + } } /** - * Returns resource attributes by combining attributes from environment variables and - * user-defined resource attributes. The final resource contains all attributes from - * both sources. + * Merge attributes with the provided resource attributes. + *

+ * If a key exists in both, the value from provided resource takes precedence, even if + * it is empty. *

- * 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. + * Keys that are null or empty will be ignored, and all keys will be trimmed. *

- * Null keys and values are ignored. - * @return the resource attributes + * Values that are null will be ignored, and all values will be trimmed. + * @param resourceAttributes resource attributes */ - public Map asMap() { - Map attributes = getResourceAttributesFromEnv(); - this.resourceAttributes.forEach((name, value) -> { - if (name != null && value != null) { - attributes.put(name, value); - } - }); - return attributes; + public void putAll(Map resourceAttributes) { + if (!CollectionUtils.isEmpty(resourceAttributes)) { + resourceAttributes.forEach(this::put); + } + } + + /** + * Adds a name-value pair to the resource attributes. Both the name and supplied value + * will be trimmed. + * @param name the attribute name to add, must not be null or empty + * @param valueSupplier the attribute value supplier + */ + public void putIfAbsent(String name, Supplier valueSupplier) { + if (!contains(name)) { + put(name, SupplierUtils.resolve(valueSupplier)); + } + } + + @Override + public boolean equals(Object obj) { + if (obj == null || getClass() != obj.getClass()) { + return false; + } + OpenTelemetryResourceAttributes that = (OpenTelemetryResourceAttributes) obj; + return this.attributes.equals(that.attributes); + } + + @Override + public int hashCode() { + return this.attributes.hashCode(); + } + + @Override + public String toString() { + return this.attributes.toString(); } /** - * Parses resource attributes from the {@link System#getenv()}. This method fetches - * attributes defined in the {@code OTEL_RESOURCE_ATTRIBUTES} and - * {@code OTEL_SERVICE_NAME} environment variables and provides them as key-value - * pairs. + * Creates an {@link OpenTelemetryResourceAttributes} instance based on environment + * variables. This method fetches attributes defined in the + * {@code OTEL_RESOURCE_ATTRIBUTES} and {@code OTEL_SERVICE_NAME} environment + * variables. *

* If {@code service.name} is also provided in {@code OTEL_RESOURCE_ATTRIBUTES}, then * {@code OTEL_SERVICE_NAME} takes precedence. - * @return resource attributes + * @return an {@link OpenTelemetryResourceAttributes} */ - private Map getResourceAttributesFromEnv() { - Map attributes = new LinkedHashMap<>(); - for (String attribute : StringUtils.tokenizeToStringArray(getEnv("OTEL_RESOURCE_ATTRIBUTES"), ",")) { + public static OpenTelemetryResourceAttributes fromEnv() { + return fromEnv(System::getenv); + } + + static OpenTelemetryResourceAttributes fromEnv(Function getEnv) { + OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(); + for (String attribute : StringUtils.tokenizeToStringArray(getEnv.apply("OTEL_RESOURCE_ATTRIBUTES"), ",")) { int index = attribute.indexOf('='); if (index > 0) { String key = attribute.substring(0, index); String value = attribute.substring(index + 1); - attributes.put(key.trim(), decode(value.trim())); + attributes.put(key, decode(value)); } } - String otelServiceName = getEnv("OTEL_SERVICE_NAME"); + String otelServiceName = getEnv.apply("OTEL_SERVICE_NAME"); if (otelServiceName != null) { attributes.put("service.name", otelServiceName); } return attributes; } - private String getEnv(String name) { - return this.getEnv.apply(name); + private boolean contains(String name) { + if (!StringUtils.hasText(name)) { + return false; + } + return this.attributes.containsKey(name.trim()); } /** @@ -122,7 +157,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..bdde6864196d 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 @@ -19,11 +19,9 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.Random; -import java.util.function.Function; import java.util.stream.Stream; import io.opentelemetry.api.internal.PercentEscaper; -import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -48,15 +46,15 @@ class OpenTelemetryResourceAttributesTests { @BeforeAll static void beforeAll() { long seed = new Random().nextLong(); - System.out.println("Seed: " + seed); random = new Random(seed); } @Test void otelServiceNameShouldTakePrecedenceOverOtelResourceAttributes() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=ignored"); - this.environmentVariables.put("OTEL_SERVICE_NAME", "otel-service"); - OpenTelemetryResourceAttributes attributes = getAttributes(); + this.environmentVariables.put("OTEL_SERVICE_NAME", " otel-service "); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); assertThat(attributes.asMap()).hasSize(1).containsEntry("service.name", "otel-service"); } @@ -64,15 +62,17 @@ void otelServiceNameShouldTakePrecedenceOverOtelResourceAttributes() { void otelServiceNameWhenEmptyShouldTakePrecedenceOverOtelResourceAttributes() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=ignored"); this.environmentVariables.put("OTEL_SERVICE_NAME", ""); - OpenTelemetryResourceAttributes attributes = getAttributes(); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); assertThat(attributes.asMap()).hasSize(1).containsEntry("service.name", ""); } @Test - void otelResourceAttributesShouldBeUsed() { + void otelResourceAttributesShouldBeCreatedGet() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", ", ,,key1=value1,key2= value2, key3=value3,key4=,=value5,key6,=,key7=spring+boot,key8=ś"); - OpenTelemetryResourceAttributes attributes = getAttributes(); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); assertThat(attributes.asMap()).hasSize(6) .containsEntry("key1", "value1") .containsEntry("key2", "value2") @@ -85,10 +85,12 @@ void otelResourceAttributesShouldBeUsed() { @Test void resourceAttributesShouldBeMergedWithEnvironmentVariables() { this.resourceAttributes.put("service.group", "custom-group"); - this.resourceAttributes.put("key2", ""); + 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(); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); + attributes.putAll(this.resourceAttributes); assertThat(attributes.asMap()).hasSize(4) .containsEntry("service.name", "custom-service") .containsEntry("service.group", "custom-group") @@ -97,13 +99,16 @@ void resourceAttributesShouldBeMergedWithEnvironmentVariables() { } @Test - void resourceAttributesWithNullKeyOrValueShouldBeIgnored() { + void resourceAttributesWithBlankKeyAndNullValueShouldBeIgnoredWhenMergingWithEnvironmentVariables() { this.resourceAttributes.put("service.group", null); this.resourceAttributes.put("service.name", null); - this.resourceAttributes.put(null, "value"); + this.resourceAttributes.put(null, "null-key"); + this.resourceAttributes.put(" ", "blank-key"); this.environmentVariables.put("OTEL_SERVICE_NAME", "custom-service"); this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key1=value1,key2=value2"); - OpenTelemetryResourceAttributes attributes = getAttributes(); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); + attributes.putAll(this.resourceAttributes); assertThat(attributes.asMap()).hasSize(3) .containsEntry("service.name", "custom-service") .containsEntry("key1", "value1") @@ -111,52 +116,60 @@ void resourceAttributesWithNullKeyOrValueShouldBeIgnored() { } @Test - @SuppressWarnings("unchecked") - void systemGetEnvShouldBeUsedAsDefaultEnvFunctionAndResourceAttributesAreEmpty() { - OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(null); - assertThat(attributes).extracting("resourceAttributes") - .asInstanceOf(InstanceOfAssertFactories.MAP) - .isNotNull() - .isEmpty(); - Function getEnv = assertThat(attributes).extracting("getEnv") - .asInstanceOf(InstanceOfAssertFactories.type(Function.class)) - .actual(); - System.getenv().forEach((key, value) -> assertThat(getEnv.apply(key)).isEqualTo(value)); - } - - @Test - void shouldDecodeOtelResourceAttributeValues() { + void otelResourceAttributesGetValueShouldBeDecoded() { 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); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); + assertThat(attributes.asMap()).hasSize(1).containsEntry(key, value.trim()); }); } @Test void shouldThrowIllegalArgumentExceptionWhenDecodingPercentIllegalHexChar() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=abc%ß"); - assertThatIllegalArgumentException().isThrownBy(() -> getAttributes().asMap()) + assertThatIllegalArgumentException() + .isThrownBy(() -> OpenTelemetryResourceAttributes.fromEnv(this.environmentVariables::get)) .withMessage("Failed to decode percent-encoded characters at index 3 in the value: 'abc%ß'"); } @Test void shouldUseReplacementCharWhenDecodingNonUtf8Character() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=%a3%3e"); - OpenTelemetryResourceAttributes attributes = getAttributes(); + OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes + .fromEnv(this.environmentVariables::get); assertThat(attributes.asMap()).containsEntry("key", "\ufffd>"); } @Test void shouldThrowIllegalArgumentExceptionWhenDecodingPercent() { this.environmentVariables.put("OTEL_RESOURCE_ATTRIBUTES", "key=%"); - assertThatIllegalArgumentException().isThrownBy(() -> getAttributes().asMap()) + assertThatIllegalArgumentException() + .isThrownBy(() -> OpenTelemetryResourceAttributes.fromEnv(this.environmentVariables::get)) .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 shouldPutValueIfKeyIsAbsentAndNotBlankAndValueNotNull() { + OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(); + attributes.put("key", "value"); + attributes.putIfAbsent(" key ", () -> "value1"); + attributes.putIfAbsent(" key1 ", () -> "value1"); + attributes.putIfAbsent(" key2 ", () -> null); + attributes.putIfAbsent(null, () -> "value3"); + attributes.putIfAbsent(" ", () -> "value4"); + assertThat(attributes.asMap()).hasSize(2).containsEntry("key", "value").containsEntry("key1", "value1"); + } + + @Test + void shouldPutValueIfKeyNotBlankAndValueNotNull() { + OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(); + attributes.put("", "empty-key"); + attributes.put(null, "null-key"); + attributes.put("empty-value", " "); + attributes.put("null-value", null); + assertThat(attributes.asMap()).hasSize(1).containsEntry("empty-value", ""); } private String generateRandomString() {