-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix(observability): propagate component span into spawned tasks #25521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Internal telemetry (metrics and logs) emitted from work that Vector runs on spawned `tokio` tasks now correctly inherits the owning component's tags (`component_id`, `component_kind`, `component_type`). Previously, several components spawned background tasks without propagating the tracing span, so some internal events emitted from those tasks were missing their component tags. Affected emissions include the `datadog_logs` sink's `component_discarded_events_total` (events too large to encode), the `gcp_pubsub` source's `component_errors_total`/`component_discarded_events_total` from its per-stream tasks, and the `splunk_hec` sinks' acknowledgement-handling `component_errors_total`. | ||
|
|
||
| authors: gwenaskell | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ use axum::{ | |
| use tokio::sync::oneshot; | ||
| use tonic::transport::Server as TonicServer; | ||
| use tonic_health::server::{HealthReporter, health_reporter}; | ||
| use tracing::Instrument; | ||
| use vector_lib::tap::topology::WatchRx; | ||
|
|
||
| use super::grpc::ObservabilityService; | ||
|
|
@@ -80,45 +81,48 @@ impl GrpcServer { | |
| let router_serving = Arc::clone(&serving); | ||
|
|
||
| // Spawn the server with the already-bound listener | ||
| tokio::spawn(async move { | ||
| // Build reflection service for tools like grpcurl | ||
| let reflection_service = tonic_reflection::server::Builder::configure() | ||
| .register_encoded_file_descriptor_set( | ||
| crate::proto::observability::FILE_DESCRIPTOR_SET, | ||
| ) | ||
| .register_encoded_file_descriptor_set(tonic_health::pb::FILE_DESCRIPTOR_SET) | ||
| .build() | ||
| .expect("Failed to build reflection service"); | ||
|
|
||
| // Build the tonic router (gRPC services) and merge with the HTTP router | ||
| // so both protocols share the same port. `accept_http1(true)` lets plain | ||
| // HTTP/1.1 requests reach the merged axum routes. | ||
| let router = TonicServer::builder() | ||
| .accept_http1(true) | ||
| .add_service(health_service) | ||
| .add_service(ObservabilityServer::new(service)) | ||
| .add_service(reflection_service) | ||
| .into_router() | ||
| .merge(http_router(router_serving)); | ||
|
|
||
| let result = hyper::Server::from_tcp(std_listener) | ||
| .expect("Failed to build HTTP server from TCP listener") | ||
| .serve(router.into_make_service()) | ||
| .with_graceful_shutdown(async { | ||
| rx.await.ok(); | ||
| info!("GRPC API server shutting down."); | ||
| }) | ||
| .await; | ||
|
|
||
| if let Err(e) = result { | ||
| error!( | ||
| message = "GRPC server encountered an error.", | ||
| error = %e, | ||
| error_source = ?e.source(), | ||
| bind_addr = %actual_addr, | ||
| ); | ||
| tokio::spawn( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be helpful to have a spawn_in_current_task wrapper for this common operation? It's a relatively trivial amount of code duplication, but it would simplify searching for spawns that are missing spans and potentially even block calling tokio::spawn directly without proper annotations. |
||
| async move { | ||
| // Build reflection service for tools like grpcurl | ||
| let reflection_service = tonic_reflection::server::Builder::configure() | ||
| .register_encoded_file_descriptor_set( | ||
| crate::proto::observability::FILE_DESCRIPTOR_SET, | ||
| ) | ||
| .register_encoded_file_descriptor_set(tonic_health::pb::FILE_DESCRIPTOR_SET) | ||
| .build() | ||
| .expect("Failed to build reflection service"); | ||
|
|
||
| // Build the tonic router (gRPC services) and merge with the HTTP router | ||
| // so both protocols share the same port. `accept_http1(true)` lets plain | ||
| // HTTP/1.1 requests reach the merged axum routes. | ||
| let router = TonicServer::builder() | ||
| .accept_http1(true) | ||
| .add_service(health_service) | ||
| .add_service(ObservabilityServer::new(service)) | ||
| .add_service(reflection_service) | ||
| .into_router() | ||
| .merge(http_router(router_serving)); | ||
|
|
||
| let result = hyper::Server::from_tcp(std_listener) | ||
| .expect("Failed to build HTTP server from TCP listener") | ||
| .serve(router.into_make_service()) | ||
| .with_graceful_shutdown(async { | ||
| rx.await.ok(); | ||
| info!("GRPC API server shutting down."); | ||
| }) | ||
| .await; | ||
|
|
||
| if let Err(e) = result { | ||
| error!( | ||
| message = "GRPC server encountered an error.", | ||
| error = %e, | ||
| error_source = ?e.source(), | ||
| bind_addr = %actual_addr, | ||
| ); | ||
| } | ||
| } | ||
| }); | ||
| .in_current_span(), | ||
| ); | ||
|
|
||
| info!("GRPC API server started on {}.", actual_addr); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.