This issue was originally reported here: https://github.com/spring-projects/spring-boot/pull/34508#issuecomment-1557438909 by @chicobento:
Hi @jonatan-ivanov , we are facing some issues integrating this into our project. The scenario is the following:
- we have a custom class that implements
SpanExporterinterface and internally wraps our own mutableOtlpHttpSpanExporter- with this, we need to prevent the
OtlpAutoConfigurationfrom creating theOtlpHttpSpanExporterBean. However, the @ConditionalOnMissingBean is based only on OtlpHttpSpanExporter and OtlpGrpcSpanExporter beans, which is not our case as we implementSpanExporterinterface and cannnot descend from theOtlpHttpSpanExportersince its finalSome possible solutions to my problem that I tried: - Remove
OtlpAutoConfiguration: this behavior is added by a internal library, so I cannot easily remove theOtlpAutoConfigurationwithout impacting the applications that uses our lib - Replace theSpanProcessor: as theOpenTelemetryAutoConfiguration.otelSpanProcessormethod does not have a@ConditionalOnMissingBean. I cannot easily replace theSpanProcessorwith a custom one that would ignore theOtlpHttpSpanExporterBean provided byOtlpAutoConfiguration- Replace theOpenTelemetryAutoConfiguration.otelSdkTracerProvider: this would not work because theSpanProcessor.otelSpanProcessorwill always instantiate a BatchSpanProcessor and start its thread. I'd have to shutdown itDo you have any other suggestion on how can I overcome this ?
I was wondering if isn't the current
OtlpAutoConfiguration.otlpHttpSpanExporterfactory method too intrusive as it always provide aSpanExporterBean even when the application has already defined its ownSpanExporter? Can we change this to a simple@ConditionalOnMissingBean(SpanExporter.class)?
Comment From: jonatan-ivanov
Some of the behaviors you described here are intentional: we wanted to allow additional SpanExporter and SpanProcessor beans to be defined while keeping the default beans that we provide. E.g.: since you have OtlpHttpSpanExporter on the classpath we assumed you want to use it as an exporter but we also let you to create another, custom one. SpanProcessor is similar, see this issue: https://github.com/spring-projects/spring-boot/issues/35560 and this comment: https://github.com/spring-projects/spring-boot/pull/35558#issuecomment-1552838028
This is also why we cannot really do this: @ConditionalOnMissingBean(SpanExporter.class) that would mean that the second bean is not an addition but an override.
I do have a couple of ideas:
1. We can introduce an enabled flag for OtlpAutoConfiguration. This is a bit problematic since we already have a signal to enable/disable it, it is the presence of the OtlpHttpSpanExporter class but if we have a valid use-case, we can reconsider this and add the flag.
2. You can make your custom SpanProcessor @Primary and shut down the second one in a bean post processor.
3. It seems you want a custom OtlpHttpSpanExporter but it was intentionally made final. What is the custom behavior you want to add to OtlpHttpSpanExporter? Can this behavior added to OtlpHttpSpanExporter itself? Or can you open an issue to make OtlpHttpSpanExporter non-final?
Comment From: mhalbritter
One thing we could think about if we should introduce an abstraction which can veto that a SpanProcessor is added to the SdkTracerProvider. Something like:
@Bean
@ConditionalOnMissingBean
SdkTracerProvider otelSdkTracerProvider(Environment environment, ObjectProvider<SpanProcessor> spanProcessors,
Sampler sampler, ObjectProvider<SdkTracerProviderBuilderCustomizer> customizers,
SpanProcessorVetoer spanProcessorVetoer) {
String applicationName = environment.getProperty("spring.application.name", DEFAULT_APPLICATION_NAME);
SdkTracerProviderBuilder builder = SdkTracerProvider.builder()
.setSampler(sampler)
.setResource(Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, applicationName)));
spanProcessors.orderedStream().forEach((spanProcessor) -> {
if (!spanProcessorVetoer.veto(spanProcessor)) {
builder.addSpanProcessor(spanProcessor);
}
});
customizers.orderedStream().forEach((customizer) -> customizer.customize(builder));
return builder.build();
}
/**
* Can veto against adding a {@link SpanProcessor} to the {@link SdkTracerProvider}.
*/
interface SpanProcessorVetoer {
/**
* Returns {@code true} if veto against the given {@link SpanProcessor}, meaning
* that the span processor won't be added to the {@link SdkTracerProvider}.
* @param spanProcessor span processor which is under vote
* @return {@code true} if vetoed against the given span processor, {@code false}
* otherwise
*/
boolean veto(SpanProcessor spanProcessor);
}
That would solve the problem with multiple SpanProcessors on a more abstract level.
Comment From: chicobento
Hi @jonatan-ivanov ,
Thanks for the quick response.
Regarding option 3, the main reason why we override the OtlpHttpSpanExporter bean is that we need support for custom TLS certificates and certificate refresh scenario.
So, in our solution we created a MutableSpanProcessor that encapsulates a OtlpHttpSpanExporter and refreshes the inner instance whenever new certificates are issued. Open telemetry already helped by improving their support for custom certificates (such as item https://github.com/open-telemetry/opentelemetry-java/issues/5211), and I dont think extending the OtlpHttpSpanExporter would be of much help.
Now regarding option 2 (and @mhalbritter proposal variant), the drawback is that the BatchSpanProcessor threads are created/started and then shutdown - which is not nice.
So I think that we are left only with option 1 because of the BWC concerns with the additivity behavior of the SpanProcessors.
Comment From: jonatan-ivanov
I think to me being able to extend OtlpHttpSpanExporter would make sense, why do you think it would not be helpful? (I'm not eure OTel will do this though.)
I think option 2 is more like a workaround till this is not fixed because of the need to shutting down the processor.
@mhalbritter What do you think about introducing enable flags (maybe for zipkin too)?
Comment From: mhalbritter
I'm not a big fan of those enabled flags, but if we don't find a better solution, maybe that's the pill we have to swallow. Maybe someone else from the team has a better idea, let's talk about that in our next meeting.
Comment From: chicobento
I think to me being able to extend OtlpHttpSpanExporter would make sense, why do you think it would not be helpful? (I'm not eure OTel will do this though.)
I agree, it'd be useful, it is just hard to find a concrete use-case to justify this and dont have big expectations since OTel is very strict with their API surface.
Comment From: philwebb
We discussed this today and one idea we're considering is to remove the default OtlpProperties.endpoint value. This would mean that if users want HTTP export they'd need to add management.otlp.tracing.endpoint=http://localhost:4318/v1/traces to their properties.
This would give us a strong signal about when we'd auto-configure an OtlpHttpSpanExporter.
There are a couple of downsides to this approach. 1) It's a breaking change. 2) It makes configuration a little more verbose.
Comment From: philwebb
In the meantime you could try adding spring.autoconfigure.exclude=org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration to your application.properties or using @SpringBootApplication(exclude=OtlpAutoConfiguration.class) to disable our auto-configuration.
Comment From: chicobento
Hi @philwebb, fair enough.
You mean that you'll add a @ConditionalOnProperty(management.otlp.tracing.endpoint) ? Since it is a breaking change anyways, why not also adding @ConditionalOnMissingBean(SpanExporter.class) ?
In the meantime you could try adding spring.autoconfigure.exclude=org.springframework.boot.actuate.autoconfigure.tracing.otlp.OtlpAutoConfiguration to your application.properties or using @SpringBootApplication(exclude=OtlpAutoConfiguration.class) to disable our auto-configuration.
Yeap, I was trying to avoid that because we provide our custom SpanExporter bean in a shared library jar, so I'd like to avoid impacts on the applications, but at this point this seems to be one of the best options (or either shutting down the processor).
Thanks a lot everyone for the quick response.
Comment From: chicobento
BTW, for our specific deployment, the OTLP Collector is running remotely, so even if we could stick to the stock OtlpHttpSpanExporter, we'd have to configure the endpoint anyways.
Comment From: mhalbritter
The auto-configuration activates now only if management.otlp.tracing.endpoint is set. I've removed the default of this property.
Comment From: vpavic
If endpoint property is becoming mandatory for auto-configuring the exporter, IMO the auto-configuration shouldn't be limited to just supporting the HTTP exporter.
Comment From: mhalbritter
You would like to see auto-configuration for OtlpGrpcSpanExporter, too?
Comment From: vpavic
Well, since the endpoint URL has to be provided now, it can be used to figure out which type of exporter the user wants to configure and also support the gRPC option.
Comment From: mhalbritter
Created https://github.com/spring-projects/spring-boot/issues/35863 for it.
Comment From: mhalbritter
Additionally to the property change, there's now a SpanExporters bean. By default it collects all SpanExporter beans. If you don't want that, you can define a custom SpanExporters bean, which will then be used by Otel. See https://github.com/spring-projects/spring-boot/commit/929283f4dc2e21e5ddbd046d939aa2a0d7a9077e.