diff --git a/changelog.d/26456_opentelemetry_http_rejection_status.fix.md b/changelog.d/26456_opentelemetry_http_rejection_status.fix.md new file mode 100644 index 0000000000000..3fa00a2ebaf77 --- /dev/null +++ b/changelog.d/26456_opentelemetry_http_rejection_status.fix.md @@ -0,0 +1,3 @@ +The `opentelemetry` source's OTLP/HTTP endpoints now return client error status codes for requests they can't route, instead of `500 Internal Server Error` with an internal debug message in the body. An unsupported or missing `Content-Type` (for example `application/json`) now returns `415 Unsupported Media Type`, a method other than `POST` returns `405 Method Not Allowed`, and an unknown path returns `404 Not Found`. + +authors: Andrew-Hinson diff --git a/src/sources/opentelemetry/http.rs b/src/sources/opentelemetry/http.rs index c762cbeecef8e..a052bb91bdafb 100644 --- a/src/sources/opentelemetry/http.rs +++ b/src/sources/opentelemetry/http.rs @@ -482,25 +482,42 @@ fn acknowledgement_failure_response(status: AcknowledgementFailure) -> Response warp::reply::with_status(response, StatusCode::INTERNAL_SERVER_ERROR).into_response() } -async fn handle_rejection(err: Rejection) -> Result { - if let Some(err_msg) = err.find::() { - let reply = protobuf(Status { - code: 2, // UNKNOWN - OTLP doesn't require use of status.code, but we can't encode a None here - message: err_msg.message().into(), - ..Default::default() - }); - - Ok(warp::reply::with_status(reply, err_msg.status_code())) +async fn handle_rejection(err: Rejection) -> Result { + let (message, status) = if let Some(err_msg) = err.find::() { + (err_msg.message().into(), err_msg.status_code()) + } else if is_content_type_rejection(&err) { + // The route only matches `Content-Type: application/x-protobuf`, so any other (or a + // missing) content type is a client error rather than an internal one. warp would + // report this as `400 Bad Request`, so it is handled here. + ( + "Unsupported content type; this endpoint requires `application/x-protobuf`.".into(), + StatusCode::UNSUPPORTED_MEDIA_TYPE, + ) + } else if err.find::().is_some() { + (format!("{err:?}"), StatusCode::INTERNAL_SERVER_ERROR) } else { - let reply = protobuf(Status { - code: 2, // UNKNOWN - OTLP doesn't require use of status.code, but we can't encode a None here - message: format!("{err:?}"), - ..Default::default() - }); - - Ok(warp::reply::with_status( - reply, - StatusCode::INTERNAL_SERVER_ERROR, - )) - } + return Err(err); + }; + + let reply = protobuf(Status { + code: 2, // UNKNOWN - OTLP doesn't require use of status.code, but we can't encode a None here + message, + ..Default::default() + }); + + Ok(warp::reply::with_status(reply, status)) +} + +/// Returns true if the request was rejected because of its `Content-Type` header. +/// +/// `warp::header::exact_ignore_case` rejects with `InvalidHeader` both when the header has a +/// different value and when it is absent; `MissingHeader` is checked as well for robustness. +fn is_content_type_rejection(err: &Rejection) -> bool { + let is_content_type = + |name: &str| name.eq_ignore_ascii_case(http::header::CONTENT_TYPE.as_str()); + err.find::() + .is_some_and(|e| is_content_type(e.name())) + || err + .find::() + .is_some_and(|e| is_content_type(e.name())) } diff --git a/src/sources/opentelemetry/tests.rs b/src/sources/opentelemetry/tests.rs index a41fe5eab556e..7276108790858 100644 --- a/src/sources/opentelemetry/tests.rs +++ b/src/sources/opentelemetry/tests.rs @@ -2001,6 +2001,73 @@ async fn http_logs_use_otlp_decoding_emits_metric() { } } +/// Sends a request to the OTLP/HTTP endpoint and returns the status code with the decoded +/// `google.rpc.Status` message from the response body. +async fn send_http_request(request: reqwest::RequestBuilder) -> (reqwest::StatusCode, String) { + let response = request.send().await.expect("Failed to send request."); + let status = response.status(); + let body = response + .bytes() + .await + .expect("Failed to read response body."); + let message = super::status::Status::decode(body) + .expect("Response body should be a protobuf `Status`.") + .message; + (status, message) +} + +#[tokio::test] +async fn http_rejections_return_client_error_status_codes() { + let env = build_otlp_test_env(LOGS, None).await; + let http_addr = env.config.http.address; + test_util::wait_for_tcp(http_addr).await; + let client = reqwest::Client::new(); + let logs_url = format!("http://{http_addr}/v1/logs"); + + // JSON is not supported, so the request should be rejected as an unsupported media type. + let (status, message) = send_http_request( + client + .post(&logs_url) + .header("Content-Type", "application/json") + .body(r#"{"resourceLogs":[]}"#), + ) + .await; + assert_eq!(status, reqwest::StatusCode::UNSUPPORTED_MEDIA_TYPE); + assert!(message.contains("application/x-protobuf"), "{message}"); + + // A missing content type is treated the same as an unsupported one. + let (status, _) = send_http_request(client.post(&logs_url)).await; + assert_eq!(status, reqwest::StatusCode::UNSUPPORTED_MEDIA_TYPE); + + // Other rejections are mapped by warp, which replies with a plain-text body. + // Only `POST` is routed. + let response = client + .get(&logs_url) + .send() + .await + .expect("Failed to send request."); + assert_eq!(response.status(), reqwest::StatusCode::METHOD_NOT_ALLOWED); + + // Unknown paths are not found. + let response = client + .post(format!("http://{http_addr}/v1/unknown")) + .header("Content-Type", "application/x-protobuf") + .send() + .await + .expect("Failed to send request."); + assert_eq!(response.status(), reqwest::StatusCode::NOT_FOUND); + + // The supported content type is still accepted. + let response = client + .post(&logs_url) + .header("Content-Type", "application/x-protobuf") + .body(ExportLogsServiceRequest::default().encode_to_vec()) + .send() + .await + .expect("Failed to send request."); + assert_eq!(response.status(), reqwest::StatusCode::OK); +} + #[cfg(test)] mod otlp_decoding_config_tests { use indoc::indoc;