🐛 on grpc when no status code into header, fallback to OK (previously Unkown)

It's a quick fix, need test and maybe a better implementation and understanding (and being able to create tonic middleware that use tonic response and not http response)

FIX #122

Signed-off-by: David Bernard <david.bernard.31@gmail.com>

Author: davidB

examples/grpc/proto/helloworld.proto

@@ -1,9 +1,11 @@
 syntax = "proto3";
 
+import "google/protobuf/empty.proto";
 package helloworld;
 
 service Greeter {
   rpc SayHello (HelloRequest) returns (HelloReply) {}
+  rpc SayStatus (StatusRequest) returns (google.protobuf.Empty) {}
 }
 
 message HelloRequest {
@@ -13,3 +15,9 @@ message HelloRequest {
 message HelloReply {
   string message = 1;
 }
+
+message StatusRequest {
+  // https://github.com/grpc/grpc/blob/master/doc/statuscodes.md#status-codes-and-their-use-in-grpc
+  int32 code = 1;
+  string message = 2;
+}

examples/grpc/src/client.rs

@@ -1,6 +1,7 @@
 use hello_world::greeter_client::GreeterClient;
-use hello_world::HelloRequest;
+use hello_world::{HelloRequest, StatusRequest};
 use tonic::transport::Channel;
+use tonic::Code;
 use tonic_tracing_opentelemetry::middleware::client::OtelGrpcLayer;
 use tower::ServiceBuilder;
 
@@ -21,14 +22,35 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
     let channel = ServiceBuilder::new().layer(OtelGrpcLayer).service(channel);
 
     let mut client = GreeterClient::new(channel);
-
-    let request = tonic::Request::new(HelloRequest {
-        name: "Tonic".into(),
-    });
-
-    let response = client.say_hello(request).await?;
-
-    println!("RESPONSE={:?}", response);
+    {
+        let request = tonic::Request::new(HelloRequest {
+            name: "Tonic".into(),
+        });
+
+        let response = client.say_hello(request).await?;
+
+        println!("RESPONSE={:?}", response);
+    }
+    {
+        let request = tonic::Request::new(StatusRequest {
+            code: Code::NotFound.into(),
+            message: "not found...".into(),
+        });
+
+        let response = client.say_status(request).await;
+
+        println!("RESPONSE={:?}", response);
+    }
+    {
+        let request = tonic::Request::new(StatusRequest {
+            code: Code::DeadlineExceeded.into(),
+            message: "deadline...".into(),
+        });
+
+        let response = client.say_status(request).await;
+
+        println!("RESPONSE={:?}", response);
+    }
 
     opentelemetry::global::shutdown_tracer_provider();
     Ok(())

examples/grpc/src/server.rs

@@ -1,5 +1,6 @@
 use hello_world::greeter_server::{Greeter, GreeterServer};
-use hello_world::{HelloReply, HelloRequest};
+use hello_world::{HelloReply, HelloRequest, StatusRequest};
+use tonic::Code;
 use tonic::{transport::Server, Request, Response, Status};
 use tonic_tracing_opentelemetry::middleware::{filters, server};
 
@@ -32,6 +33,12 @@ impl Greeter for MyGreeter {
         };
         Ok(Response::new(reply))
     }
+
+    #[tracing::instrument(skip(self, request))]
+    async fn say_status(&self, request: Request<StatusRequest>) -> Result<Response<()>, Status> {
+        let request = request.into_inner();
+        Err(Status::new(Code::from(request.code), request.message))
+    }
 }
 
 #[tokio::main]

rust-toolchain.toml

@@ -0,0 +1,2 @@
+[toolchain]
+channel = "1.72.1"

tracing-opentelemetry-instrumentation-sdk/src/http/tools.rs

@@ -101,19 +101,73 @@ pub fn http_host<B>(req: &http::Request<B>) -> &str {
         .unwrap_or("")
 }
 
-/// If "grpc-status" can not be extracted from http response, the status "2" (UNKNOWN error) is defined
-//TODO create similar but with tonic::Response<B> ?
+/// gRPC status codes
+/// [gRPC status codes]: https://github.com/grpc/grpc/blob/master/doc/statuscodes.md#status-codes-and-their-use-in-grpc
+/// copied from tonic
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+pub enum GrpcCode {
+    /// The operation completed successfully.
+    Ok = 0,
+
+    /// The operation was cancelled.
+    Cancelled = 1,
+
+    /// Unknown error.
+    Unknown = 2,
+
+    /// Client specified an invalid argument.
+    InvalidArgument = 3,
+
+    /// Deadline expired before operation could complete.
+    DeadlineExceeded = 4,
+
+    /// Some requested entity was not found.
+    NotFound = 5,
+
+    /// Some entity that we attempted to create already exists.
+    AlreadyExists = 6,
+
+    /// The caller does not have permission to execute the specified operation.
+    PermissionDenied = 7,
+
+    /// Some resource has been exhausted.
+    ResourceExhausted = 8,
+
+    /// The system is not in a state required for the operation's execution.
+    FailedPrecondition = 9,
+
+    /// The operation was aborted.
+    Aborted = 10,
+
+    /// Operation was attempted past the valid range.
+    OutOfRange = 11,
+
+    /// Operation is not implemented or not supported.
+    Unimplemented = 12,
+
+    /// Internal error.
+    Internal = 13,
+
+    /// The service is currently unavailable.
+    Unavailable = 14,
+
+    /// Unrecoverable data loss or corruption.
+    DataLoss = 15,
+
+    /// The request does not have valid authentication credentials
+    Unauthenticated = 16,
+}
+
+/// If "grpc-status" can not be extracted from http response, the status "0" (Ok) is defined
+//TODO create similar but with tonic::Response<B> ? and use of [Status in tonic](https://docs.rs/tonic/latest/tonic/struct.Status.html) (more complete)
 pub fn grpc_update_span_from_response<B>(
     span: &tracing::Span,
     response: &http::Response<B>,
     is_spankind_server: bool,
 ) {
-    let status = response
-        .headers()
-        .get("grpc-status")
-        .and_then(|v| v.to_str().ok())
-        .and_then(|v| v.parse::<u16>().ok())
-        .unwrap_or(2);
+    let status = grpc_status_from_http_header(response.headers())
+        .or_else(|| grpc_status_from_http_status(response.status()))
+        .unwrap_or(GrpcCode::Ok as u16);
     span.record("rpc.grpc.status_code", status);
 
     if grpc_status_is_error(status, is_spankind_server) {
@@ -123,6 +177,36 @@ pub fn grpc_update_span_from_response<B>(
     }
 }
 
+/// based on [Status in tonic](https://docs.rs/tonic/latest/tonic/struct.Status.html#method.from_header_map)
+fn grpc_status_from_http_header(headers: &HeaderMap) -> Option<u16> {
+    headers
+        .get("grpc-status")
+        .and_then(|v| v.to_str().ok())
+        .and_then(|v| v.parse::<u16>().ok())
+}
+
+fn grpc_status_from_http_status(status_code: http::StatusCode) -> Option<u16> {
+    match status_code {
+        // Borrowed from https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md
+        http::StatusCode::BAD_REQUEST => Some(GrpcCode::Internal as u16),
+        http::StatusCode::UNAUTHORIZED => Some(GrpcCode::Unauthenticated as u16),
+        http::StatusCode::FORBIDDEN => Some(GrpcCode::PermissionDenied as u16),
+        http::StatusCode::NOT_FOUND => Some(GrpcCode::Unimplemented as u16),
+        http::StatusCode::TOO_MANY_REQUESTS
+        | http::StatusCode::BAD_GATEWAY
+        | http::StatusCode::SERVICE_UNAVAILABLE
+        | http::StatusCode::GATEWAY_TIMEOUT => Some(GrpcCode::Unavailable as u16),
+        // We got a 200 but no trailers, we can infer that this request is finished.
+        //
+        // This can happen when a streaming response sends two Status but
+        // gRPC requires that we end the stream after the first status.
+        //
+        // https://github.com/hyperium/tonic/issues/681
+        http::StatusCode::OK => None,
+        _ => Some(GrpcCode::Unknown as u16),
+    }
+}
+
 #[inline]
 #[must_use]
 /// see [Semantic Conventions for gRPC | OpenTelemetry](https://opentelemetry.io/docs/specs/semconv/rpc/grpc/)
@@ -166,4 +250,18 @@ mod tests {
         let uri: Uri = input.parse().unwrap();
         check!(url_scheme(&uri) == expected);
     }
+
+    #[rstest]
+    #[case(0)]
+    #[case(16)]
+    #[case(-1)]
+    fn test_grpc_status_from_http_header(#[case] input: i32) {
+        let mut headers = http::HeaderMap::new();
+        headers.insert("grpc-status", input.to_string().parse().unwrap());
+        if input > -1 {
+            assert_eq!(grpc_status_from_http_header(&headers), Some(input as u16));
+        } else {
+            assert_eq!(grpc_status_from_http_header(&headers), None);
+        }
+    }
 }