Skip to content

OtlpMetricsPropertiesConfigAdapter does not merge user's attributes with OTEL_RESOURCE_ATTRIBUTES #44400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nosan opened this issue Feb 22, 2025 · 4 comments
Labels
status: duplicate A duplicate of another issue

Comments

@nosan
Copy link
Contributor

nosan commented Feb 22, 2025

According to the OpenTelemetry specification (https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#specifying-resource-information-via-an-environment-variable), OTEL_RESOURCE_ATTRIBUTES must always be merged with the user-provided attributes.

Currently, OtlpMetricsPropertiesConfigAdapter uses the user's attributes if they are provided; otherwise, it falls back to the attributes from OTEL_RESOURCE_ATTRIBUTES.

https://github.com/nosan/spring-boot/blob/fdcc8d9d1f632f00dd71093d604c374b6d0a38d3/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/otlp/OtlpMetricsPropertiesConfigAdapter.java#L79-L87

As I understand it, the logic should be:

public Map<String, String> resourceAttributes() {
        // OTEL_RESOURCE_ATTRIBUTES
	Map<String, String> result = new LinkedHashMap<>(OtlpConfig.super.resourceAttributes());   
        //merge with user's attributes
	Map<String, String> resourceAttributes = this.openTelemetryProperties.getResourceAttributes();
	if (!CollectionUtils.isEmpty(resourceAttributes)) {
		result.putAll(resourceAttributes);
	}
	else if (this.properties.getResourceAttributes() != null) {
		result.putAll(this.properties.getResourceAttributes());
	}
        
	result.computeIfAbsent("service.name", (key) -> getApplicationName());
	result.computeIfAbsent("service.group", (key) -> getApplicationGroup());
	return Collections.unmodifiableMap(result);
}
@nosan nosan changed the title OtlpMetricsPropertiesConfigAdapter does not merge attributes with attributes from OTEL_RESOURCE_ATTRIBUTES OtlpMetricsPropertiesConfigAdapter does not merge user's attributes with OTEL_RESOURCE_ATTRIBUTES Feb 22, 2025
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 22, 2025
nosan added a commit to nosan/spring-boot that referenced this issue Feb 22, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
@nosan
Copy link
Contributor Author

nosan commented Feb 22, 2025

3.4.x...nosan:spring-boot:44400 includes a fix for this issue. The OpenTelemetryResourceAttributes can also be reused later for #43712.

nosan added a commit to nosan/spring-boot that referenced this issue Feb 23, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
nosan added a commit to nosan/spring-boot that referenced this issue Feb 23, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
nosan added a commit to nosan/spring-boot that referenced this issue Feb 23, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
nosan added a commit to nosan/spring-boot that referenced this issue Feb 23, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
nosan added a commit to nosan/spring-boot that referenced this issue Feb 23, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
nosan added a commit to nosan/spring-boot that referenced this issue Feb 23, 2025
Introduce OpenTelemetryResourceAttributes to merge user-defined
resource attributes with those from OTEL_RESOURCE_ATTRIBUTES.

See spring-projectsgh-44400

Signed-off-by: Dmytro Nosan <dimanosan@gmail.com>
@mhalbritter
Copy link
Contributor

Thanks @nosan, i've added a comment to #43712.

@mhalbritter mhalbritter added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 25, 2025
@nosan
Copy link
Contributor Author

nosan commented Feb 25, 2025

@mhalbritter

I’m a bit confused, to be honest. Just to clarify, would you like to include merge support for OtlpMetricsPropertiesConfigAdapter within the scope of #43712?

The focus of #43712 is on org.springframework.boot.actuate.autoconfigure.opentelemetry.OpenTelemetryAutoConfiguration, whereas the current issue is specifically to org.springframework.boot.actuate.autoconfigure.metrics.export.otlp.OtlpMetricsPropertiesConfigAdapter

@mhalbritter
Copy link
Contributor

mhalbritter commented Feb 25, 2025

Yeah, I would include spec-compliant proper handling of OTEL_RESOURCE_ATTRIBUTES into #43712, both in OtlpMetricsPropertiesConfigAdapter and in OpenTelemetryAutoConfiguration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants