[CONFIGURATION] Split programmatic and YAML cmake targets#4138
Open
pranitaurlam wants to merge 2 commits into
Open
[CONFIGURATION] Split programmatic and YAML cmake targets#4138pranitaurlam wants to merge 2 commits into
pranitaurlam wants to merge 2 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
Author
|
Hi @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.01% 82.02% +0.02%
==========================================
Files 385 385
Lines 16093 16093
==========================================
+ Hits 13197 13199 +2
+ Misses 2896 2894 -2
🚀 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 @marcalff @lalitb @dbarker @ThomsonTan