feat: metrics reporting from trace export #510
Conversation
fbogsany
requested review from
dazuma,
ericmustin,
mwear and
robertlaurin
as code owners
|
@fbogsany on first pass this looks good, I think this is super valuable but curious if there's any specification guidance around this? I maybe have some nits on the naming conventions but otherwise lgtm |
There's no guidance in the spec yet. I was asked awhile back to add some, but they're considered post-GA at this point. Feel free to bikeshed the naming conventions (though also keep in mind that an implementation can map these to whatever makes sense in a given production environment). |
| end | ||
|
|
||
| def report_dropped_spans(count, reason:) | ||
| @metrics_reporter.add_to_counter('otel.bsp.dropped_span', increment: count, labels: { 'reason' => reason }) |
There was a problem hiding this comment.
nit: should this be plural?
| OpenTelemetry.logger.error("Unable to export #{batch.size} spans") unless result_code == SUCCESS | ||
| if result_code == SUCCESS | ||
| @metrics_reporter.add_to_counter('otel.bsp.export.success') | ||
| @metrics_reporter.add_to_counter('otel.bsp.export.spans', increment: batch.size) |
There was a problem hiding this comment.
very small nit, should we have otel.bsp.export.spans and otel.bsp.dropped_spanhave the same namespace? so, eitherotel.bsp.export.dropped_spanorotel.bsp.spans`? non blocking imo
There was a problem hiding this comment.
Hmm, so initially I 👍 this suggestion, but upon reflection I don't think they should share a namespace. otel.bsp.export.spans is intended to communicate the number of spans successfully exported. otel.bsp.dropped_spans is a count of spans dropped, which might not be due to a failed export (we have 3 reasons at least for dropping spans: termination timeout, full buffer, failed export).
There was a problem hiding this comment.
I could see renaming otel.bsp.export.spans -> otel.bsp.exported_spans, so back to 👍 . I'll do that.
…metry/opentelemetry-ruby into trace_export_metrics_reporting
This enables reporting of metrics from the trace export pipeline, including the OTLP exporter and the BatchSpanProcessor. We need this mechanism to allow reporting metrics that are useful for monitoring trace export from applications in production, and we can't create a binding to the (currently) unstable Metrics API from the stable SDK.
The interface to metrics reporting is defined by the
OpenTelemetry::SDK::Trace::Export::MetricsReporter, which also provides the default empty implementation. I tried to stick with concepts that would map cleanly to the current spec for OTel Metrics, and also to StatsD. It's slightly weird in that we haverecord_value, which is a synchronous operation in OTel Metrics that maps to StatsDdistribution(orhistogramif you prefer that), and alsoobserve_value, which is asynchronous in OTel Metrics and maps to StatsDgauge. The weirdness comes about because we're calling both synchronously.The metrics I defined include:
otel.otlp_exporter.request_durationotel.otlp_exporter.failure("soft" failure - request will be retried)otel.bsp.buffer_utilization(a snapshot of "fullness" of the BSP buffer)otel.bsp.export.successotel.bsp.export.failure(hard failure - request will not be retried)otel.bsp.exported_spansotel.bsp.dropped_spans