From c9769d6efe4475b80a3ea7ece77331a3847438d9 Mon Sep 17 00:00:00 2001 From: Bryan Naegele Date: Wed, 12 May 2021 17:10:25 -0600 Subject: [PATCH 1/2] Better comply with status compliance --- apps/opentelemetry_api/include/opentelemetry.hrl | 12 ++++++------ apps/opentelemetry_api/lib/open_telemetry.ex | 7 +++++-- apps/opentelemetry_api/lib/open_telemetry/span.ex | 2 +- apps/opentelemetry_api/lib/open_telemetry/tracer.ex | 2 +- apps/opentelemetry_api/src/opentelemetry.erl | 10 ++++++---- .../test/opentelemetry_api_SUITE.erl | 2 +- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/apps/opentelemetry_api/include/opentelemetry.hrl b/apps/opentelemetry_api/include/opentelemetry.hrl index b9c1d8b8..71baf64c 100644 --- a/apps/opentelemetry_api/include/opentelemetry.hrl +++ b/apps/opentelemetry_api/include/opentelemetry.hrl @@ -30,6 +30,10 @@ -define(SPAN_KIND_PRODUCER, producer). -define(SPAN_KIND_CONSUMER, consumer). +-define(OTEL_STATUS_UNSET, unset). +-define(OTEL_STATUS_OK, ok). +-define(OTEL_STATUS_ERROR, error). + -record(span_ctx, { %% 128 bit int trace id trace_id :: opentelemetry:trace_id(), @@ -70,11 +74,7 @@ }). -record(status, { - code :: atom() | integer(), + code = ?OTEL_STATUS_UNSET :: opentelemetry:status_code(), %% developer-facing error message - message :: unicode:unicode_binary() + message = <<"">> :: unicode:unicode_binary() }). - --define(OTEL_STATUS_UNSET, unset). --define(OTEL_STATUS_OK, ok). --define(OTEL_STATUS_ERROR, error). diff --git a/apps/opentelemetry_api/lib/open_telemetry.ex b/apps/opentelemetry_api/lib/open_telemetry.ex index 78f9d7fe..b11b74ad 100644 --- a/apps/opentelemetry_api/lib/open_telemetry.ex +++ b/apps/opentelemetry_api/lib/open_telemetry.ex @@ -111,7 +111,10 @@ defmodule OpenTelemetry do @typedoc """ An optional final status for this span. Semantically when Status - wasn't set it means span ended without errors and assume `ok`. + wasn't set it means span ended without errors and assume `unset`. + + Application developers may set the status as `ok` when the operation + has been validated to have completed successfully. """ @type status() :: :opentelemetry.status() @@ -212,6 +215,6 @@ defmodule OpenTelemetry do @doc """ Creates a Status. """ - @spec status(atom(), String.t()) :: status() + @spec status(:opentelemetry.status_code(), String.t()) :: status() defdelegate status(code, message), to: :opentelemetry end diff --git a/apps/opentelemetry_api/lib/open_telemetry/span.ex b/apps/opentelemetry_api/lib/open_telemetry/span.ex index a97ba741..da2d6841 100644 --- a/apps/opentelemetry_api/lib/open_telemetry/span.ex +++ b/apps/opentelemetry_api/lib/open_telemetry/span.ex @@ -126,7 +126,7 @@ defmodule OpenTelemetry.Span do @doc """ Sets the Status of the currently active Span. - If used, this will override the default Span Status, which is `ok`. + If used, this will override the default Span Status, which is `:unset`. """ @spec set_status(OpenTelemetry.span_ctx(), OpenTelemetry.status()) :: boolean() defdelegate set_status(span_ctx, status), to: :otel_span diff --git a/apps/opentelemetry_api/lib/open_telemetry/tracer.ex b/apps/opentelemetry_api/lib/open_telemetry/tracer.ex index c862d939..95a296de 100644 --- a/apps/opentelemetry_api/lib/open_telemetry/tracer.ex +++ b/apps/opentelemetry_api/lib/open_telemetry/tracer.ex @@ -173,7 +173,7 @@ defmodule OpenTelemetry.Tracer do @doc """ Sets the Status of the currently active Span. - If used, this will override the default Span Status, which is `ok`. + If used, this will override the default Span Status, which is `:unset`. """ @spec set_status(OpenTelemetry.status()) :: boolean() def set_status(status) do diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index 8c64927c..f49d0801 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -298,10 +298,12 @@ events(List) -> -spec status(Code, Message) -> status() | undefined when Code :: status_code(), Message :: unicode:unicode_binary(). -status(Code, Message) when is_atom(Code), - is_binary(Message) -> - #status{code=Code, - message=Message}; +status(?OTEL_STATUS_ERROR, Message) when is_binary(Message) -> + #status{code=?OTEL_STATUS_ERROR}; +status(ok, _Message) -> + #status{code=?OTEL_STATUS_OK}; +status(unset, _Message) -> + #status{code=?OTEL_STATUS_UNSET}; status(_, _) -> undefined. diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index aed0896b..004890c8 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -120,7 +120,7 @@ update_span_data(_Config) -> Events = opentelemetry:events([{opentelemetry:timestamp(), <<"timed-event-name">>, []}]), Status = opentelemetry:status(?OTEL_STATUS_OK, <<"This is Ok">>), - ?assertMatch(#status{code = ?OTEL_STATUS_OK, message = <<"This is Ok">>}, Status), + ?assertMatch(#status{code = ?OTEL_STATUS_OK, message = <<"">>}, Status), otel_span:set_status(SpanCtx1, Status), otel_span:add_events(SpanCtx1, Events), From 7ba7c5cd56d33efb094ee2ad565f5d696871ff01 Mon Sep 17 00:00:00 2001 From: Bryan Naegele Date: Thu, 13 May 2021 09:14:47 -0600 Subject: [PATCH 2/2] Make pattern match consistent, add tests, and set message on error --- apps/opentelemetry_api/src/opentelemetry.erl | 6 +++--- apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index f49d0801..392eccc9 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -299,10 +299,10 @@ events(List) -> Code :: status_code(), Message :: unicode:unicode_binary(). status(?OTEL_STATUS_ERROR, Message) when is_binary(Message) -> - #status{code=?OTEL_STATUS_ERROR}; -status(ok, _Message) -> + #status{code=?OTEL_STATUS_ERROR, message=Message}; +status(?OTEL_STATUS_OK, _Message) -> #status{code=?OTEL_STATUS_OK}; -status(unset, _Message) -> +status(?OTEL_STATUS_UNSET, _Message) -> #status{code=?OTEL_STATUS_UNSET}; status(_, _) -> undefined. diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index 004890c8..32fd158f 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -119,6 +119,12 @@ update_span_data(_Config) -> Events = opentelemetry:events([{opentelemetry:timestamp(), <<"timed-event-name">>, []}]), + ErrorStatus = opentelemetry:status(?OTEL_STATUS_ERROR, <<"This is an error!">>), + ?assertMatch(#status{code = ?OTEL_STATUS_ERROR, message = <<"This is an error!">>}, ErrorStatus), + + UnsetStatus = opentelemetry:status(?OTEL_STATUS_UNSET, <<"This is a message">>), + ?assertMatch(#status{code = ?OTEL_STATUS_UNSET, message = <<"">>}, UnsetStatus), + Status = opentelemetry:status(?OTEL_STATUS_OK, <<"This is Ok">>), ?assertMatch(#status{code = ?OTEL_STATUS_OK, message = <<"">>}, Status),