feat: enable OTLP metrics exporter for daemon services #3086
Conversation
Signed-off-by: George Zachariah <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3086 +/- ##
==========================================
+ Coverage 79.72% 79.74% +0.01%
==========================================
Files 288 289 +1
Lines 64971 65024 +53
==========================================
+ Hits 51800 51852 +52
+ Misses 12623 12621 -2
- Partials 548 551 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
please run "make codegen".. also this does not fix the parent issue, can you open a sub issue under the parent issue? |
pkg/shared/telemetry/otel.go
Outdated
| // OTEL_EXPORTER_OTLP_ENDPOINT environment variable is set. This allows metrics | ||
| // to be exported to an OTLP collector while keeping Prometheus scraping unchanged. | ||
| func InitOTLPExporter(ctx context.Context, componentName, componentInstance string, gatherer prometheus.Gatherer) (func(context.Context) error, error) { | ||
| endpoint := os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT") |
There was a problem hiding this comment.
Use pkg/shared/util.LookupEnvStringOr
pkg/shared/telemetry/otel.go
Outdated
| res, err := resource.New(ctx, | ||
| resource.WithAttributes( | ||
| attribute.String("service.name", componentName), | ||
| attribute.String("service.namespace", "numaflow"), |
There was a problem hiding this comment.
Use pkg/apis.Project to replace "numaflow".
Signed-off-by: George Zachariah <[email protected]>
Signed-off-by: George Zachariah <[email protected]>
Signed-off-by: George Zachariah <[email protected]>
| log.Warnw("Failed to initialize OTLP exporter, continuing without OTLP", zap.Error(err)) | ||
| } else { | ||
| defer func() { | ||
| _ = shutdown(context.Background()) |
There was a problem hiding this comment.
| _ = shutdown(context.Background()) | |
| _ = shutdown(ctx) |
any reason why we can't do this?
There was a problem hiding this comment.
I used context.Background() here because the server’s ctx is cancelled during shutdown which prevents the OTLP exporter from flushing its final metrics.
This follows OTEL Go examples, which also use context.Background() for graceful shutdown https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric#section-readme
That said, I’m happy to adjust based on what we prefer in Numaflow
- should we wrap it in a timed context (eg ctx with a timeout) to avoid hanging on shutdown
- should we skip calling
Shutdownentirely and just rely on process exit?
Please let me know if we need a change here.
3 participants
What this PR does / why we need it
This PR adds optional support for exporting Numaflow metrics through the OpenTelemetry Protocol (OTLP), alongside the existing Prometheus scraping. The goal is to make the metrics pipeline more vendor-neutral, similar to Argo Workflows’ approach, where metrics can be scraped by Prometheus or pushed via OTLP. Prometheus behavior remains unchanged. OTLP export is enabled only when an OTEL endpoint environment variable is set.
Related issues
Fixes #3087
Part of #3037
Discussed in #3035
Testing
Verified:
Special notes for reviewers