Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 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 @@ -368,6 +369,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
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,6 +6,7 @@
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;
Expand All @@ -30,4 +31,34 @@ public static ConfigProperties getConfig(
"Error calling getConfig on AutoConfiguredOpenTelemetrySdk", e);
}
}

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

/** Sets the {@link ConfigProperties} to be used in the auto-configuration process. */
public static void setConfigProperties(
AutoConfiguredOpenTelemetrySdkBuilder builder, ConfigProperties config) {
try {
Method method =
AutoConfiguredOpenTelemetrySdkBuilder.class.getDeclaredMethod(
"setConfig", ConfigProperties.class);
method.setAccessible(true);
method.invoke(builder, config);
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
throw new IllegalStateException(
"Error calling setConfig on AutoConfiguredOpenTelemetrySdkBuilder", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.autoconfigure.internal;

import io.opentelemetry.sdk.autoconfigure.spi.Ordered;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

/**
* 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);

/**
* Load implementations of an ordered SPI (i.e. implements {@link Ordered}).
*
* @param spiClass the SPI class
* @param <T> the SPI type
* @return list of SPI implementations, in order
*/
default <T extends Ordered> List<T> loadOrdered(Class<T> spiClass) {
return StreamSupport.stream(load(spiClass).spliterator(), false)
.sorted(Comparator.comparing(Ordered::order))
.collect(Collectors.toList());
}
Copy link
Member

Choose a reason for hiding this comment

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

do we need this method? (can inline as private method in SpiHelper?)

Copy link
Member Author

Choose a reason for hiding this comment

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

the method is meant for overriding.
In spring, the spring beans are given a higher priority - https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/10453/files#diff-8c1885af89564bfbd461edbbf02fe19e23e40a2e54f6e264d41d786b760e5d26R40-R53.

I'm still unsure if that's what users actually expect though.

It affects

  • AutoConfigurationCustomizerProvider
  • ResourceProvider

Copy link
Member Author

Choose a reason for hiding this comment

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

can't find a really compelling reason - I'll remove it.

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.opentelemetry.sdk.autoconfigure.spi.internal.AutoConfigureListener;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.List;
Expand All @@ -27,20 +26,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 @@ -82,9 +83,7 @@ public <T, S> NamedSpiManager<T> loadConfigurable(
* @return list of SPI implementations, in order
*/
public <T extends Ordered> List<T> loadOrdered(Class<T> spiClass) {
List<T> result = load(spiClass);
result.sort(Comparator.comparing(Ordered::order));
return result;
return init(componentLoader.loadOrdered(spiClass));
}

/**
Expand All @@ -95,27 +94,46 @@ public <T extends Ordered> List<T> loadOrdered(Class<T> spiClass) {
* @return list of SPI implementations
*/
public <T> List<T> load(Class<T> spiClass) {
return init(componentLoader.load(spiClass));
}

/**
* Load implementations of an SPI.
*
* @param components the SPI implementations
* @param <T> the SPI type
* @return list of SPI implementations
*/
private <T> List<T> init(Iterable<T> components) {
List<T> result = new ArrayList<>();
for (T service : spiFinder.load(spiClass, classLoader)) {
maybeAddListener(service);
result.add(service);
for (T service : components) {
result.add(maybeAddListener(service));
}
return result;
}

private void maybeAddListener(Object object) {
private <T> T maybeAddListener(T object) {
if (object instanceof AutoConfigureListener) {
listeners.add((AutoConfigureListener) object);
}
return object;
}

/** Return the set of SPIs loaded which implement {@link AutoConfigureListener}. */
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,7 +37,9 @@
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
import io.opentelemetry.sdk.OpenTelemetrySdk;
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;
Expand Down Expand Up @@ -315,6 +318,33 @@ void builder_addSpanProcessorCustomizer() {
verifyNoInteractions(spanExporter1);
}

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

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

builder
.setComponentLoader(
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand All @@ -28,11 +29,11 @@ public class SpiHelperTest {

@Test
public void canRetrieveByName() {
SpiHelper.SpiFinder mockFinder = mock(SpiHelper.SpiFinder.class);
when(mockFinder.load(any(), any()))
ComponentLoader mockLoader = spy(ComponentLoader.class);
when(mockLoader.load(any()))
.thenReturn(Collections.singletonList(new SpiExampleProviderImplementation()));

SpiHelper spiHelper = new SpiHelper(SpiHelperTest.class.getClassLoader(), mockFinder);
SpiHelper spiHelper = SpiHelper.create(mockLoader);

NamedSpiManager<SpiExample> spiProvider =
spiHelper.loadConfigurable(
Expand All @@ -49,10 +50,10 @@ public void canRetrieveByName() {
public void instantiatesImplementationsLazily() {
SpiExampleProvider mockProvider = mock(SpiExampleProvider.class);
when(mockProvider.getName()).thenReturn("lazy-init-example");
SpiHelper.SpiFinder mockFinder = mock(SpiHelper.SpiFinder.class);
when(mockFinder.load(any(), any())).thenReturn(Collections.singletonList(mockProvider));
ComponentLoader mockLoader = spy(ComponentLoader.class);
when(mockLoader.load(any())).thenReturn(Collections.singletonList(mockProvider));

SpiHelper spiHelper = new SpiHelper(SpiHelperTest.class.getClassLoader(), mockFinder);
SpiHelper spiHelper = SpiHelper.create(mockLoader);

NamedSpiManager<SpiExample> spiProvider =
spiHelper.loadConfigurable(
Expand All @@ -68,11 +69,11 @@ public void instantiatesImplementationsLazily() {

@Test
public void onlyInstantiatesOnce() {
SpiHelper.SpiFinder mockFinder = mock(SpiHelper.SpiFinder.class);
when(mockFinder.load(any(), any()))
ComponentLoader mockLoader = mock(ComponentLoader.class);
when(mockLoader.load(any()))
.thenReturn(Collections.singletonList(new SpiExampleProviderImplementation()));

SpiHelper spiHelper = new SpiHelper(SpiHelperTest.class.getClassLoader(), mockFinder);
SpiHelper spiHelper = SpiHelper.create(mockLoader);

NamedSpiManager<SpiExample> spiProvider =
spiHelper.loadConfigurable(
Expand All @@ -93,10 +94,10 @@ public void failureToInitializeThrows() {
when(mockProvider.getName()).thenReturn("init-failure-example");
when(mockProvider.createSpiExample(any())).thenThrow(new RuntimeException());

SpiHelper.SpiFinder mockFinder = mock(SpiHelper.SpiFinder.class);
when(mockFinder.load(any(), any())).thenReturn(Collections.singletonList(mockProvider));
ComponentLoader mockLoader = spy(ComponentLoader.class);
when(mockLoader.load(any())).thenReturn(Collections.singletonList(mockProvider));

SpiHelper spiHelper = new SpiHelper(SpiHelperTest.class.getClassLoader(), mockFinder);
SpiHelper spiHelper = SpiHelper.create(mockLoader);

NamedSpiManager<SpiExample> spiProvider =
spiHelper.loadConfigurable(
Expand All @@ -120,11 +121,10 @@ void loadsOrderedSpi() {
when(spi2.order()).thenReturn(0);
when(spi3.order()).thenReturn(1);

SpiHelper.SpiFinder mockFinder = mock(SpiHelper.SpiFinder.class);
when(mockFinder.load(ResourceProvider.class, SpiHelper.class.getClassLoader()))
.thenReturn(asList(spi1, spi2, spi3));
ComponentLoader mockLoader = spy(ComponentLoader.class);
when(mockLoader.load(ResourceProvider.class)).thenReturn(asList(spi1, spi2, spi3));

SpiHelper spiHelper = new SpiHelper(SpiHelperTest.class.getClassLoader(), mockFinder);
SpiHelper spiHelper = SpiHelper.create(mockLoader);

List<ResourceProvider> loadedSpi = spiHelper.loadOrdered(ResourceProvider.class);

Expand Down