Skip to content

Resource in AutoConfiguredOpenTelemetrySdk object is configured from the 'resource' YAML node #7418

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.

Sign up for GitHub

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
wants to merge 11 commits into from
Closed

Resource in AutoConfiguredOpenTelemetrySdk object is configured from the 'resource' YAML node #7418

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
db4e2e0
Resource set in the AutoConfiguredOpenTelemetrySdk object is now conf…
robsunday Jun 11, 2025
d45b6a8
Reverting some unnecessary changes.
robsunday Jun 11, 2025
99826d3
UT fix
robsunday Jun 11, 2025
a23d196
spotless
robsunday Jun 11, 2025
6defc6d
UT added
robsunday Jun 13, 2025
0e684c0
Merge branch 'main' into fbc-create-resource-from-model
robsunday Jun 20, 2025
14f0b08
UT added
robsunday Jun 20, 2025
f451e45
Merge branch 'open-telemetry:main' into fbc-create-resource-from-model
robsunday Jul 8, 2025
593ffb0
Merge branch 'main' into fbc-create-resource-from-model
robsunday Jul 16, 2025
a0be1ff
Fix after merge
robsunday Jul 17, 2025
08fe64e
Merge branch 'main' into fbc-create-resource-from-model
robsunday Jul 30, 2025
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
34 changes: 30 additions & 4 deletions ...nsions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/IncubatingUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,16 @@ static AutoConfiguredOpenTelemetrySdk configureFromFile(
declarativeConfiguration.getMethod(
"create", openTelemetryConfiguration, ComponentLoader.class);
OpenTelemetrySdk sdk = (OpenTelemetrySdk) create.invoke(null, model, componentLoader);

Class<?> sdkConfigProvider =
Class.forName("io.opentelemetry.sdk.extension.incubator.fileconfig.SdkConfigProvider");
Method createFileConfigProvider =
sdkConfigProvider.getMethod("create", openTelemetryConfiguration);
ConfigProvider configProvider = (ConfigProvider) createFileConfigProvider.invoke(null, model);
// Note: can't access file configuration resource without reflection so setting a dummy
// resource
return AutoConfiguredOpenTelemetrySdk.create(
sdk, Resource.getDefault(), null, configProvider);

Resource configuredResource = createResourceFromModel(model, componentLoader);
Copy link
Member

Choose a reason for hiding this comment

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

This means that we are calling ResourceFactory twice, but the alternative is for DeclarativeConfiguration#create to return a tuple of OpenTelemetrySdk and Resource such that the resource is accessible, which isn't great either.

I actually want to get rid of the incubating API to access the autoconfigured resource. Its come up in a variety of conversations (including this comment), but the TL;DR is:

The problem with accessing the autoconfigured resource is that in all the cases I've seen so far, its always been the wrong thing to do.

Copy link
Contributor Author

@robsunday robsunday Jun 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Returning tuple from DeclarativeConfiguration#create would actually require updates of OpenTelemetryConfigurationFactory#create. This would also require some additional classes and maybe reane of OpenTelemetryConfigurationFactory.
I think instantiating resource twice is good enough solution.

Regarding the change you mentioned:

I actually want to get rid of the incubating API to access the autoconfigured resource

Do you plan to get rid of the resource property and getResource metod from AutoConfiguredOpenTelemetrySdk class?
If yes, then it may cause lots of issues in some vendor's code. We use it in our code in few places.

Copy link
Member

Choose a reason for hiding this comment

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

If yes, then it may cause lots of issues in some vendor's code. We use it in our code in few places.

I know, but why? I'm still searching for a compelling reason vendors do this. Every instance I've seen so far has been dubious.

Copy link
Member

Choose a reason for hiding this comment

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

Hey @robsunday you may be interested in this proposal and conversation: https://github.com/open-telemetry/opentelemetry-specification/pull/4565/files#r2193326632

Copy link
Member

Choose a reason for hiding this comment

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

The agent is using the resource as well: https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-java-instrumentation%20ConfiguredResourceAttributesHolder&type=code


return AutoConfiguredOpenTelemetrySdk.create(sdk, configuredResource, null, configProvider);
} catch (FileNotFoundException e) {
throw new ConfigurationException("Configuration file not found", e);
} catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException e) {
Expand All @@ -76,6 +77,31 @@ static AutoConfiguredOpenTelemetrySdk configureFromFile(
}
}

private static Resource createResourceFromModel(
Object openTelemetryConfigurationModel, ComponentLoader componentLoader)
throws NoSuchMethodException,
InvocationTargetException,
IllegalAccessException,
ClassNotFoundException {
Class<?> configurationModelClass =
Class.forName(
"io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel");
Method getResource = configurationModelClass.getMethod("getResource");
Object resourceModel = getResource.invoke(openTelemetryConfigurationModel);

Class<?> resourceModelClass =
Class.forName(
"io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ResourceModel");
Class<?> declarativeConfigurationClass =
Class.forName(
"io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfiguration");
Method createResource =
declarativeConfigurationClass.getMethod(
"createResource", resourceModelClass, ComponentLoader.class);

return (Resource) createResource.invoke(null, resourceModel, componentLoader);
}

private static ConfigurationException toConfigurationException(
DeclarativeConfigException exception) {
String message = Objects.requireNonNull(exception.getMessage());
Expand Down
10 changes: 4 additions & 6 deletions .../testIncubating/java/io/opentelemetry/sdk/autoconfigure/DeclarativeConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,16 @@ void configFile_fileNotFound() {

@Test
void configFile_Valid() {
Resource expectedResource =
Resource.getDefault().toBuilder().put("service.name", "test").build();
ConfigProperties config =
DefaultConfigProperties.createFromMap(
Collections.singletonMap("otel.experimental.config.file", configFilePath.toString()));
OpenTelemetrySdk expectedSdk =
OpenTelemetrySdk.builder()
.setTracerProvider(
SdkTracerProvider.builder()
.setResource(
Resource.getDefault().toBuilder().put("service.name", "test").build())
.setResource(expectedResource)
.addSpanProcessor(SimpleSpanProcessor.create(LoggingSpanExporter.create()))
.build())
.build();
Expand All @@ -136,10 +137,7 @@ void configFile_Valid() {

Assertions.assertThat(autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk().toString())
.isEqualTo(expectedSdk.toString());
// AutoConfiguredOpenTelemetrySdk#getResource() is set to a dummy value when configuring from
// file
Assertions.assertThat(autoConfiguredOpenTelemetrySdk.getResource())
.isEqualTo(Resource.getDefault());
Assertions.assertThat(autoConfiguredOpenTelemetrySdk.getResource()).isEqualTo(expectedResource);
verify(builder, times(1)).shutdownHook(autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk());
Assertions.assertThat(Runtime.getRuntime().removeShutdownHook(thread)).isTrue();
logCapturer.assertContains("Autoconfiguring from configuration file: " + configFilePath);
Expand Down
12 changes: 12 additions & 0 deletions ...in/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper;
import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider;
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel;
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ResourceModel;
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.SamplerModel;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import java.io.Closeable;
import java.io.IOException;
Expand Down Expand Up @@ -205,6 +207,16 @@
samplerModel);
}

/** Create a {@link Resource} from the {@code resourceModel} representing the resource config. */
public static Resource createResource(
ResourceModel resourceModel, ComponentLoader componentLoader) {
if (resourceModel == null) {
return Resource.getDefault();

Check warning on line 214 in sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java#L214 

Added line #L214 was not covered by tests
}
return createAndMaybeCleanup(
ResourceFactory.getInstance(), SpiHelper.create(componentLoader), resourceModel);
}

private static YamlDeclarativeConfigProperties requireYamlDeclarativeConfigProperties(
DeclarativeConfigProperties declarativeConfigProperties) {
if (!(declarativeConfigProperties instanceof YamlDeclarativeConfigProperties)) {
Expand Down
Loading