Skip to content

Commit

Permalink
Try to stop OTLP controllers when Telemetry is dropped
Browse files Browse the repository at this point in the history
We already have code to specifically drop tracers and we are adding some
additional logic to do the same thing with metrics exporters.

fixes: #3140
  • Loading branch information
garypen committed May 25, 2023
1 parent 3ed3e85 commit db0ba71
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions apollo-router/src/plugins/telemetry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use opentelemetry::propagation::text_map_propagator::FieldIter;
use opentelemetry::propagation::Extractor;
use opentelemetry::propagation::Injector;
use opentelemetry::propagation::TextMapPropagator;
use opentelemetry::sdk::metrics::controllers::BasicController;
use opentelemetry::sdk::propagation::TextMapCompositePropagator;
use opentelemetry::sdk::trace::Builder;
use opentelemetry::trace::SpanContext;
Expand All @@ -36,6 +37,7 @@ use opentelemetry::trace::TraceContextExt;
use opentelemetry::trace::TraceFlags;
use opentelemetry::trace::TraceState;
use opentelemetry::trace::TracerProvider;
use opentelemetry::Context as OtelContext;
use opentelemetry::KeyValue;
use rand::Rng;
use router_bridge::planner::UsageReporting;
Expand Down Expand Up @@ -139,10 +141,10 @@ const DEFAULT_EXPOSE_TRACE_ID_HEADER: &str = "apollo-trace-id";
pub(crate) struct Telemetry {
config: Arc<config::Conf>,
metrics: BasicMetrics,
// Do not remove _metrics_exporters. Metrics will not be exported if it is removed.
// Do not remove metrics_exporters. Metrics will not be exported if it is removed.
// Typically the handles are a PushController but may be something else. Dropping the handle will
// shutdown exporter.
_metrics_exporters: Vec<MetricsExporterHandle>,
metrics_exporters: Vec<MetricsExporterHandle>,
custom_endpoints: MultiMap<ListenAddr, Endpoint>,
apollo_metrics_sender: apollo_exporter::Sender,
field_level_instrumentation_ratio: f64,
Expand Down Expand Up @@ -186,6 +188,20 @@ fn setup_metrics_exporter<T: MetricsConfigurator>(

impl Drop for Telemetry {
fn drop(&mut self) {
// If we can downcast the metrics exporter to be a `BasicController`, then we
// should stop it to ensure metrics are transmitted before the exporter is dropped.
for exporter in self.metrics_exporters.drain(..) {
if let Ok(controller) = MetricsExporterHandle::downcast::<BasicController>(exporter) {
::tracing::debug!("stopping basic controller: {controller:?}");
let cx = OtelContext::current();

thread::spawn(move || {
if let Err(e) = controller.stop(&cx) {
::tracing::error!("error during basic controller stop: {e}");
}
});
}
}
// If for some reason we didn't use the trace provider then safely discard it e.g. some other plugin failed `new`
// To ensure we don't hang tracing providers are dropped in a blocking task.
// https://github.com/open-telemetry/opentelemetry-rust/issues/868#issuecomment-1250387989
Expand Down Expand Up @@ -218,7 +234,7 @@ impl Plugin for Telemetry {
let meter_provider = metrics_builder.meter_provider();
Ok(Telemetry {
custom_endpoints: metrics_builder.custom_endpoints(),
_metrics_exporters: metrics_builder.exporters(),
metrics_exporters: metrics_builder.exporters(),
metrics: BasicMetrics::new(&meter_provider),
apollo_metrics_sender: metrics_builder.apollo_metrics_provider(),
field_level_instrumentation_ratio,
Expand Down

0 comments on commit db0ba71

Please sign in to comment.