Skip to content
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

add ComponentLoader to support more auto configuration scenarios #6217

Merged
merged 21 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ if (!project.hasProperty("otel.release") && !project.name.startsWith("bom")) {
// Reproduce defaults from https://github.com/melix/japicmp-gradle-plugin/blob/09f52739ef1fccda6b4310cf3f4b19dc97377024/src/main/java/me/champeau/gradle/japicmp/report/ViolationsGenerator.java#L130
// with some changes.
val exclusions = mutableListOf<String>()
// Generics are not detected correctly
exclusions.add("CLASS_GENERIC_TEMPLATE_CHANGED")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't need this anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not for this PR - but it's still true that jApiCmp will trip over generics - I'm fine to take it out, of course.

// Allow new default methods on interfaces
exclusions.add("METHOD_NEW_DEFAULT")
// Allow adding default implementations for default methods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.OpenTelemetrySdkBuilder;
import io.opentelemetry.sdk.autoconfigure.internal.ComponentLoader;
import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
Expand Down Expand Up @@ -99,6 +100,9 @@ public final class AutoConfiguredOpenTelemetrySdkBuilder implements AutoConfigur
private final List<Function<ConfigProperties, Map<String, String>>> propertiesCustomizers =
new ArrayList<>();

private Function<ConfigProperties, ConfigProperties> configPropertiesCustomizer =
Function.identity();

private SpiHelper spiHelper =
SpiHelper.create(AutoConfiguredOpenTelemetrySdk.class.getClassLoader());

Expand Down Expand Up @@ -250,6 +254,21 @@ public AutoConfiguredOpenTelemetrySdkBuilder addPropertiesCustomizer(
return this;
}

/**
* Adds a {@link Function} to invoke the with the {@link ConfigProperties} to allow customization.
*
* <p>The argument to the function is the {@link ConfigProperties}, with the {@link
* #addPropertiesCustomizer(Function)} already applied.
*
* <p>The return value of the {@link Function} replace the {@link ConfigProperties} to be used.
*/
AutoConfiguredOpenTelemetrySdkBuilder setConfigPropertiesCustomizer(
Function<ConfigProperties, ConfigProperties> configPropertiesCustomizer) {
requireNonNull(configPropertiesCustomizer, "configPropertiesCustomizer");
this.configPropertiesCustomizer = configPropertiesCustomizer;
return this;
}

/**
* Adds a {@link BiFunction} to invoke the with the {@link SdkMeterProviderBuilder} to allow
* customization. The return value of the {@link BiFunction} will replace the passed-in argument.
Expand Down Expand Up @@ -368,6 +387,13 @@ public AutoConfiguredOpenTelemetrySdkBuilder setServiceClassLoader(
return this;
}

/** Sets the {@link ComponentLoader} to be used to load SPI implementations. */
AutoConfiguredOpenTelemetrySdkBuilder setComponentLoader(ComponentLoader componentLoader) {
requireNonNull(componentLoader, "componentLoader");
this.spiHelper = SpiHelper.create(componentLoader);
return this;
}

/**
* Returns a new {@link AutoConfiguredOpenTelemetrySdk} holding components auto-configured using
* the settings of this {@link AutoConfiguredOpenTelemetrySdkBuilder}.
Expand Down Expand Up @@ -568,7 +594,7 @@ private ConfigProperties computeConfigProperties() {
Map<String, String> overrides = customizer.apply(properties);
properties = properties.withOverrides(overrides);
}
return properties;
return configPropertiesCustomizer.apply(properties);
}

// Visible for testing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,13 @@ static Sampler configureSampler(String sampler, ConfigProperties config, SpiHelp
case "always_off":
return Sampler.alwaysOff();
case "traceidratio":
{
double ratio =
config.getDouble("otel.traces.sampler.arg", DEFAULT_TRACEIDRATIO_SAMPLE_RATIO);
return Sampler.traceIdRatioBased(ratio);
}
return ratioSampler(config);
case PARENTBASED_ALWAYS_ON:
return Sampler.parentBased(Sampler.alwaysOn());
case "parentbased_always_off":
return Sampler.parentBased(Sampler.alwaysOff());
case "parentbased_traceidratio":
{
double ratio =
config.getDouble("otel.traces.sampler.arg", DEFAULT_TRACEIDRATIO_SAMPLE_RATIO);
return Sampler.parentBased(Sampler.traceIdRatioBased(ratio));
}
return Sampler.parentBased(ratioSampler(config));
Comment on lines -186 to +182
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nice change, but seems unrelated to this PR? in the future splitting out unrelated changes can make life easier for reviewers

case "parentbased_jaeger_remote":
Sampler jaegerRemote = spiSamplersManager.getByName("jaeger_remote");
if (jaegerRemote == null) {
Expand All @@ -205,5 +197,10 @@ static Sampler configureSampler(String sampler, ConfigProperties config, SpiHelp
}
}

private static Sampler ratioSampler(ConfigProperties config) {
double ratio = config.getDouble("otel.traces.sampler.arg", DEFAULT_TRACEIDRATIO_SAMPLE_RATIO);
return Sampler.traceIdRatioBased(ratio);
}

private TracerProviderConfiguration() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
package io.opentelemetry.sdk.autoconfigure.internal;

import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.function.Function;

/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
Expand All @@ -30,4 +32,38 @@
"Error calling getConfig on AutoConfiguredOpenTelemetrySdk", e);
}
}

/** Sets the {@link ComponentLoader} to be used in the auto-configuration process. */
public static AutoConfiguredOpenTelemetrySdkBuilder setComponentLoader(
AutoConfiguredOpenTelemetrySdkBuilder builder, ComponentLoader componentLoader) {
try {
Method method =
AutoConfiguredOpenTelemetrySdkBuilder.class.getDeclaredMethod(
"setComponentLoader", ComponentLoader.class);
method.setAccessible(true);
method.invoke(builder, componentLoader);
return builder;
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
throw new IllegalStateException(

Check warning on line 47 in sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/internal/AutoConfigureUtil.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/internal/AutoConfigureUtil.java#L46-L47

Added lines #L46 - L47 were not covered by tests
"Error calling setComponentLoader on AutoConfiguredOpenTelemetrySdkBuilder", e);
}
}

/** Sets the {@link ConfigProperties} customizer to be used in the auto-configuration process. */
public static AutoConfiguredOpenTelemetrySdkBuilder setConfigPropertiesCustomizer(
AutoConfiguredOpenTelemetrySdkBuilder builder,
Function<ConfigProperties, ConfigProperties> customizer) {
try {
Method method =
AutoConfiguredOpenTelemetrySdkBuilder.class.getDeclaredMethod(
"setConfigPropertiesCustomizer", Function.class);
method.setAccessible(true);
method.invoke(builder, customizer);
return builder;
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
throw new IllegalStateException(

Check warning on line 64 in sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/internal/AutoConfigureUtil.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/internal/AutoConfigureUtil.java#L63-L64

Added lines #L63 - L64 were not covered by tests
"Error calling setConfigPropertiesCustomizer on AutoConfiguredOpenTelemetrySdkBuilder",
e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.autoconfigure.internal;

/**
* A loader for components that are discovered via SPI.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public interface ComponentLoader {
/**
* Load implementations of an SPI.
*
* @param spiClass the SPI class
* @param <T> the SPI type
* @return iterable of SPI implementations
*/
<T> Iterable<T> load(Class<T> spiClass);
zeitlinger marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,22 @@
*/
public final class SpiHelper {

private final ClassLoader classLoader;
private final SpiFinder spiFinder;
private final ComponentLoader componentLoader;
private final Set<AutoConfigureListener> listeners =
Collections.newSetFromMap(new IdentityHashMap<>());

// Visible for testing
SpiHelper(ClassLoader classLoader, SpiFinder spiFinder) {
this.classLoader = classLoader;
this.spiFinder = spiFinder;
private SpiHelper(ComponentLoader componentLoader) {
this.componentLoader = componentLoader;
}

/** Create a {@link SpiHelper} which loads SPIs using the {@code classLoader}. */
public static SpiHelper create(ClassLoader classLoader) {
return new SpiHelper(classLoader, ServiceLoader::load);
return new SpiHelper(new ServiceLoaderComponentLoader(classLoader));
}

/** Create a {@link SpiHelper} which loads SPIs using the {@code componentLoader}. */
public static SpiHelper create(ComponentLoader componentLoader) {
return new SpiHelper(componentLoader);
}

/**
Expand Down Expand Up @@ -96,7 +98,7 @@ public <T extends Ordered> List<T> loadOrdered(Class<T> spiClass) {
*/
public <T> List<T> load(Class<T> spiClass) {
List<T> result = new ArrayList<>();
for (T service : spiFinder.load(spiClass, classLoader)) {
for (T service : componentLoader.load(spiClass)) {
maybeAddListener(service);
result.add(service);
}
Expand All @@ -114,8 +116,16 @@ public Set<AutoConfigureListener> getListeners() {
return Collections.unmodifiableSet(listeners);
}

// Visible for testing
interface SpiFinder {
<T> Iterable<T> load(Class<T> spiClass, ClassLoader classLoader);
private static class ServiceLoaderComponentLoader implements ComponentLoader {
private final ClassLoader classLoader;

private ServiceLoaderComponentLoader(ClassLoader classLoader) {
this.classLoader = classLoader;
}

@Override
public <T> Iterable<T> load(Class<T> spiClass) {
return ServiceLoader.load(spiClass, classLoader);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import io.github.netmikey.logunit.api.LogCapturer;
Expand All @@ -36,10 +37,14 @@
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
import io.opentelemetry.sdk.autoconfigure.internal.ComponentLoader;
import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.autoconfigure.spi.internal.AutoConfigureListener;
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.logs.LogRecordProcessor;
import io.opentelemetry.sdk.logs.SdkLoggerProvider;
Expand Down Expand Up @@ -315,6 +320,32 @@ void builder_addSpanProcessorCustomizer() {
verifyNoInteractions(spanExporter1);
}

@Test
void builder_addAutoConfigurationCustomizerProviderUsingComponentLoader() {
AutoConfigurationCustomizerProvider customizerProvider =
mock(AutoConfigurationCustomizerProvider.class);

SpiHelper spiHelper =
SpiHelper.create(AutoConfiguredOpenTelemetrySdkBuilder.class.getClassLoader());

AutoConfigureUtil.setComponentLoader(
builder,
new ComponentLoader() {
@SuppressWarnings("unchecked")
@Override
public <T> Iterable<T> load(Class<T> spiClass) {
if (spiClass.equals(AutoConfigurationCustomizerProvider.class)) {
return Collections.singletonList((T) customizerProvider);
}
return spiHelper.load(spiClass);
}
})
.build();

verify(customizerProvider).customize(any());
verifyNoMoreInteractions(customizerProvider);
}

@Test
void builder_addPropertiesSupplier() {
AutoConfiguredOpenTelemetrySdk autoConfigured =
Expand Down Expand Up @@ -356,6 +387,23 @@ void builder_addPropertiesCustomizer() {
assertThat(autoConfigured.getConfig().getString("some.key")).isEqualTo("override-2");
}

@Test
void builder_setConfigPropertiesCustomizer() {
AutoConfiguredOpenTelemetrySdk autoConfigured =
AutoConfigureUtil.setConfigPropertiesCustomizer(
builder.addPropertiesCustomizer(config -> singletonMap("some-key", "defaultValue")),
config -> {
assertThat(config.getString("some-key")).isEqualTo("defaultValue");

Map<String, String> map = new HashMap<>(singletonMap("some-key", "override"));
map.putAll(disableExportPropertySupplier().get());
return DefaultConfigProperties.createFromMap(map);
})
.build();

assertThat(autoConfigured.getConfig().getString("some.key")).isEqualTo("override");
}

@Test
void builder_addMeterProviderCustomizer() {
Mockito.lenient().when(metricReader.shutdown()).thenReturn(CompletableResultCode.ofSuccess());
Expand Down
Loading
Loading