inject span data using tower http

crates/bin/pd/Cargo.toml

@@ -110,7 +110,7 @@ tonic-web = { workspace = true }
 tower = { workspace = true, features = ["full"] }
 tower-abci = "0.18"
 tower-actor = "0.1.0"
-tower-http = { workspace = true, features = ["cors"] }
+tower-http = { workspace = true, features = ["cors", "trace"] }
 tower-service = { workspace = true }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true, features = ["env-filter", "ansi"] }

crates/bin/pd/src/main.rs

@@ -21,10 +21,13 @@ use pd::{
 };
 use penumbra_app::app_version::check_and_update_app_version;
 use penumbra_app::{APP_VERSION, SUBSTORE_PREFIXES};
+use penumbra_tower_trace::remote_addr;
 use rand::Rng;
 use rand_core::OsRng;
 use tendermint_config::net::Address as TendermintAddress;
+use tower::ServiceBuilder;
 use tower_http::cors::CorsLayer;
+use tower_http::trace::TraceLayer;
 use tracing::Instrument as _;
 use tracing_subscriber::{prelude::*, EnvFilter};
 use url::Url;
@@ -130,16 +133,25 @@ async fn main() -> anyhow::Result<()> {
 
             let tm_proxy = penumbra_tendermint_proxy::TendermintProxy::new(cometbft_addr);
 
-            let grpc_routes = penumbra_app::rpc::routes(&storage, tm_proxy, enable_expensive_rpc)?;
-            // let grpc_server = penumbra_app::rpc::router(&storage, tm_proxy, enable_expensive_rpc)?;
+            let grpc_routes = penumbra_app::rpc::routes(&storage, tm_proxy, enable_expensive_rpc)?
+                .into_axum_router()
+                .layer(
+                    ServiceBuilder::new().layer(TraceLayer::new_for_grpc().make_span_with(
+                        |req: &http::Request<_>| match remote_addr(req) {
+                            Some(remote_addr) => {
+                                tracing::error_span!("grpc", ?remote_addr)
+                            }
+                            None => tracing::error_span!("grpc"),
+                        },
+                    )),
+                );
 
             // Create Axum routes for the frontend app.
             let frontend = pd::zipserve::router("/app/", pd::MINIFRONT_ARCHIVE_BYTES);
             let node_status = pd::zipserve::router("/", pd::NODE_STATUS_ARCHIVE_BYTES);
 
             // Now we drop down a layer of abstraction, from tonic to axum, and merge handlers.
             let router = grpc_routes
-                .into_axum_router()
                 .merge(frontend)
                 .merge(node_status)
                 // Set rather permissive CORS headers for pd's gRPC: the service
@@ -152,6 +164,13 @@ async fn main() -> anyhow::Result<()> {
             // Now start the GRPC server, initializing an ACME client to use as a certificate
             // resolver if auto-https has been enabled. if auto-https is not enabled, we will
             // instead spawn a future that will never return.
+
+            // TODO(janis): Is `axum_server::bind` sufficient to accept http1 (for grpc-web) and
+            // http2 (for grpc) requests?
+            //
+            // See also this (about axum::serve, suggesting that it just works out of the box; does that
+            // apply to axum_server::bind as well?)
+            // https://github.com/tokio-rs/axum/blob/c596deafe48ed608775e312eef7d12ddbb0fd424/examples/websockets-http2/src/main.rs#L57-L59
             let grpc_server = axum_server::bind(grpc_bind);
             let (grpc_server, acme_worker) = match grpc_auto_https {
                 Some(domain) => {

crates/core/app/src/rpc.rs

@@ -65,29 +65,6 @@ pub fn routes(
 ) -> anyhow::Result<tonic::service::Routes> {
     let ibc = penumbra_ibc::component::rpc::IbcQuery::<PenumbraHost>::new(storage.clone());
 
-    // TODO(janis): upgrading to tonic 0.12 meant that the batteries included server could no
-    // longer be used. So we now need to configure this on the axum/hyper server directly.
-    // What can be done? Instead of `trace_fn` one could make use of tower-http's
-    // TraceLayer.
-    //
-    // HTTP1 and HTTP2 now need to be configured at a higher level - but if we make use
-    // of `axum::serve`, then they seem to both be activated?
-    // See also this:
-    // https://github.com/tokio-rs/axum/blob/c596deafe48ed608775e312eef7d12ddbb0fd424/examples/websockets-http2/src/main.rs#L57-L59
-
-    // let grpc_server = tonic::transport::server::Server::builder()
-    //     .trace_fn(|req| match remote_addr(req) {
-    //         Some(remote_addr) => {
-    //             tracing::error_span!("grpc", ?remote_addr)
-    //         }
-    //         None => tracing::error_span!("grpc"),
-    //     })
-    //     // Allow HTTP/1, which will be used by grpc-web connections.
-    //     // This is particularly important when running locally, as gRPC
-    //     // typically uses HTTP/2, which requires HTTPS. Accepting HTTP/2
-    //     // allows local applications such as web browsers to talk to pd.
-    //     .accept_http1(true)
-
     let mut builder = Routes::builder();
     builder
         // As part of #2932, we are disabling all timeouts until we circle back to our

crates/util/tower-trace/src/lib.rs

@@ -11,7 +11,7 @@ use std::net::SocketAddr;
 // request level, but the hook available to do that gives us an http::Request
 // rather than a tonic::Request, so the tonic::Request::remote_addr method isn't
 // available.
-pub fn remote_addr(req: &http::Request<()>) -> Option<SocketAddr> {
+pub fn remote_addr<B>(req: &http::Request<B>) -> Option<SocketAddr> {
     use tonic::transport::server::TcpConnectInfo;
     // NOTE: needs to also check TlsConnectInfo if we use TLS
     req.extensions()