diff --git a/sentry-actix/src/lib.rs b/sentry-actix/src/lib.rs index 00dd3951..376854d4 100644 --- a/sentry-actix/src/lib.rs +++ b/sentry-actix/src/lib.rs @@ -237,12 +237,10 @@ where .as_ref() .map_or(false, |client| client.options().send_default_pii); - let (mut tx, sentry_req) = sentry_request_from_http(&req, with_pii); + let sentry_req = sentry_request_from_http(&req, with_pii); + let name = transaction_name_from_http(&req); let transaction = if inner.start_transaction { - let name = std::mem::take(&mut tx) - .unwrap_or_else(|| format!("{} {}", req.method(), req.uri())); - let headers = req.headers().iter().flat_map(|(header, value)| { value.to_str().ok().map(|value| (header.as_str(), value)) }); @@ -262,7 +260,7 @@ where if let Some(transaction) = transaction.as_ref() { scope.set_span(Some(transaction.clone().into())); } else { - scope.set_transaction(tx.as_deref()); + scope.set_transaction((!inner.start_transaction).then_some(&name)); } scope.add_event_processor(move |event| Some(process_event(event, &sentry_req))); parent_span @@ -336,14 +334,14 @@ fn map_status(status: StatusCode) -> protocol::SpanStatus { } } -/// Build a Sentry request struct from the HTTP request -fn sentry_request_from_http(request: &ServiceRequest, with_pii: bool) -> (Option, Request) { - let transaction = if let Some(name) = request.match_name() { - Some(String::from(name)) - } else { - request.match_pattern() - }; +/// Extract a transaction name from the HTTP request +fn transaction_name_from_http(req: &ServiceRequest) -> String { + let path_part = req.match_pattern().unwrap_or_else(|| "".to_string()); + format!("{} {}", req.method(), path_part) +} +/// Build a Sentry request struct from the HTTP request +fn sentry_request_from_http(request: &ServiceRequest, with_pii: bool) -> Request { let mut sentry_req = Request { url: format!( "{}://{}{}", @@ -369,7 +367,7 @@ fn sentry_request_from_http(request: &ServiceRequest, with_pii: bool) -> (Option } }; - (transaction, sentry_req) + sentry_req } /// Add request data to a Sentry event @@ -451,13 +449,56 @@ mod tests { assert_eq!(events.len(), 2); for event in events { let request = event.request.expect("Request should be set."); - assert_eq!(event.transaction, Some("/test".into())); + assert_eq!(event.transaction, Some("GET /test".into())); assert_eq!(event.message, Some("Message".into())); assert_eq!(event.level, Level::Warning); assert_eq!(request.method, Some("GET".into())); } } + /// Test transaction name HTTP verb. + #[actix_web::test] + async fn test_match_pattern() { + let events = sentry::test::with_captured_events(|| { + block_on(async { + let service = |_name: String| { + // Current Hub should have no events + _assert_hub_no_events(); + + sentry::capture_message("Message", Level::Warning); + + // Current Hub should have the event + _assert_hub_has_events(); + + HttpResponse::Ok() + }; + + let app = init_service( + App::new() + .wrap(Sentry::builder().with_hub(Hub::current()).finish()) + .service(web::resource("/test/{name}").route(web::post().to(service))), + ) + .await; + + // Call the service twice (sequentially) to ensure the middleware isn't sticky + for _ in 0..2 { + let req = TestRequest::post().uri("/test/fake_name").to_request(); + let res = call_service(&app, req).await; + assert!(res.status().is_success()); + } + }) + }); + + assert_eq!(events.len(), 2); + for event in events { + let request = event.request.expect("Request should be set."); + assert_eq!(event.transaction, Some("POST /test/{name}".into())); + assert_eq!(event.message, Some("Message".into())); + assert_eq!(event.level, Level::Warning); + assert_eq!(request.method, Some("POST".into())); + } + } + /// Ensures errors returned in the Actix service trigger an event. #[actix_web::test] async fn test_response_errors() { @@ -490,7 +531,7 @@ mod tests { assert_eq!(events.len(), 2); for event in events { let request = event.request.expect("Request should be set."); - assert_eq!(event.transaction, Some("failing".into())); // Transaction name is the name of the function + assert_eq!(event.transaction, Some("GET /test".into())); // Transaction name is the matcher of the route assert_eq!(event.message, None); assert_eq!(event.exception.values[0].ty, String::from("Custom")); assert_eq!(event.exception.values[0].value, Some("Test Error".into()));