Handle binary and undefined response status and rename http.status to http.status_code in `opentelemetry-cowboy` (#48)

* Handle binary resp_status from Cowboy and rename http.status to http.status_code

* Fix test to use http.status_code as well

* Handle resp_status could be undefined but not error

- This could be due to websocket upgrade request.

* Rename Status1 and transform_status to a more concise naming

* Add test case for handling binary response code

* Fix syntax and failing tests

* Always convert cowboy status to status code

* Set otel span status as error when status code >= 500

instrumentation/opentelemetry_cowboy/src/opentelemetry_cowboy.erl

@@ -56,16 +56,24 @@ handle_event([cowboy, request, stop], Measurements, Meta, _Config) ->
                   'http.response_content_length' => maps:get(resp_body_length, Measurements)
                  },
     otel_span:set_attributes(Ctx, Attributes),
-    case Status of
+    StatusCode = transform_status_to_code(Status),
+    case StatusCode of
         undefined ->
-            {ErrorType, Error, Reason} = maps:get(error, Meta),
+            case maps:get(error, Meta, undefined) of
+              {ErrorType, Error, Reason} ->
                 otel_span:add_events(Ctx, [opentelemetry:event(ErrorType, #{error => Error, reason => Reason})]),
                 otel_span:set_status(Ctx, opentelemetry:status(?OTEL_STATUS_ERROR, Reason));
-        Status when Status >= 400 ->
+              _ ->
-            otel_span:set_attribute(Ctx, 'http.status', Status),
+                % do nothing first as I'm unsure how should we handle this
+                ok
+            end;
+        StatusCode when StatusCode >= 500 ->
+            otel_span:set_attribute(Ctx, 'http.status_code', StatusCode),
             otel_span:set_status(Ctx, opentelemetry:status(?OTEL_STATUS_ERROR, <<"">>));
-        Status when Status < 400 ->
+        StatusCode when StatusCode >= 400 ->
-            otel_span:set_attribute(Ctx, 'http.status', Status)
+            otel_span:set_attribute(Ctx, 'http.status_code', StatusCode);
+        StatusCode when StatusCode < 400 ->
+            otel_span:set_attribute(Ctx, 'http.status_code', StatusCode)
     end,
     otel_telemetry:end_telemetry_span(?TRACER_ID, Meta),
     otel_ctx:clear();
@@ -80,8 +88,9 @@ handle_event([cowboy, request, exception], Measurements, Meta, _Config) ->
      } = Meta,
     otel_span:record_exception(Ctx, Kind, Reason, Stacktrace, []),
     otel_span:set_status(Ctx, opentelemetry:status(?OTEL_STATUS_ERROR, <<"">>)),
+    StatusCode = transform_status_to_code(Status),
     otel_span:set_attributes(Ctx, #{
-                                   'http.status' => Status,
+                                   'http.status_code' => StatusCode,
                                    'http.request_content_length' => maps:get(req_body_length, Measurements),
                                    'http.response_content_length' => maps:get(resp_body_length, Measurements)
                                   }),
@@ -93,8 +102,9 @@ handle_event([cowboy, request, early_error], Measurements, Meta, _Config) ->
       reason := {ErrorType, Error, Reason},
       resp_status := Status
      } = Meta,
+    StatusCode = transform_status_to_code(Status),
     Attributes = #{
-                   'http.status' => Status,
+                   'http.status_code' => StatusCode,
                    'http.response_content_length' => maps:get(resp_body_length, Measurements)
                   },
     Ctx = otel_telemetry:start_telemetry_span(?TRACER_ID, <<"HTTP Error">>, Meta, #{attributes => Attributes}),
@@ -103,6 +113,13 @@ handle_event([cowboy, request, early_error], Measurements, Meta, _Config) ->
     otel_telemetry:end_telemetry_span(?TRACER_ID, Meta),
     otel_ctx:clear().
 
+transform_status_to_code(Status) when is_binary(Status) ->
+  [CodeString | _Message] = string:split(Status, " "),
+  {Code, _Rest} = string:to_integer(CodeString),
+  Code;
+transform_status_to_code(Status) ->
+  Status.
+
 http_flavor(Req) ->
     case maps:get(version, Req, undefined) of
         'HTTP/1.0' -> '1.0';

instrumentation/opentelemetry_cowboy/test/opentelemetry_cowboy_SUITE.erl

@@ -9,8 +9,6 @@
 -include_lib("opentelemetry/include/otel_span.hrl").
 -include_lib("opentelemetry_api/include/otel_tracer.hrl").
 
--define(assertListsMatch(List1, List2), ?assertEqual(lists:sort(List1), lists:sort(List2))).
-
 all() ->
     [
      successful_request,
@@ -19,7 +17,8 @@ all() ->
      client_timeout_request,
      idle_timeout_request,
      chunk_timeout_request,
-     bad_request
+     bad_request,
+     binary_status_code_request
      ].
 
 init_per_suite(Config) ->
@@ -30,7 +29,9 @@ init_per_suite(Config) ->
                                       {"/chunked", test_h, chunked},
                                       {"/chunked_slow", test_h, chunked_slow},
                                       {"/slow", test_h, slow},
-                                      {"/failure", test_h, failure}
+                                      {"/failure", test_h, failure},
+                                      {"/binary_status_code", test_h,
+                                       binary_status_code}
                                      ]}]),
     {ok, _} = cowboy:start_clear(http, [{port, 8080}], #{
                   env => #{dispatch => Dispatch},
@@ -84,7 +85,7 @@ successful_request(_Config) ->
                              'http.user_agent' => <<"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:81.0) Gecko/20100101 Firefox/81.0">>,
                              'net.host.ip' => <<"127.0.0.1">>,
                              'net.transport' => 'IP.TCP',
-                             'http.status' => 200,
+                             'http.status_code' => 200,
                              'http.request_content_length' => 0,
                              'http.response_content_length' => 12},
             ?assertMatch(ExpectedAttrs, otel_attributes:map(Attributes))
@@ -109,7 +110,7 @@ chunked_request(_Config) ->
                              'http.user_agent' => <<>>,
                              'net.host.ip' => <<"127.0.0.1">>,
                              'net.transport' => 'IP.TCP',
-                             'http.status' => 200,
+                             'http.status_code' => 200,
                              'http.request_content_length' => 0,
                              'http.response_content_length' => 14},
             ?assertMatch(ExpectedAttrs, otel_attributes:map(Attributes))
@@ -136,7 +137,7 @@ failed_request(_Config) ->
                              'http.user_agent' => <<>>,
                              'net.host.ip' => <<"127.0.0.1">>,
                              'net.transport' => 'IP.TCP',
-                             'http.status' => 500,
+                             'http.status_code' => 500,
                              'http.request_content_length' => 0,
                              'http.response_content_length' => 0},
             ?assertMatch(ExpectedAttrs, otel_attributes:map(Attributes))
@@ -222,7 +223,7 @@ chunk_timeout_request(_Config) ->
                              'http.user_agent' => <<>>,
                              'net.host.ip' => <<"127.0.0.1">>,
                              'net.transport' => 'IP.TCP',
-                             'http.status' => 200,
+                             'http.status_code' => 200,
                              'http.request_content_length' => 0,
                              'http.response_content_length' => 0},
             ?assertMatch(ExpectedAttrs, otel_attributes:map(Attributes))
@@ -248,9 +249,41 @@ bad_request(_Config) ->
             ?assertMatch(ExpectedEventAttrs, otel_attributes:map(EventAttributes)),
             ?assertEqual(<<"HTTP Error">>, Name),
             ExpectedAttrs = #{
-                             'http.status' => 501,
+                             'http.status_code' => 501,
                              'http.response_content_length' => 0},
             ?assertMatch(ExpectedAttrs, otel_attributes:map(Attributes))
     after
         1000 -> ct:fail(bad_request)
     end.
+
+binary_status_code_request(_Config) ->
+    Headers = [
+               {"traceparent", "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"},
+               {"tracestate", "congo=t61rcWkgMzE"},
+               {"x-forwarded-for", "203.0.133.195, 70.41.3.18, 150.172.238.178"},
+               {"user-agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:81.0) Gecko/20100101 Firefox/81.0"}],
+    {ok, {{_Version, 200, _ReasonPhrase}, _Headers, _Body}} =
+        httpc:request(get, {"http://localhost:8080/binary_status_code", Headers}, [], []),
+    receive
+        {span, #span{name=Name,attributes=Attributes,parent_span_id=ParentSpanId}} ->
+            ?assertEqual(<<"HTTP GET">>, Name),
+            ?assertEqual(13235353014750950193, ParentSpanId),
+            ExpectedAttrs = #{
+                             'http.client_ip' => <<"203.0.133.195">>,
+                             'http.flavor' => '1.1',
+                             'http.host' => <<"localhost">>,
+                             'http.host.port' => 8080,
+                             'http.method' => <<"GET">>,
+                             'http.scheme' => <<"http">>,
+                             'http.target' => <<"/binary_status_code">>,
+                             'http.user_agent' => <<"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:81.0) Gecko/20100101 Firefox/81.0">>,
+                             'net.host.ip' => <<"127.0.0.1">>,
+                             'net.transport' => 'IP.TCP',
+                             'http.status_code' => 200,
+                             'http.request_content_length' => 0,
+                             'http.response_content_length' => 12
+                            },
+            ?assertMatch(ExpectedAttrs, otel_attributes:map(Attributes))
+    after
+        1000 -> ct:fail(binary_status_code_request)
+    end.

instrumentation/opentelemetry_cowboy/test/test_h.erl

@@ -7,6 +7,8 @@ init(_, failure) ->
     error(failure);
 init(Req, success = Opts) ->
     {ok, cowboy_req:reply(200, #{}, <<"Hello world!">>, Req), Opts};
+init(Req, binary_status_code = Opts) ->
+    {ok, cowboy_req:reply(<<"200 OK">>, #{}, <<"Hello world!">>, Req), Opts};
 init(Req, slow = Opts) ->
     timer:sleep(200),
     {ok, cowboy_req:reply(200, #{}, <<"I'm slow">>, Req), Opts};