feat: Add opentelemetry initial declarative configuration #487
Conversation
andborja
force-pushed
the
andborja/DeclarativeConfig
branch
from
05acef2 to
85505ee
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #487 +/- ##
=======================================
+ Coverage 54.0% 56.2% +2.1%
=======================================
Files 71 78 +7
Lines 11782 12581 +799
=======================================
+ Hits 6370 7078 +708
- Misses 5412 5503 +91 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| fn main() -> Result<(), Box<dyn std::error::Error>> { | ||
| // Create configuration registry and register console exporter | ||
| let mut registry = ConfigurationProvidersRegistry::new(); | ||
| let metrics_registry = registry.metrics_mut(); |
There was a problem hiding this comment.
Is metrics_mut avoidable by directly providing register_metric_config?
| // Load configuration from YAML file | ||
| let telemetry_provider = TelemetryProvider::new(); | ||
| let providers = telemetry_provider | ||
| .configure_from_yaml_file(®istry, "otel-config.yaml")?; |
There was a problem hiding this comment.
Can this be a static method, so we don't need to create instance of TelemetryProvider?
|
|
||
| pub fn register_periodic_exporter_factory( | ||
| &mut self, | ||
| name: String, |
There was a problem hiding this comment.
The exporter name would be static string - we can make it &'static str or &srr ?
| #[derive(serde::Deserialize, Debug)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub struct Periodic { | ||
| pub exporter: serde_yaml::Value, |
There was a problem hiding this comment.
Add TODO to support missing fields - interval_millis, timeout_millis, etc.
| } | ||
| } | ||
| serde_yaml::Value::Bool(b) => KeyValue::new(key.to_string(), *b), | ||
| _ => KeyValue::new(key.to_string(), format!("{:?}", value)), |
There was a problem hiding this comment.
add TODO to support Array - currently it would be stringified.
There was a problem hiding this comment.
LGTM for this first iteration. The console and OTLP schemas validated at run-time, and we can revisit the approach later if needed. Thanks for being super patient through all the review rounds - really appreciated.
Also, @utpilla had some solid architecture feedback, so let’s get his final approval before merging.
3 participants
Changes
Adds initial declarative configuration support.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes