fix(opentelemetry source): return 4xx instead of 500 for rejected HTTP requests

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

src/sources/opentelemetry/http.rs

@@ -452,24 +452,45 @@ async fn handle_request(
 }
 
 async fn handle_rejection(err: Rejection) -> Result<impl Reply, std::convert::Infallible> {
-    if let Some(err_msg) = err.find::<ErrorMessage>() {
-        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()))
+    let (message, status) = if let Some(err_msg) = err.find::<ErrorMessage>() {
+        (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.
+        (
+            "Unsupported content type; this endpoint requires `application/x-protobuf`.".into(),
+            StatusCode::UNSUPPORTED_MEDIA_TYPE,
+        )
+    } else if err.find::<warp::reject::MethodNotAllowed>().is_some() {
+        (
+            "Method not allowed; this endpoint requires `POST`.".into(),
+            StatusCode::METHOD_NOT_ALLOWED,
+        )
+    } else if err.is_not_found() {
+        ("Not found.".into(), StatusCode::NOT_FOUND)
     } 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,
-        ))
-    }
+        (format!("{err:?}"), StatusCode::INTERNAL_SERVER_ERROR)
+    };
+
+    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::<warp::reject::InvalidHeader>()
+        .is_some_and(|e| is_content_type(e.name()))
+        || err
+            .find::<warp::reject::MissingHeader>()
+            .is_some_and(|e| is_content_type(e.name()))
 }

src/sources/opentelemetry/tests.rs

@@ -1746,6 +1746,68 @@ 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);
+
+    // Only `POST` is routed.
+    let (status, _) = send_http_request(client.get(&logs_url)).await;
+    assert_eq!(status, reqwest::StatusCode::METHOD_NOT_ALLOWED);
+
+    // Unknown paths are not found.
+    let (status, _) = send_http_request(
+        client
+            .post(format!("http://{http_addr}/v1/unknown"))
+            .header("Content-Type", "application/x-protobuf"),
+    )
+    .await;
+    assert_eq!(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;