Feedback on conditional compilation for tracing (actix)

[ckcr4lyf]

Hey guys, I am adding tracing (via opentelemetry and jaeger) to my actix server. However, I want the tracing to be configurable at compile time, in case someone does not want the feature at all.

I am able to kinda separate it via something like this:

#[cfg(feature = "tracing")]
#[get("/healthz")]
async fn healthz(data: web::Data<AppState>) -> HttpResponse {
    let tracer = global::tracer("healthz");
    tracer.in_span("index", |ctx| async move {
        ctx.span().set_attribute(Key::new("parameter").i64(10));
        let mut rc = data.redis_connection.clone();
        let () = match redis::cmd("PING").query_async::<redis::aio::MultiplexedConnection, ()>(&mut rc).await {
            Ok(_) => {
                return HttpResponse::build(StatusCode::OK).append_header(header::ContentType::plaintext()).body("OK");
            },
            Err(_) => {
                return HttpResponse::build(StatusCode::INTERNAL_SERVER_ERROR).append_header(header::ContentType::plaintext()).body("OOF");
            }
        };
    }).await
}

#[cfg(not(feature = "tracing"))]
#[get("/healthz")]
async fn healthz(data: web::Data<AppState>) -> HttpResponse {
    let mut rc = data.redis_connection.clone();
    let () = match redis::cmd("PING").query_async::<redis::aio::MultiplexedConnection, ()>(&mut rc).await {
        Ok(_) => {
            return HttpResponse::build(StatusCode::OK).append_header(header::ContentType::plaintext()).body("OK");
        },
        Err(_) => {
            return HttpResponse::build(StatusCode::INTERNAL_SERVER_ERROR).append_header(header::ContentType::plaintext()).body("OOF");
        }
    };
}

I was wondering, since basically only the initialization of global tracer and the closure are the differences, is there some cleaner way to achieve this? One approach I can think of is take out the actual handler into its own function, and then just call that, but still have two separately defined handlers, based on config. E.g.:

#[cfg(feature = "tracing")]
#[get("/healthz")]
async fn healthz(data: web::Data<AppState>) -> HttpResponse {
    let tracer = global::tracer("healthz");
    tracer.in_span("index", |ctx| async move {
        ctx.span().set_attribute(Key::new("parameter").i64(10));
        return healthz_handler(data).await;
    }).await
}


#[cfg(not(feature = "tracing"))]
#[get("/healthz")]
async fn healthz(data: web::Data<AppState>) -> HttpResponse {
    return healthz_handler(data).await;
}

async fn healthz_handler(data: web::Data<AppState>) -> HttpResponse {
    let mut rc = data.redis_connection.clone();
    let () = match redis::cmd("PING").query_async::<redis::aio::MultiplexedConnection, ()>(&mut rc).await {
        Ok(_) => {
            return HttpResponse::build(StatusCode::OK).append_header(header::ContentType::plaintext()).body("OK");
        },
        Err(_) => {
            return HttpResponse::build(StatusCode::INTERNAL_SERVER_ERROR).append_header(header::ContentType::plaintext()).body("OOF");
        }
    };
}

Thanks for any feedback.

Link to code in repo: kiryuu/src/main.rs at e0d16eba69acc96e805b6ddd86f11cfbcbb2f738 · ckcr4lyf/kiryuu · GitHub

[jofas]

How about putting logging in a middleware and only conditionally adding it to your server, if someone compiles with the tracing feature? I see in your repo you already have a middleware for general logging. You can register middlewares for individual routes as well, if you put the route in a resource.

[Hyeonu]

Why don't you use #[tracing::instrument]?

#[get("/healthz")]
#[cfg_attr(feature = "tracing", tracing::instrument)]
async fn healthz(data: web::Data<AppState>) -> HttpResponse {...}

[ckcr4lyf]

Thanks for the suggestion, I think it kinda works. I'll play around with it a bit more. kiryuu/src/main.rs at e5987fb9c8bc8c940e337a59043eb80339d6b11d · ckcr4lyf/kiryuu · GitHub

        .service(web::resource("/healthz")
            .wrap_fn(|req, srv| {
                #[cfg(feature = "tracing")]
                {
                    let tracer = global::tracer("healthz");
                    tracer.in_span("healthz", move |cx| {
                        cx.span()
                            .set_attribute(Key::new("path").string(req.path().to_string()));
                        srv.call(req).with_context(cx)
                    })  
                }
                #[cfg(not(feature = "tracing"))]
                {
                    srv.call(req)
                }
            })
            .route(web::get().to(move |data| {
                healthz_handler(data)
            }))
        )

[ckcr4lyf]

I tried at a very high level but it doesn't seem to work. I think I will need to go through the docs to see how to make it work with the tracing exporter already configured, but it looks promising