Spring Boot Starter service-name is constant

[aschugunov]

Pattern-based resource attributes configuration
fix 3480

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Andrei Chugunov (822f53d)

[aschugunov]

@mateuszrzeszutek could you provide some feedback, please?

[trask]

thx @aschugunov! sorry for the delay in reviewing

[trask]

what do you think of loading the resources via SPI? https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk-extensions/resources/src/main/resources/META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider

or are there advantages to having them as separate named beans?

[aschugunov]

To be honest, I dont deeply understand how SPI works.
Currently, trace does not contain any inforamtion about os, container and other resources.
Main idea is combine all resources via supplier resource beans and then inject complex resource into tracerProvider.
maybe it is possbile to do with SPI, I will try to research. Thanks.

[aschugunov]

I tried to load resource objects via spring.factories and it seems spring does not load bean by interface or I did it in wrong manner.
If I understand correctly java loads objects via SPI when implementation is located in classpath, spring can customize context via spring.factories file. But this way does not work

io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider=\
io.opentelemetry.sdk.extension.resources.ContainerResourceProvider,\
io.opentelemetry.sdk.extension.resources.HostResourceProvider,\
io.opentelemetry.sdk.extension.resources.OsResourceProvider,\
io.opentelemetry.sdk.extension.resources.ProcessResourceProvider,\
io.opentelemetry.sdk.extension.resources.ProcessRuntimeResourceProvider

I added @ConditionalOnClass over appropriate bean, maybe it will be more suitable

[mateuszrzeszutek]

How about registering the ResourceProviders as beans? That's the same interface that OTel SDK autoconfigure uses - but instead of using SPI/ServiceLoader you'd just load all beans of that type.

[aschugunov]

Could you explain a bit more, please?
Did you mean to create resourceProvider beans in addition to resource beans?

[mateuszrzeszutek]

So, instead of all the Supplier<Resource> beans in this class, you could use ResourceProvider instead:

  @Bean
  @ConditionalOnClass(OsResource.class)
  public ResourceProvider osResourceProvider() {
    return new OsResourceProvider();
  }

  @Bean
  @ConditionalOnClass(ProcessResource.class)
  public ResourceProvider processResourceProvider() {
    return new ProcessResourceProvider();
  }

 // ...

Then, in the OpenTelemetryAutoConfiguration class you'd change the otelResource method so that it uses these beans:

    @Bean
    @ConditionalOnMissingBean
    public Resource otelResource(
        Environment env, ObjectProvider<List<ResourceProvider>> resourceProviders) {
      String applicationName = env.getProperty("spring.application.name");
      Resource resource = defaultResource(applicationName);
      ConfigProperties config = SpringConfigProperties(env);
      for (ResourceProvider provider : resourceProviders.getIfAvailable(Collections::emptyList)) {
        resource = resource.merge(provider.createResource(config));
      }
      return resource;
    }

ResourceProvider#createResource requires a ConfigProperties argument, but it would be quite simple to implement one that just simply delegates to Spring's Environment class.

This way you could reuse the SDK ResourceProvider interface in a way that's compatible with Spring beans -- instead of using SPI, the user of the instrumented application could just configure a ResourceProvider bean in their app and it'd get picked up automatically by this lib.

WDYT about that?

[aschugunov]

Yes it is a great idea to reuse existed interface for getting all available resources.
I see benefit of interface, it can be more clear for whole open telemetry project because interface provides consistent concept.
On the other hand, supplier follows the same idea - to provide resource. I mean there is no difference how spring will provide beans - via ResourceProvider interface or via Supplier<Resource>. Also interface enforce to use ConfigProperties that is not used in any current resourceProviders implementations.
OtelResourceProperties is such type of config. It is created by spring automatically.

If I understand correctly I should implement something like SpringResourceProvider and encapsulate there logic of determination of application name and attributes.

I will add these changes in second commit in this PR in a day or two. So you will be able to evaluate changes and choose more appropriate implementation.

[mateuszrzeszutek]

I will add these changes in second commit in this PR in a day or two. So you will be able to evaluate changes and choose more appropriate implementation.

👍 Thanks!

[trask]

@open-telemetry/java-instrumentation-approvers last chance to review!

[mateuszrzeszutek]

Nice 👍

[mateuszrzeszutek]

How about registering the ResourceProviders as beans? That's the same interface that OTel SDK autoconfigure uses - but instead of using SPI/ServiceLoader you'd just load all beans of that type.

[mateuszrzeszutek]

Thanks @aschugunov !