[CONFIGURATION] Split programmatic and YAML cmake targets#4138
[CONFIGURATION] Split programmatic and YAML cmake targets#4138pranitaurlam wants to merge 8 commits into
Conversation
Split opentelemetry_configuration into two targets: - opentelemetry_configuration: programmatic config, no ryml dependency, built unconditionally as part of the SDK - opentelemetry_configuration_yaml: YAML config with ryml dependency, gated by WITH_CONFIGURATION Add default constructor to Configuration for programmatic use.
c0f6400 to
eab1f2b
Compare
|
Hi @lalitb @marcalff @lalitb @dbarker @ThomsonTan I've opened PR #4138 to resolve issue #4134 — splitting the opentelemetry_configuration CMake target into two separate targets so users can use programmatic configuration without pulling in the ryml dependency. The EasyCLA check has passed. Could one of you please approve the workflow runs so CI can execute? Any feedback on the implementation is also very welcome. Thank you! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4138 +/- ##
=======================================
Coverage 82.82% 82.82%
=======================================
Files 406 406
Lines 16913 16913
=======================================
Hits 14007 14007
Misses 2906 2906
🚀 New features to boost your workflow:
|
- Restore configuration as standalone install component (not part of sdk) - Add configuration_yaml to EXPECTED_COMPONENTS in do_ci.sh - Fix cmake-format in CMakeLists.txt, sdk/src/configuration, sdk/test/configuration
- Add test_configuration_yaml.cc and component_tests/configuration_yaml/ - registry.cc: add missing text_map_propagator.h include (IWYU) - sdk_builder.cc: replace filter_type.h with exemplar_filter.h (IWYU)
- cmake-format: put PRIVATE on same line as target in target_link_libraries - clang-format: move exemplar_filter.h include to alphabetical position
sdk_builder.cc directly uses ExemplarFilterType from sdk/metrics/exemplar/filter_type.h. The prior commit moved exemplar_filter.h into alphabetical order but did not restore this separate metrics SDK header that was originally guarded by ENABLE_METRICS_EXEMPLAR_PREVIEW. Add it back unconditionally.
c092938 to
efe4b12
Compare
There was a problem hiding this comment.
Thanks for the fix.
EDIT 2026-06-16: no longer needed due to renaming to opentelemetry_configuration_core, opentelemetry_configuration.
Please add an "Important note" in the CHANGELOG, to explain that user makefiles are affected, and needs to be adjusted, to use the new opentelemetry_configuration_yaml library.
Will merge after that.
dbarker
left a comment
There was a problem hiding this comment.
Thanks for the PR! Please see initial feedback and a question below.
|
|
||
| if(NOT TARGET Threads::Threads) | ||
| message(FATAL_ERROR "Threads::Threads target not found") | ||
| endif() |
There was a problem hiding this comment.
Please add a check that the ryml target has been imported.
| { | ||
| public: | ||
| Configuration() = default; | ||
| Configuration(std::unique_ptr<Document> doc) : doc_(std::move(doc)) {} |
There was a problem hiding this comment.
is coupling the Configuration class with the parsed document still required?
|
@pranitaurlam Thanks for the contribution. Marc and I discussed the PR in the SIG meeting today and request the following changes to minimize impact on current users of the Instead of placing the yaml components in the configuration_yaml target we suggest:
This way current users of the yaml config will continue to link to the |
|
So, in short:
This way, applications using opentelemetry-cpp::configuration in their makefiles will be unchanged. |
Fixes #4134
Summary
opentelemetry_configurationinto two CMake targets:opentelemetry_configuration: programmatic configuration only (sdk_builder.cc,configured_sdk.cc,registry.cc,configuration_parser.cc,document_node.cc, composable sampler builders). Norymldependency. Built unconditionally as part of the SDK.opentelemetry_configuration_yaml: YAML configuration components (yaml_configuration_parser.cc,ryml_document.cc,ryml_document_node.cc) with theryml::rymldependency. Gated byWITH_CONFIGURATION.WITH_CONFIGURATIONto gate only the YAML/ryml target.Configurationso it can be instantiated programmatically without aDocument.configuration_yamlreplacesconfigurationfor the YAML component; theconfigurationcomponent is now the programmatic-only piece included in the SDK.Test plan
WITH_CONFIGURATIONand verifyopentelemetry_configurationlinks successfully without rymlWITH_CONFIGURATION=ONand verify both targets build and tests passopentelemetry_configurationalonecc @lalitb @marcalff @lalitb @dbarker @ThomsonTan