Use the new configuration bridge in spring boot starter#15714
Use the new configuration bridge in spring boot starter#15714trask merged 8 commits intoopen-telemetry:mainfrom
Conversation
github-actions
Bot
added
the
test native
trask
force-pushed
the
declarative-config/spring-step-one
branch
from
a676120 to
9137432
Compare
trask
changed the title
| ((EnumerablePropertySource<?>) propertySource).getPropertyNames()) { | ||
| if (propertyName.startsWith("otel.")) { | ||
| String property = environment.getProperty(propertyName); | ||
| Object property = propertySource.getProperty(propertyName); |
There was a problem hiding this comment.
This change preserves boolean values across the bridge
There was a problem hiding this comment.
yeah. the reason it worked before is that all of the spring boot starter config access was done via ConfigProperties which is string based anyways
and I've updated some places in this PR to now access config via Declarative Config API (which ignores properties if they're not the requested type instead of converting them)
There was a problem hiding this comment.
spring was using type coercion before - because I thought it was necessary.
I'll play around with this PR a little more to get a better picture.
There was a problem hiding this comment.
I figured it out: the wrong config provider was used: trask#101
There was a problem hiding this comment.
| return autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk(); | ||
| OpenTelemetrySdk openTelemetry = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk(); | ||
| ConfigProvider configProvider = ConfigPropertiesBackedConfigProvider.create(otelProperties); | ||
| return new SpringOpenTelemetrySdk(openTelemetry, configProvider); |
There was a problem hiding this comment.
A test failed when I just implemented OpenTelemetry instead of OpenTelemetrySdk
OpenTelemetryAutoConfigurationTest.shouldInitializeSdkWhenNotDisabled
I guess it could be considered a breaking change if anyone is relying on it really being an instance of OpenTelemetrySdk, so went with OpenTelemetrySdk instead.
There was a problem hiding this comment.
the test was just meant to assert that it's not the noop instance
2 participants
Follow-up PR(s) will migrate other usages of
InstrumentationConfigin Spring Boot Starter.Part of larger effort: