Do not set span status on 4XX errors

Author: jjatria

Changes

@@ -2,6 +2,9 @@ Revision history for Mojolicious-Plugin-OpenTelemetry
 
 {{$NEXT}}
 
+    * Fixed an issue where spans were set to error in the case
+      of 4XX responses, which goes agains the specification.
+
 0.001     2023-12-03 16:51:34+00:00 Europe/London
 
     First version.

lib/Mojolicious/Plugin/OpenTelemetry.pm

@@ -80,10 +80,18 @@ sub register ( $, $app, $config, @ ) {
 
             $promise->then( sub {
                 my $code  = $tx->res->code;
-                my $error = $code >= 400 && $code < 600;
+
+                # The status of server spans must be left unset if the
+                # response is a 4XX error
+                # See https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md#status
+                if ( $code < 400 ) {
+                    $span->set_status(SPAN_STATUS_OK);
+                }
+                elsif ( $code >= 500 ) {
+                    $span->set_status(SPAN_STATUS_ERROR);
+                }
 
                 $span
-                    ->set_status( $error ? SPAN_STATUS_ERROR : SPAN_STATUS_OK )
                     ->set_attribute( 'http.response.status_code' => $code )
                     ->end;
             })->wait;

t/basic.t

@@ -64,6 +64,10 @@ get '/async' => sub ( $c, @ ) {
     });
 };
 
+get '/status/:code' => sub ( $c, @ ) {
+    $c->render( text => 'OK', status => $c->stash('code') );
+};
+
 get '/error' => sub ( $c, @ ) {
     die 'oops';
 };
@@ -206,6 +210,37 @@ subtest Error => sub {
     ];
 };
 
+subtest 'Response codes' => sub {
+    $tst->get_ok('/status/400')
+        ->status_is(400);
+
+    is $span->{otel}, {
+        attributes => {
+            'client.address'           => '127.0.0.1',
+            'client.port'              => T,
+            'http.request.method'      => 'GET',
+            'http.route'               => '/status/:code',
+            'network.protocol.version' => '1.1',
+            'server.address'           => '127.0.0.1',
+            'server.port'              => T,
+            'url.path'                 => '/status/400',
+            'url.scheme'               => U,
+            'user_agent.original'      => 'Mojolicious (Perl)',
+        },
+        kind => SPAN_KIND_SERVER,
+        name => 'GET /status/:code',
+        parent => D, # FIXME: cannot use an object check on 5.32?
+      # parent => object {
+      #     prop isa => 'OpenTelemetry::Context';
+      # },
+    };
+
+    span_calls [
+        set_attribute    => [ 'http.response.status_code', 400 ],
+        end              => [],
+    ];
+};
+
 describe 'Host / port parsing' => sub {
     my $port;