Refine the handling of OpenTelemetry resource attributes

OpenTelemetryResourceAttributes now has convenient APIs to construct,
modify, and merge attributes from various sources, including environment
variables and user-defined attribute maps.

Signed-off-by: Dmytro Nosan <[email protected]>

Author: nosan

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<String, String> resourceAttributes() {
-		Map<String, String> 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() {

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<String, String> 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();
 	}
 

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/opentelemetry/OpenTelemetryResourceAttributes.java

@@ -18,100 +18,110 @@
 
 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.
- * <p>
- * <b>User-provided resource attributes take precedence.</b>
+ * {@link OpenTelemetryResourceAttributes} for managing OpenTelemetry resource attributes
+ * and provides convenient API to construct, modify, and merge attributes from different
+ * sources.
  * <p>
  * <a href= "https://opentelemetry.io/docs/specs/otel/resource/sdk/">OpenTelemetry
  * Resource Specification</a>
  *
  * @author Dmytro Nosan
  * @since 3.5.0
+ * @see #fromEnv()
  */
 public final class OpenTelemetryResourceAttributes {
 
-	private final Map<String, String> resourceAttributes;
-
-	private final Function<String, String> getEnv;
+	private final Map<String, String> 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<String, String> resourceAttributes) {
-		this(resourceAttributes, null);
+	public Map<String, String> 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
+	 * Merge attributes with the provided resource attributes.
+	 * <p>
+	 * If a key exists in both, the value from provided resource takes precedence, even if
+	 * it is empty.
+	 * <p>
+	 * <b>Keys that are null or empty will be ignored, and all keys will be trimmed.</b>
+	 * <p>
+	 * <b>Values that are null will be ignored, and all values will be trimmed.</b>
+	 * @param resourceAttributes resource attributes
 	 */
-	OpenTelemetryResourceAttributes(Map<String, String> resourceAttributes, Function<String, String> getEnv) {
-		this.resourceAttributes = (resourceAttributes != null) ? resourceAttributes : Collections.emptyMap();
-		this.getEnv = (getEnv != null) ? getEnv : System::getenv;
+	public void putAll(Map<String, String> resourceAttributes) {
+		if (!CollectionUtils.isEmpty(resourceAttributes)) {
+			resourceAttributes.forEach(this::put);
+		}
 	}
 
 	/**
-	 * Returns resource attributes by combining attributes from environment variables and
-	 * user-defined resource attributes. The final resource contains all attributes from
-	 * both sources.
-	 * <p>
-	 * 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.
-	 * <p>
-	 * <b>Null keys and values are ignored.</b>
-	 * @return the resource attributes
+	 * 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 Map<String, String> asMap() {
-		Map<String, String> attributes = getResourceAttributesFromEnv();
-		this.resourceAttributes.forEach((name, value) -> {
-			if (name != null && value != null) {
-				attributes.put(name, value);
-			}
-		});
-		return attributes;
+	public void putIfAbsent(String name, Supplier<String> valueSupplier) {
+		if (!contains(name)) {
+			put(name, SupplierUtils.resolve(valueSupplier));
+		}
 	}
 
 	/**
-	 * 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.
 	 * <p>
 	 * 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<String, String> getResourceAttributesFromEnv() {
-		Map<String, String> attributes = new LinkedHashMap<>();
-		for (String attribute : StringUtils.tokenizeToStringArray(getEnv("OTEL_RESOURCE_ATTRIBUTES"), ",")) {
+	public static OpenTelemetryResourceAttributes fromEnv() {
+		return fromEnv(System::getenv);
+	}
+
+	static OpenTelemetryResourceAttributes fromEnv(Function<String, String> 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 void put(String name, String value) {
+		if (StringUtils.hasText(name) && value != null) {
+			this.attributes.put(name.trim(), value.trim());
+		}
+	}
+
+	private boolean contains(String name) {
+		if (!StringUtils.hasText(name)) {
+			return false;
+		}
+		return this.attributes.containsKey(name.trim());
 	}
 
 	/**
@@ -122,7 +132,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;
 		}

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,47 +46,52 @@ 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");
 	}
 
 	@Test
 	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 resourceAttributesShouldBeCreatedFromEnvironmentVariables() {
 		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)
+				", ,,key1=value1,key2= value2, key3=value3,key4=,=value5,key6,=,key7=spring+boot,key8=ś,key9=%20A%20");
+		OpenTelemetryResourceAttributes attributes = OpenTelemetryResourceAttributes
+			.fromEnv(this.environmentVariables::get);
+		assertThat(attributes.asMap()).hasSize(7)
 			.containsEntry("key1", "value1")
 			.containsEntry("key2", "value2")
 			.containsEntry("key3", "value3")
 			.containsEntry("key4", "")
 			.containsEntry("key7", "spring+boot")
-			.containsEntry("key8", "ś");
+			.containsEntry("key8", "ś")
+			.containsEntry("key9", "A");
 	}
 
 	@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,66 +100,67 @@ 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")
 			.containsEntry("key2", "value2");
 	}
 
 	@Test
-	@SuppressWarnings("unchecked")
-	void systemGetEnvShouldBeUsedAsDefaultEnvFunctionAndResourceAttributesAreEmpty() {
-		OpenTelemetryResourceAttributes attributes = new OpenTelemetryResourceAttributes(null);
-		assertThat(attributes).extracting("resourceAttributes")
-			.asInstanceOf(InstanceOfAssertFactories.MAP)
-			.isNotNull()
-			.isEmpty();
-		Function<String, String> 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 otelResourceAttributesShouldBeDecoded() {
 		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.putIfAbsent("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");
 	}
 
 	private String generateRandomString() {