From 5d7427960e86e12df6012c5064106954bf4504d7 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Mon, 12 Feb 2024 13:59:37 +0100 Subject: [PATCH] optimize resource detection by skipping providers that would add no new keys --- .../autoconfigure/spi/ResourceProvider.java | 14 ++++++++ .../autoconfigure/ResourceConfiguration.java | 24 ++++++++++++-- .../ConditionalResourceProviderTest.java | 28 ++++++++-------- .../provider/FirstResourceProvider.java | 19 ++++++++--- .../provider/SecondResourceProvider.java | 16 ++++++++-- .../opentelemetry/sdk/resources/Resource.java | 32 +++++++++++++++++-- 6 files changed, 105 insertions(+), 28 deletions(-) diff --git a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/ResourceProvider.java b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/ResourceProvider.java index 0957929e5a9..b9a27e5b381 100644 --- a/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/ResourceProvider.java +++ b/sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/ResourceProvider.java @@ -5,7 +5,10 @@ package io.opentelemetry.sdk.autoconfigure.spi; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.sdk.resources.Resource; +import java.util.Collections; +import java.util.Set; /** * A service provider interface (SPI) for providing a {@link Resource} that is merged into the @@ -14,4 +17,15 @@ public interface ResourceProvider extends Ordered { Resource createResource(ConfigProperties config); + + /** + * Returns the set of attribute keys that this provider supports. This is used to determine if a + * provider should be used to create a resource. + * + * @return the set of attribute keys that this provider supports - an empty set indicates that the + * provider should always be used + */ + default Set> supportedKeys() { + return Collections.emptySet(); + } } diff --git a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/ResourceConfiguration.java b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/ResourceConfiguration.java index 010d219acba..27d89f75298 100644 --- a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/ResourceConfiguration.java +++ b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/ResourceConfiguration.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.BiFunction; @@ -85,13 +86,18 @@ static Resource configureResource( ConfigProperties config, SpiHelper spiHelper, BiFunction resourceCustomizer) { - Resource result = Resource.getDefault(); + + // don't add the default service name yet + Resource result = + Resource.getDefault().toBuilder().removeIf(key -> key.equals(SERVICE_NAME)).build(); Set enabledProviders = new HashSet<>(config.getList("otel.java.enabled.resource.providers")); Set disabledProviders = new HashSet<>(config.getList("otel.java.disabled.resource.providers")); - for (ResourceProvider resourceProvider : spiHelper.loadOrdered(ResourceProvider.class)) { + List providers = spiHelper.loadOrdered(ResourceProvider.class); + Collections.reverse(providers); + for (ResourceProvider resourceProvider : providers) { if (!enabledProviders.isEmpty() && !enabledProviders.contains(resourceProvider.getClass().getName())) { continue; @@ -103,9 +109,21 @@ static Resource configureResource( && !((ConditionalResourceProvider) resourceProvider).shouldApply(config, result)) { continue; } - result = result.merge(resourceProvider.createResource(config)); + + Set> supportedKeys = resourceProvider.supportedKeys(); + if (!supportedKeys.isEmpty()) { + // empty means it always applies + Map, Object> attributes = result.getAttributes().asMap(); + if (supportedKeys.stream().allMatch(attributes::containsKey)) { + continue; + } + } + result = result.mergeReverse(resourceProvider.createResource(config)); } + // now add the default service name + result = result.mergeReverse(Resource.getDefault()); + result = filterAttributes(result, config); return resourceCustomizer.apply(result, config); diff --git a/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/ConditionalResourceProviderTest.java b/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/ConditionalResourceProviderTest.java index 6d3c90bb620..590c6f56b11 100644 --- a/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/ConditionalResourceProviderTest.java +++ b/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/ConditionalResourceProviderTest.java @@ -5,31 +5,29 @@ package io.opentelemetry.sdk.autoconfigure; -import static io.opentelemetry.api.common.AttributeKey.stringKey; import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.entry; -import org.junit.jupiter.api.Test; +import io.opentelemetry.sdk.autoconfigure.provider.FirstResourceProvider; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; class ConditionalResourceProviderTest { - @Test - void shouldConditionallyProvideResourceAttributes_skipBasedOnPreviousResource() { - AutoConfiguredOpenTelemetrySdk sdk = AutoConfiguredOpenTelemetrySdk.builder().build(); - - assertThat(sdk.getResource().getAttributes().asMap()) - .contains(entry(stringKey("service.name"), "test-service")); - } - - @Test - void shouldConditionallyProvideResourceAttributes_skipBasedOnConfig() { + @ParameterizedTest + @CsvSource({ + "false, test-service-2, 0", + "true, test-service, 1", + }) + void shouldConditionallyProvideResourceAttributes_skipBasedOnConfig(boolean skipSecond, String expectedServiceName, int expectedCallsToFirst) { AutoConfiguredOpenTelemetrySdk sdk = AutoConfiguredOpenTelemetrySdk.builder() - .addPropertiesSupplier(() -> singletonMap("skip-first-resource-provider", "true")) + .addPropertiesSupplier(() -> singletonMap("skip-second-resource-provider", String.valueOf(skipSecond))) .build(); assertThat(sdk.getResource().getAttributes().asMap()) - .contains(entry(stringKey("service.name"), "test-service-2")); + .containsEntry(FirstResourceProvider.KEY, expectedServiceName); + + assertThat(FirstResourceProvider.calls).isEqualTo(expectedCallsToFirst); } } diff --git a/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/FirstResourceProvider.java b/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/FirstResourceProvider.java index 97dfc1b1b7b..060bfd6f2b3 100644 --- a/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/FirstResourceProvider.java +++ b/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/FirstResourceProvider.java @@ -7,16 +7,25 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider; +import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.sdk.resources.Resource; +import java.util.Collections; +import java.util.Set; -public class FirstResourceProvider implements ConditionalResourceProvider { +public class FirstResourceProvider implements ResourceProvider { + + @SuppressWarnings("NonFinalStaticField") + public static int calls = 0; + + public static final AttributeKey KEY = stringKey("service.name"); @Override public Resource createResource(ConfigProperties config) { - return Resource.create(Attributes.of(stringKey("service.name"), "test-service")); + calls++; + return Resource.create(Attributes.of(KEY, "test-service")); } @Override @@ -25,7 +34,7 @@ public int order() { } @Override - public boolean shouldApply(ConfigProperties config, Resource existing) { - return !config.getBoolean("skip-first-resource-provider", false); + public Set> supportedKeys() { + return Collections.singleton(KEY); } } diff --git a/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/SecondResourceProvider.java b/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/SecondResourceProvider.java index d8978b8bfde..e2b09f5873d 100644 --- a/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/SecondResourceProvider.java +++ b/sdk-extensions/autoconfigure/src/testConditionalResourceProvider/java/io/opentelemetry/sdk/autoconfigure/provider/SecondResourceProvider.java @@ -7,16 +7,21 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider; import io.opentelemetry.sdk.resources.Resource; +import java.util.Collections; +import java.util.Set; public class SecondResourceProvider implements ConditionalResourceProvider { + public static final AttributeKey KEY = stringKey("service.name"); + @Override public Resource createResource(ConfigProperties config) { - return Resource.create(Attributes.of(stringKey("service.name"), "test-service-2")); + return Resource.create(Attributes.of(KEY, "test-service-2")); } @Override @@ -24,9 +29,14 @@ public int order() { return 200; } + @Override + public Set> supportedKeys() { + return Collections.singleton(KEY); + } + @Override public boolean shouldApply(ConfigProperties config, Resource existing) { - String serviceName = existing.getAttribute(stringKey("service.name")); - return serviceName == null || "unknown_service:java".equals(serviceName); + return !config.getBoolean("skip-second-resource-provider", false); } + } diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/resources/Resource.java b/sdk/common/src/main/java/io/opentelemetry/sdk/resources/Resource.java index c406ef87a5e..a949ad58caa 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/resources/Resource.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/resources/Resource.java @@ -63,6 +63,11 @@ public abstract class Resource { private static final Resource DEFAULT = MANDATORY.merge(TELEMETRY_SDK); + private enum MergePrecedence { + OTHER_TAKES_PRECEDENCE, + THIS_TAKES_PRECEDENCE + } + /** * Returns the default {@link Resource}. This resource contains the default attributes provided by * the SDK. @@ -146,13 +151,36 @@ public T getAttribute(AttributeKey key) { * @return the newly merged {@code Resource}. */ public Resource merge(@Nullable Resource other) { + return doMerge(other, MergePrecedence.OTHER_TAKES_PRECEDENCE); + } + + /** + * Returns a new, merged {@link Resource} by merging the current {@code Resource} with the {@code + * other} {@code Resource}. In case of a collision, the "this" {@code Resource} takes precedence. + * + * @param other the {@code Resource} that will be merged with {@code this}. + * @return the newly merged {@code Resource}. + */ + public Resource mergeReverse(@Nullable Resource other) { + return doMerge(other, MergePrecedence.THIS_TAKES_PRECEDENCE); + } + + private Resource doMerge(@Nullable Resource other, MergePrecedence precedence) { if (other == null || other == EMPTY) { return this; } AttributesBuilder attrBuilder = Attributes.builder(); - attrBuilder.putAll(this.getAttributes()); - attrBuilder.putAll(other.getAttributes()); + switch (precedence) { + case OTHER_TAKES_PRECEDENCE: + attrBuilder.putAll(this.getAttributes()); + attrBuilder.putAll(other.getAttributes()); + break; + case THIS_TAKES_PRECEDENCE: + attrBuilder.putAll(other.getAttributes()); + attrBuilder.putAll(this.getAttributes()); + break; + } if (other.getSchemaUrl() == null) { return create(attrBuilder.build(), getSchemaUrl());