Make it easier for agent extensions to get the autoconfigured Resource

[breedx-splk]

Now that OpenTelemetryResourceAutoConfiguration.configureResource() is deprecated, it's difficult for agent extensions to get the resource.

The javadoc for the deprecated configureResource() says to use AutoConfiguredOpenTelemetrySdk.getResource() (an instance method). This suggest that a user who is migrating away from the deprecated call should use a new AutoConfiguredOpenTelemetrySdk, something like this:

    Resource resource = AutoConfiguredOpenTelemetrySdk.builder()
            .addPropertiesSupplier(config::getAllProperties)
            .build()
            .getResource();

But that doesn't seem right -- doing this in multiple places could lead to multiple instances of the sdk getting created, possibly with different configurations/customizations.

There should be a clearer and consistent way to get the autoconfigured Resource.

[anuraaga]

Hmm this is a good point. The intent was that the autoconfigured SDK would be available within an app for where Resource is needed. I wonder if the agent should be providing an API for accessing the SDK it built.

Would you be able to provide the code you have that references the Resource to understand the pattern fully? With the new knobs perhaps there is an alternative just want to check.

[anuraaga]

I might recall the code copied resource attributes into env vars?

In that case, using a resource customizer may be a reasonably clean solution. While ordering considerations may come to mind we have the broader issue anyways of adding the concept of ordering to all the SPIs.

[jack-berg]

Now that OpenTelemetryResourceAutoConfiguration.configureResource() is deprecated, it's difficult for agent extensions to get the resource.

I'm looking at the agent extension examples and don't understand why an extension would need access to the resource. Curious if you can provide a concrete example @breedx-splk.

[jack-berg]

I did find the AgentListener interface, and see a couple of usages of it here.

Going off @anuraaga's idea of the agent providing an API to access the SDK it built, we could extend agent listener to include access to AutoConfiguredOpenTelemetrySdk:

public interface AgentListener extends Ordered {

  default void beforeAgent(Config config) {}

  default void beforeAgent(AutoConfiguredOpenTelemetrySdk sdk) {}
  
  default void afterAgent(Config config) {}

  default void afterAgent(AutoConfiguredOpenTelemetrySdk sdk) {}

}

Not sure if it matters that AutoConfiguredOpenTelemetrySdk is part of an alpha module.

[jkwatson]

Now that OpenTelemetryResourceAutoConfiguration.configureResource() is deprecated, it's difficult for agent extensions to get the resource.

I'm looking at the agent extension examples and don't understand why an extension would need access to the resource. Curious if you can provide a concrete example @breedx-splk.

Splunk has several extensions that need access to the Resource. Jason can provide links to the concrete examples in our distro when he's back from the holidays, I think.

[anuraaga]

I found another code affected by the change

open-telemetry/opentelemetry-java-contrib#139

But in this case I feel the root cause is the inability for even a normal SDK user to be able to use the SDK's resource automatically when creating a sampler which could be remedied with the ability to register a factory instead.

[anuraaga]

@jack-berg Thanks for finding Splunk's references - indeed, for AgentListener, it seems to make sense to have it provide the SDK, assuming it doesn't want to provide it statically. configureResource was nice for coincidentally creating a Resource with the same configuration as auto-configuration, but now it's not the case with things like addResourceCustomizer. I think we're not really able to constrain our autoconfiguration to allow static access to the resource with such instance-level customization, but think that the customization has improved things a lot. It's being put to great use to pass in Resource information from the Maven context that is otherwise not possible with our SPI

https://github.com/open-telemetry/opentelemetry-java-contrib/pull/132/files#diff-8625ba4b8e68496923fa886962656dc739e75194535a5d9a9c7a81e555fff294R106

I guess in principle, anything that uses Resource needs to be executed after autoconfiguration as that is where we create the Resource. For cases like the AgentListener and Sampler, we'll need to find ways of accomplishing that.

One other usage in Splunk is this logs thing

https://github.com/signalfx/splunk-otel-java/blob/3a6d3adaf9239a4d201b8cb7a8b9fe4d7f919a4a/profiler/src/main/java/com/splunk/opentelemetry/profiler/LogsExporterBuilder.java

But not thinking too deeply about it yet given it's logs :P

[mateuszrzeszutek]

The problem with adding AutoConfiguredOpenTelemetrySdk to the AgentListener is that we're initializing OTel SDK in an AgentListener too - so the auto-configuration object won't be accessible in beforeAgent(), and making it accessible in the afterAgent() method would require a global variable anyway.

[jack-berg]

The problem with adding AutoConfiguredOpenTelemetrySdk to the AgentListener is that we're initializing OTel SDK in an AgentListener too

It seems convenient, not necessary, that we initialize the OTel SDK in an AgentListener. What about adjusting the startup sequence to be:

1. Initialize the OTel SDK
2. Call AgentListener#beforeAgent(Config, AutoConfiguredOpenTelemetrySdk)
3. Install agent
4. Call AgentListener#afterAgent(Config, AutoConfiguredOpenTelemetrySdk)

[mateuszrzeszutek]

That would work too, I think - both instrumentation and splunk repos don't really contain anything that desperately needs to be run before OTel initialization.
@pavolloffay you initially added the ComponentInstaller interface, do you remember if there was anything that had to be executed before OTel SDK setup?

[breedx-splk]

Thanks for powering thru some of this while I was afk. :)

It seems convenient, not necessary, that we initialize the OTel SDK in an AgentListener. What about adjusting the startup sequence to be:

1. Initialize the OTel SDK
2. Call AgentListener#beforeAgent(Config, AutoConfiguredOpenTelemetrySdk)
3. Install agent
4. Call AgentListener#afterAgent(Config, AutoConfiguredOpenTelemetrySdk)

This seems workable. It's a little goofy tho because AutoConfiguredOpenTelemetrySdk has a getConfig() method that makes the config parameter here seem redundant....

Unfortunately, however, it is complicated by the fact that AutoConfiguredOpenTelemetrySdk.getConfig() doesn't return a Config instance but instead a ConfigProperties instance. I took a quick scratch to see if ConfigProperties is a subset of Config, and it is, so in concept it's possible that Config could implement ConfigProperties...but I'm not sure that having the instrumentation api coupled to the sdk autoconf extension is the best thing (it ends up being a bit far reaching).

ConfigProperties is a pretty useful/generic interface, I wonder if folks would consider making it a bit more visible for purposes like this (config consolidation).

[anuraaga]

@breedx-splk Yeah the main motivation of the addPropertiesSupplier method was to try to use the same type for Config in the agent too, we haven't gotten to that but it'd probably make that idea a lot cleaner indeed.

[breedx-splk]

Yeah makes sense. I'd be ok with the two-arg version as a stop-gap measure.

[jsuereth]

I'd also like access to Resource for the OpenCensus migration tool, but from a pure-library standpoint

[anuraaga]

@jsuereth If you're using autoconfigure, then Resource is available on AutoConfiguredOpenTelemetrySdk.

[jack-berg]

Opened a ticket in opentelemetry-java-instrumentation to track what I believe is the consensus to extend AgentListener to access to AutoConfiguredOpenTelemetrySdk.

I'll try to get that implemented before the next release, so that we can merge the PR to remove deprecated autoconfigured methods.

[jack-berg]

@breedx-splk can we close this ticket now that AgentListener has been extended?

[breedx-splk]

@breedx-splk can we close this ticket now that AgentListener has been extended?

Absolutely. Thanks for the great work (and for this reminder!)

[trask]

just as a heads up for anyone looking at this issue, getResource() was removed prior to stability (in #5467)

as a workaround, you can use AutoConfigurationCustomizer.addResourceCustomizer and capture the Resource from inside of your resource customizer