don't fail spring application startup if sdk is disabled #10602
Conversation
zeitlinger
force-pushed
the
spring-starter-disabled
branch
from
9919997
to
17f07ca
Compare
|
@laurit here's the fix 😄 |
There was a problem hiding this comment.
I woud disable all the autoconfigurations if otel.sdk.disabled=true: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports
|
I have created #10612 to do this. |
| @@ -19,6 +19,10 @@ private ExporterConfigEvaluator() {} | |||
| public static boolean isExporterEnabled( | |||
| Environment environment, String exportersKey, String wantExporter, boolean defaultValue) { | |||
|
|
|||
| if (environment.getProperty("otel.sdk.disabled", Boolean.class, false)) { | |||
There was a problem hiding this comment.
I would add a comment, something like this: the method is used to enable some Spring autoconfigurations
There was a problem hiding this comment.
putting this condition on each exporter's @Configuration could make it more visible and consistent (where we just throw the @SdkEnabled condition on everything)
There was a problem hiding this comment.
putting this condition on each exporter's
@Configurationcould make it more visible and consistent (where we just throw the@SdkEnabledcondition on everything)
Should it not be another property configuration allowing to disable the export?
By setting otel.sdk.disabled=true, I understand disable the OpenTelemtry SDK, and so disable the instrumentation features and the export.
There was a problem hiding this comment.
putting this condition on each exporter's
@Configurationcould make it more visible and consistent (where we just throw the@SdkEnabledcondition on everything)Should it not be another property configuration allowing to disable the export?
you still have otel.traces.exporter=none (similar for logs and metrics)
By setting
otel.sdk.disabled=true, I understand disable the OpenTelemtry SDK, and so disable the instrumentation features and the export.
yes, otel.sdk.disabled=true disables everything at once.
...lemetry/instrumentation/spring/autoconfigure/exporters/internal/ExporterConfigEvaluator.java
Outdated
Show resolved
Hide resolved
|
|
I am also fine with the current implementation if you think it is more explicit |
this wouldn't work, because the exporter beans are required to create the OpenTelemetry instance in OpenTelemetryAutoConfiguration |
| @Configuration | ||
| public class InstrumentationAnnotationsAutoConfiguration { | ||
|
|
||
| static final class Condition extends AllNestedConditions { |
There was a problem hiding this comment.
Just curious why you implemented it this way. Would having
@ConditionalOnProperty(name = "otel.instrumentation.annotations.enabled", matchIfMissing = true)
@Conditional(SdkEnabled.class)where SdkEnabled is shared by all auto configuration classes also work? If it would work this way I'd guess it needs a lot less code.
There was a problem hiding this comment.
Interesting idea! I hadn't considered that - I'll try.
zeitlinger
force-pushed
the
spring-starter-disabled
branch
from
f9742af
to
af3c2c6
Compare
zeitlinger
force-pushed
the
spring-starter-disabled
branch
from
b129069
to
b0bbbd1
Compare
Fixes #10600