Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2467 +/- ##
==========================================
+ Coverage 50.78% 51.23% +0.45%
==========================================
Files 48 48
Lines 4005 4042 +37
==========================================
+ Hits 2034 2071 +37
Misses 1811 1811
Partials 160 160
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| // Configure exporters based on selector's export modes | ||
| if selector != nil { |
There was a problem hiding this comment.
This is great, but in OBI we have two export sections, one is the global one and then there are the per selector modes, which override the global one. I think we need to first consult the global ones and then if the selector has export modes, we override those to what we pass to the pod.
We start with this:
tracesEnabled := pm.cfg.Traces.Enabled()
metricsEnabled := pm.cfg.Metrics.Enabled()
For metrics technically we have the prometheus scrape too, but this wouldn't be possible with the SDKs since they assume otel push, so just checking cfg.Metrics.Enabled() is good to start.
Then the code you already have if selector != nil ...
We can start by creating a services.ExportModes and then call AllowTraces() and AllowMetrics() on it to have a common struct that we pass to pm.configureExporters() and then if the selector has export modes, just override the global before passing down to the function.
There was a problem hiding this comment.
The whole Prometheus export made me think we need another option to say Injector exports metrics globally on that config section. The reason is that currently Beyla disallows prometheus and OTel metrics export at the same time, it's one or the other, but the injector can only export with OTel, so we need metrics exporter config for that section, which will tell us we need metrics otel export from the SDKs and not rely on the Beyla metrics config.
Changes exporter configuration to: 1. Start with global config (pm.cfg.Traces.Enabled() and pm.cfg.OTELMetrics.EndpointEnabled()) 2. Create a services.ExportModes struct and enable based on global config 3. Override with selector's export modes if selector has them defined 4. Pass final ExportModes to pm.configureExporters() This ensures the webhook respects both global export configuration and per-selector overrides, with selector config taking precedence.
Adds a new `export` section to `injector` configuration that allows
controlling which signals (traces, metrics, logs) are exported from
injected SDKs, independent of Beyla's global export configuration.
This addresses the second review comment which noted that the injector
needs separate metrics export configuration since:
- Beyla can use Prometheus scraping for metrics
- Injected SDKs can only export via OTLP
- Both need to work simultaneously
Configuration:
- injector.export.traces: bool (default: true)
- injector.export.metrics: bool (default: true)
- injector.export.logs: bool (default: false)
These settings are checked first, then can be overridden per-service
by selector export modes.
Example YAML:
```yaml
injector:
export:
traces: true
metrics: true
logs: false
```
Adds support for controlling logs export from injected SDKs via the OTEL_LOGS_EXPORTER environment variable. Changes: - Added envOtelLogsExporterName constant - Updated configureExporters() to accept logsEnabled parameter - Logs exporter set to "otlp" when enabled, "none" when disabled - Logs default to disabled (per SDKExport.LogsEnabled() default) - Selector export modes only control traces/metrics, not logs - Updated all tests to cover logs export scenarios The logs enabled flag is passed separately from the ExportModes (which only covers traces/metrics) since logs are not yet part of the per-selector export mode configuration.
No description provided.