feat: Add ConfigPropertiesBackedConfigProvider options for extensions and distros#15835
feat: Add ConfigPropertiesBackedConfigProvider options for extensions and distros#15835aviralgarg05 wants to merge 14 commits into
Conversation
…BackedConfigProvider options - Enhance ConfigPropertiesBackedConfigProvider with Builder for custom mappings - Refactor ConfigPropertiesBackedDeclarativeConfigProperties to support mappings - Deprecate DeclarativeConfigPropertiesBridge - Addresses issue open-telemetry#15811
- Fix ConfigPropertiesBackedConfigProvider compilation warnings - Fix Spotless formatting violations in all bridge files - Suppress deprecation warnings in backward compatibility tests
The spring-boot-autoconfigure and javaagent-tooling modules need to continue using the deprecated DeclarativeConfigPropertiesBridge API to maintain backward compatibility during the transition period. This commit adds @SuppressWarnings annotations with explanatory comments and compiler flags to suppress deprecation warnings.
|
cc @zeitlinger |
|
pr title is misleading - nothing is replaced it this PR - just new functions added |
# Conflicts: # declarative-config-bridge/src/test/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java
# Conflicts: # declarative-config-bridge/src/test/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java
There was a problem hiding this comment.
Pull request overview
This PR extends the declarative-config-bridge module so that extensions and distros can supply custom property mappings when bridging ConfigProperties into DeclarativeConfigProperties (declarative/YAML config). It adds a Builder to ConfigPropertiesBackedConfigProvider and threads a custom-mappings map through ConfigPropertiesBackedDeclarativeConfigProperties, merging user mappings on top of the built-in SPECIAL_MAPPINGS while rejecting attempts to override built-ins. This addresses issue #15811.
Changes:
- Added
ConfigPropertiesBackedConfigProvider.builder()withaddMapping(...)andbuild(ConfigProperties)for supplying custom declarative→config-property mappings. - Added an overload
createInstrumentationConfig(ConfigProperties, Map<String,String>)that merges custom mappings with built-ins and throwsIllegalArgumentExceptionon override of a built-in mapping; instance now carries the mergedmappingsmap throughgetStructurednavigation. - Added tests for builder mappings, default mappings, and rejection of built-in overrides.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ConfigPropertiesBackedConfigProvider.java |
Introduces Builder and routes create/build through the new mapping-aware factory. |
ConfigPropertiesBackedDeclarativeConfigProperties.java |
Adds mappings field, a mapping-aware factory with built-in override validation, and propagates mappings through navigation. |
ConfigPropertiesBackedConfigProviderTest.java |
New tests covering builder mappings, defaults, and override rejection. |
ConfigPropertiesBackedDeclarativeConfigPropertiesTest.java |
Adds a test verifying custom mappings coexist with special mappings. |
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
|
@robsunday @zeitlinger do you still need this feature? thanks! |
|
This would unblock follow-up work in In particular, the custom mapping support here is what we need for the contrib declarative component providers. Once this lands, it should let us finish migrating those callers and keep the old bridge on track for removal in 3.0. |
|
Closing this in favor of #19220, which carries the current version of the bridge work, including the configurable access path, the shared duration helper, and the added docs/tests based on the contrib follow-up. |
Enhance ConfigPropertiesBackedConfigProvider with Builder for custom mappings
Refactor ConfigPropertiesBackedDeclarativeConfigProperties to support mappings
Addresses issue create a replacement for DeclarativeConfigPropertiesBridge for extensions and distros #15811