Add file configuration types for Tracer, Meter, and Logger#3920
Add file configuration types for Tracer, Meter, and Logger#3920UMMAN2005 wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3920 +/- ##
=======================================
Coverage 90.06% 90.06%
=======================================
Files 226 226
Lines 7223 7223
=======================================
Hits 6505 6505
Misses 718 718 🚀 New features to boost your workflow:
|
UMMAN2005
force-pushed
the
feature/file-config-tracer-meter-logger
branch
from
61e4ad7 to
d71a591
Compare
Implement ExperimentalTracerConfig, ExperimentalTracerConfigurator, ExperimentalTracerMatcherAndConfig and their Meter/Logger equivalents as defined in the declarative configuration spec. - Add 9 new configuration model headers (Config, MatcherAndConfig, Configurator for each signal) - Wire configurator fields into TracerProviderConfiguration, MeterProviderConfiguration, and LoggerProviderConfiguration - Implement YAML parsing for the new configuration types in ConfigurationParser - Add ScopeConfigurator creation with wildcard name matching in SdkBuilder - Add unit tests for parsing the new configurator sections Resolves open-telemetry#3915
UMMAN2005
force-pushed
the
feature/file-config-tracer-meter-logger
branch
from
d71a591 to
a6ad72d
Compare
|
|
||
| std::unique_ptr<opentelemetry::sdk::instrumentationscope::ScopeConfigurator< | ||
| opentelemetry::sdk::trace::TracerConfig>> | ||
| SdkBuilder::CreateTracerConfigurator( |
There was a problem hiding this comment.
CreateTracerConfigurator, CreateMeterConfigurator, and CreateLoggerConfigurator are nearly identical. Not a blocker, as this follows the existing code pattern we have, but good to see if we can use the template helper to reduce this duplication.
| } | ||
|
|
||
| return model; | ||
| } |
There was a problem hiding this comment.
These 3 methods are mostly identical with their metrics and logs equivalent. Good to see if we can reduce the duplication with the templatised versions.
| namespace configuration | ||
| { | ||
|
|
||
| static bool WildcardMatch(const std::string &pattern, const std::string &text) |
There was a problem hiding this comment.
WildcardMatch is file-scoped static, making it impossible to directly unit-test or reuse. Please move it to a shared utility header (e.g., sdk/src/configuration/wildcard_match.h) and add dedicated unit tests covering the edge cases.
| namespace configuration | ||
| { | ||
|
|
||
| // YAML-SCHEMA: schema/meter_provider.yaml |
There was a problem hiding this comment.
the meter config here says// YAML-SCHEMA: schema/meter_provider.yaml while tracer/logger headers say .json. Should be consistent across all three signals.
marcalff
left a comment
marcalff
left a comment
There was a problem hiding this comment.
Nice work, which fits into the existing code.
See preliminary comments.
| ExperimentalLoggerConfigConfiguration ParseExperimentalLoggerConfigConfiguration( | ||
| const std::unique_ptr<DocumentNode> &node) const; |
There was a problem hiding this comment.
Some global naming comments, that apply everywhere:
The yaml schema type is ExperimentalFoo, but the corresponding C++ code should not say Experimental all the time.
Please change:
- the file names
- the classes names
- the method names
to remove the Experimental part.
| class ExperimentalLoggerConfigConfiguration | ||
| { | ||
| public: | ||
| bool disabled{false}; |
There was a problem hiding this comment.
The yaml attribute has been renamed to enabled in the spec, in a recent change:
Rename to enabled.
| namespace configuration | ||
| { | ||
|
|
||
| // YAML-SCHEMA: schema/meter_provider.yaml |
3 participants
Fixes #3915
Summary
Implements the file configuration types for Tracer, Meter, and Logger as described in #3915.
Changes
ExperimentalTracerConfig,ExperimentalTracerMatcherAndConfig,ExperimentalTracerConfigurator(and their Meter/Logger equivalents), following the project's one-class-per-header convention.FIXMEcomments inTracerProviderConfiguration,MeterProviderConfiguration, andLoggerProviderConfigurationwith the new configurator fields.tracer_configurator/development,meter_configurator/development, andlogger_configurator/developmentsections inConfigurationParser.CreateTracerConfigurator,CreateMeterConfigurator,CreateLoggerConfiguratorinSdkBuilderto constructScopeConfigurator<T>objects from parsed configuration, including wildcard name matching (?and*).yaml_trace_test.cc,yaml_metrics_test.cc, andyaml_logs_test.cc.Types implemented
Test plan