diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c6f3fb..a2222e6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- The Tower integration's [`SentryHttpLayer`](https://docs.rs/sentry-tower/0.49.3/sentry_tower/struct.SentryHttpLayer.html) now records the [`http.response.status_code`](https://getsentry.github.io/sentry-conventions/attributes/http/) attribute on transactions ([#1253](https://github.com/getsentry/sentry-rust/pull/1253)). + ## 0.49.2 ### Fixes diff --git a/Cargo.lock b/Cargo.lock index 57e995df..8bce1d24 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3348,6 +3348,7 @@ dependencies = [ "curl", "embedded-svc", "esp-idf-svc", + "http 1.5.0", "httpdate", "log", "native-tls", diff --git a/sentry-tower/src/http.rs b/sentry-tower/src/http.rs index adc86225..597133a1 100644 --- a/sentry-tower/src/http.rs +++ b/sentry-tower/src/http.rs @@ -138,12 +138,27 @@ where match slf.future.poll(cx) { Poll::Ready(res) => { if let Some((transaction, parent_span)) = slf.transaction.take() { - if transaction.get_status().is_none() { - let status = match &res { - Ok(res) => map_status(res.status()), - Err(_) => protocol::SpanStatus::UnknownError, - }; - transaction.set_status(status); + match &res { + Ok(res) => { + if !transaction + .get_trace_context() + .data + .contains_key("http.response.status_code") + { + transaction.set_data( + "http.response.status_code", + res.status().as_u16().into(), + ); + } + if transaction.get_status().is_none() { + transaction.set_status(map_status(res.status())); + } + } + Err(_) => { + if transaction.get_status().is_none() { + transaction.set_status(protocol::SpanStatus::UnknownError); + } + } } transaction.finish(); sentry_core::configure_scope(|scope| scope.set_span(parent_span)); diff --git a/sentry/Cargo.toml b/sentry/Cargo.toml index b45cb11d..840c1d04 100644 --- a/sentry/Cargo.toml +++ b/sentry/Cargo.toml @@ -96,10 +96,11 @@ esp-idf-svc = { workspace = true, optional = true } sentry-anyhow = { workspace = true } sentry-log = { workspace = true } sentry-slog = { workspace = true } -sentry-tower = { workspace = true } +sentry-tower = { workspace = true, features = ["http"] } sentry-tracing = { workspace = true } actix-web = { workspace = true } anyhow = { workspace = true } +http = { workspace = true } log = { workspace = true, features = ["std"] } pretty_env_logger = { workspace = true } slog = { workspace = true } diff --git a/sentry/tests/test_tower.rs b/sentry/tests/test_tower.rs index 4023e2cd..cc8c51c7 100644 --- a/sentry/tests/test_tower.rs +++ b/sentry/tests/test_tower.rs @@ -3,13 +3,64 @@ use std::sync::Arc; use sentry::{ - protocol::{Breadcrumb, Level}, + protocol::{Breadcrumb, Context, EnvelopeItem, Level, SpanStatus}, test::TestTransport, ClientOptions, Hub, }; -use sentry_tower::SentryLayer; +use sentry_tower::{SentryHttpLayer, SentryLayer}; use tower::{ServiceBuilder, ServiceExt}; +#[test] +fn test_tower_http_records_response_status_code() { + let options = ClientOptions::new().traces_sample_rate(1.0); + + let envelopes = sentry::test::with_captured_envelopes_options( + || { + let service = ServiceBuilder::new() + .layer(SentryHttpLayer::new().enable_transaction()) + .service_fn(|_req: http::Request<()>| async move { + Ok::<_, std::convert::Infallible>( + http::Response::builder() + .status(http::StatusCode::NOT_FOUND) + .body(()) + .unwrap(), + ) + }); + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let request = http::Request::builder() + .method(http::Method::GET) + .uri("http://example.com/missing") + .body(()) + .unwrap(); + let response = rt.block_on(service.oneshot(request)).unwrap(); + assert_eq!(response.status(), http::StatusCode::NOT_FOUND); + }, + options, + ); + + assert_eq!(envelopes.len(), 1); + let transaction = match envelopes[0].items().next().unwrap() { + EnvelopeItem::Transaction(transaction) => transaction, + _ => panic!("expected a transaction item"), + }; + + assert_eq!(transaction.name.as_deref(), Some("GET /missing")); + let Context::Trace(trace) = transaction.contexts.get("trace").unwrap() else { + panic!("expected a trace context"); + }; + assert_eq!(trace.op.as_deref(), Some("http.server")); + assert_eq!(trace.origin.as_deref(), Some("auto.http.tower")); + assert_eq!(trace.status, Some(SpanStatus::NotFound)); + assert_eq!( + trace.data.get("http.response.status_code"), + Some(&404.into()) + ); +} + #[test] fn test_tower_hub() { // Create a fake transport for new hubs