From ffadbbcca3e988ed4571d17662f78e8936a410bc Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 15 Feb 2022 06:06:44 -0700 Subject: [PATCH 01/13] add set_status that takes a code and message --- apps/opentelemetry/src/otel_span_ets.erl | 6 ++++-- apps/opentelemetry/test/opentelemetry_SUITE.erl | 9 ++++++--- apps/opentelemetry_api/include/otel_tracer.hrl | 3 +++ apps/opentelemetry_api/lib/open_telemetry.ex | 2 ++ apps/opentelemetry_api/lib/open_telemetry/tracer.ex | 10 ++++++++++ apps/opentelemetry_api/src/otel_span.erl | 8 ++++++++ .../opentelemetry_api/test/opentelemetry_api_SUITE.erl | 1 + 7 files changed, 34 insertions(+), 5 deletions(-) diff --git a/apps/opentelemetry/src/otel_span_ets.erl b/apps/opentelemetry/src/otel_span_ets.erl index f5490fa2..d424cc7b 100644 --- a/apps/opentelemetry/src/otel_span_ets.erl +++ b/apps/opentelemetry/src/otel_span_ets.erl @@ -136,8 +136,10 @@ add_events(#span_ctx{span_id=SpanId}, NewEvents) -> end. -spec set_status(opentelemetry:span_ctx(), opentelemetry:status()) -> boolean(). -set_status(#span_ctx{span_id=SpanId}, Status) -> - ets:update_element(?SPAN_TAB, SpanId, {#span.status, Status}). +set_status(#span_ctx{span_id=SpanId}, Status) when is_record(Status, status) -> + ets:update_element(?SPAN_TAB, SpanId, {#span.status, Status}); +set_status(_, _) -> + false. -spec update_name(opentelemetry:span_ctx(), opentelemetry:span_name()) -> boolean(). update_name(#span_ctx{span_id=SpanId}, Name) -> diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 524a5227..af69f8d0 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -373,15 +373,18 @@ update_span_data(Config) -> tracestate=[]}], SpanCtx1=#span_ctx{trace_id=TraceId, - span_id=SpanId} = ?start_span(<<"span-1">>, #{links => Links}), + span_id=SpanId, + is_recording=true} = ?start_span(<<"span-1">>, #{links => Links}), ?set_current_span(SpanCtx1), ?set_attribute(<<"key-1">>, <<"value-1">>), Events = opentelemetry:events([{opentelemetry:timestamp(), <<"event-name">>, []}]), - Status = opentelemetry:status(0, <<"status">>), + Status = opentelemetry:status(?OTEL_STATUS_ERROR, <<"status">>), - otel_span:set_status(SpanCtx1, Status), + ?assert(otel_span:set_status(SpanCtx1, Status)), + %% returns false if called with something that isn't a status record + ?assertNot(otel_span:set_status(SpanCtx1, notastatus)), %% returning not false means it successfully called the SDK ?assertNotEqual(false, otel_span:add_event(SpanCtx1, event_1, #{<<"attr-1">> => <<"attr-value-1">>})), diff --git a/apps/opentelemetry_api/include/otel_tracer.hrl b/apps/opentelemetry_api/include/otel_tracer.hrl index 0693c4ee..cee2f4cf 100644 --- a/apps/opentelemetry_api/include/otel_tracer.hrl +++ b/apps/opentelemetry_api/include/otel_tracer.hrl @@ -42,6 +42,9 @@ -define(add_events(Events), otel_span:add_events(?current_span_ctx, Events)). +-define(set_status(Code, Message), + otel_span:set_status(?current_span_ctx, Code, Message)). + -define(set_status(Status), otel_span:set_status(?current_span_ctx, Status)). diff --git a/apps/opentelemetry_api/lib/open_telemetry.ex b/apps/opentelemetry_api/lib/open_telemetry.ex index 00615bdb..0e7d8c8d 100644 --- a/apps/opentelemetry_api/lib/open_telemetry.ex +++ b/apps/opentelemetry_api/lib/open_telemetry.ex @@ -105,6 +105,8 @@ defmodule OpenTelemetry do """ @type status() :: :opentelemetry.status() + @type status_code() :: :opentelemetry.status_code() + defdelegate get_tracer(name), to: :opentelemetry defdelegate get_tracer(name, vsn, schema_url), to: :opentelemetry defdelegate set_default_tracer(t), to: :opentelemetry diff --git a/apps/opentelemetry_api/lib/open_telemetry/tracer.ex b/apps/opentelemetry_api/lib/open_telemetry/tracer.ex index ae38a1c0..fa398994 100644 --- a/apps/opentelemetry_api/lib/open_telemetry/tracer.ex +++ b/apps/opentelemetry_api/lib/open_telemetry/tracer.ex @@ -167,6 +167,16 @@ defmodule OpenTelemetry.Tracer do :otel_span.add_events(:otel_tracer.current_span_ctx(), events) end + @doc """ + Creates and sets the Status of the currently active Span. + + If used, this will override the default Span Status, which is `:unset`. + """ + @spec set_status(OpenTelemetry.status_code(), String.t()) :: boolean() + def set_status(code, message) do + :otel_span.set_status(:otel_tracer.current_span_ctx(), code, message) + end + @doc """ Sets the Status of the currently active Span. diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index 12ef9f79..d50c0df7 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -35,6 +35,7 @@ record_exception/5, record_exception/6, set_status/2, + set_status/3, update_name/2, end_span/1, end_span/2]). @@ -259,6 +260,13 @@ set_status(SpanCtx=#span_ctx{span_sdk={Module, _}}, Status) when ?is_recording(S set_status(_, _) -> false. +-spec set_status(SpanCtx, Code, Message) -> boolean() when + Code :: opentelemetry:status_code(), + Message :: unicode:unicode_binary(), + SpanCtx :: opentelemetry:span_ctx(). +set_status(SpanCtx, Code, Message) -> + set_status(SpanCtx, opentelemetry:status(Code, Message)). + -spec update_name(SpanCtx, Name) -> boolean() when Name :: opentelemetry:span_name(), SpanCtx :: opentelemetry:span_ctx(). diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index 3b7d0eb2..7abc7f22 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -212,6 +212,7 @@ update_span_data(_Config) -> ?assertMatch(#status{code = ?OTEL_STATUS_OK, message = <<"">>}, Status), otel_span:set_status(SpanCtx1, Status), + otel_span:set_status(SpanCtx1, ?OTEL_STATUS_ERROR, <<"this is not ok">>), otel_span:add_events(SpanCtx1, Events), ?assertMatch(SpanCtx1, ?current_span_ctx), From 1aa86dfd0fbe38acb8fb82ea4d9c404b59388b87 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 15 Feb 2022 14:36:59 -0700 Subject: [PATCH 02/13] add API macro docs to the README --- apps/opentelemetry_api/README.md | 164 ++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 3 deletions(-) diff --git a/apps/opentelemetry_api/README.md b/apps/opentelemetry_api/README.md index 713e72e2..3828bce0 100644 --- a/apps/opentelemetry_api/README.md +++ b/apps/opentelemetry_api/README.md @@ -30,17 +30,175 @@ some_fun() -> ``` elixir require OpenTelemetry.Tracer -require OpenTelemetry.Span def some_fun() do OpenTelemetry.Tracer.with_span "some-span" do ... - OpenTelemetry.Span.set_attribute("key", "value") + OpenTelemetry.Tracer.set_attribute("key", "value") ... end end ``` +### Tracing API + +The macros and functions available for Elixir in `OpenTelemetry.Tracer` and the +Erlang macros in `otel_tracer.hrl` are the best way to work with Spans. They +will automatically use the Tracer named for the Application the module using the +macro is in. For example, the Spans created in +[opentelemetry_oban](https://hex.pm/packages/opentelemetry_oban) use the +`with_span` macro resulting in the Span being created with the +`opentelemetry_oban` named Tracer and associated with the [Instrumentation +Library](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/glossary.md#instrumentation-library) +of the same name and version of the Tracer -- the version also matches the +`opentelemetry_oban` Application version. + +#### Context + +[Context](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/context/context.md) is used to pass values associated with the current [execution +unit](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/glossary.md#execution-unit). +At this time the only values kept in the Context by this OpenTelemetry library +are the [Span +Context](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#spancontext) +for the currently active Span and the +[Baggage](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/baggage/api.md) + +When a Context variable is not an explicit argument in the API macros or +functions the Context from the [process +dictionary](https://www.erlang.org/doc/reference_manual/processes.html#process-dictionary) +is used. If no Context is found in the current process's pdict then one is +created. + +#### Starting and Ending Spans + +A Span represents a single operation in a Trace. It has a start and end time, +can have a single parent and one or more children. The easiest way to create +Spans is to wrap the operation you want a Span to represent in the `with_span` +macro. The macro handles getting a +[Tracer](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#tracer) +associated with the OTP Application the module is in, starting the Span, setting +it as the currently active Span in the Context stored in the process dictionary +and ending the Span when the `Fun` or body of the Elixir macro finish, even if +an exception is thrown. After the Span is end'ed the Context in the process +dictionary is reset to its value before the newly started Span was set as the +active Span. + +``` erlang +?with_span(SpanName, StartOpts, Fun) +``` + +``` elixir +OpenTelemetry.Tracer.with_span name, start_opts do + ... +end +``` + +`StartOpts`/`start_opts` is a map of [Span creation options](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#span-creation): + +- `kind`: +[SpanKind](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#spankind) +defines the relationship between the Span, its parents, and its children in a +Trace. Possible values: `internal`, `server`, `client`, `producer` and +`consumer`. +- `attributes`: [Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/common/common.md#attributes) +- `links`: List of [Links](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/overview.md#links-between-spans) to causally related Spans from the same or a different Trace. +- `start_time`: The start time of the Span operation. Defaults to the current + time. The option should only be set if the start of the operation described by + the Span has already passed. + +current_span_ctx(ctx) + +set_current_span(span_ctx) + +When using `start_span` instead of `with_span` there must be a corresponding +call to the [end Span +API](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#end) +to signal that the operation described by the Span has ended. `end_span` +optionally takes a timestamp to use as the end time of the Span. + +``` erlang +?end_span() +?end_span(Timestamp) +``` + +``` elixir +OpenTelemetry.Tracer.end_span(timestamp \\ :undefined) +``` + +#### Setting Attributes + +[Setting +Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#set-attributes) +can be done with a single key and value passed to `set_attribute` or through a +map of +[Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/common/common.md#attributes) +all at once. Setting an attribute with a key that already exists in the Span's +map of attributes will result in that key's value being overwritten. + +``` erlang +?set_attribute(Key, Value) +?set_attributes(Attributes) +``` + +``` elixir +OpenTelemetry.Tracer.set_attribute(key, value) +OpenTelemetry.Tracer.set_attributes(attributes) +``` + +Be aware that there are [configurable limits](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/common/common.md#attribute-limits) on the number and size of +Attributes per Span. + +#### Adding Events + +[Adding +Events](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#add-events) +can be done by passing the name of the event and the +[Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/common/common.md#attributes) +to associate with it or as a list of Events. Each Event in the list of Events is +a map containing the timestamp, name, and Attributes which can be created with +the function `event/2` and `event/3` in the `opentelemetry` and `OpenTelemetry` +modules. + +``` erlang +?add_event(Name, Attributes) +?add_events(Events) +``` + +``` elixir +OpenTelemetry.Tracer.add_event(event, attributes) +OpenTelemetry.Tracer.add_events(events) +``` + +#### Setting the Status + +[Set +Status](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#set-status) +will override the default Span Status of `Unset`. A Status is a code (`ok`, +`error` or `unset`) and, only if the code is `error`, an optional message string +that describes the error. + +``` erlang +?set_status(Code, Message) +``` + +``` elixir +OpenTelemetry.Tracer.set_status(code, message) +``` + +#### Update Span Name + +[Updating the Span +name](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#updatename) +can be done after starting the Span but must be done before the Span is end'ed. + +``` erlang +?update_name(Name) +``` + +``` elixir +OpenTelemetry.Tracer.update_name(name) +``` + ### Including the OpenTelemetry SDK When only the API is available at runtime a no-op Tracer is used and no Traces @@ -54,7 +212,7 @@ not as a dependency of any individual Application. Included in the same [Github repo](https://github.com/open-telemetry/opentelemetry-erlang) as the API and SDK are an exporter for the [OpenTelemetry Protocol -(OTLP)](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md) +(OTLP)](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/protocol/otlp.md) and [Zipkin](https://zipkin.io/): - [OpenTelemetry Protocol](https://hex.pm/packages/opentelemetry_exporter) From b1694036f3cee7ad1aa54335dece13ba54f8773f Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 15 Feb 2022 16:58:16 -0700 Subject: [PATCH 03/13] exclude experimental apps from coverage --- rebar.config | 1 + 1 file changed, 1 insertion(+) diff --git a/rebar.config b/rebar.config index 8f7c1ca5..2b84bb34 100644 --- a/rebar.config +++ b/rebar.config @@ -33,3 +33,4 @@ {cover_enabled, true}. {cover_export_enabled, true}. {covertool, [{coverdata_files, ["ct.coverdata"]}]}. +{cover_excl_apps, [opentelemetry_api_experimental, opentelemetry_experimental]}. From f90f799ae63c9f5fbec04b7c957dc665435dff55 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 16 Feb 2022 15:47:13 -0700 Subject: [PATCH 04/13] api readme: make clear exceptions are not caught in with_span --- apps/opentelemetry_api/README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/opentelemetry_api/README.md b/apps/opentelemetry_api/README.md index 3828bce0..c0cd245e 100644 --- a/apps/opentelemetry_api/README.md +++ b/apps/opentelemetry_api/README.md @@ -79,9 +79,10 @@ macro. The macro handles getting a associated with the OTP Application the module is in, starting the Span, setting it as the currently active Span in the Context stored in the process dictionary and ending the Span when the `Fun` or body of the Elixir macro finish, even if -an exception is thrown. After the Span is end'ed the Context in the process -dictionary is reset to its value before the newly started Span was set as the -active Span. +an exception is thrown -- however, the exception is not caught, so it does not +change how user code should deal with raised exceptions. After the Span is +end'ed the Context in the process dictionary is reset to its value before the +newly started Span was set as the active Span. ``` erlang ?with_span(SpanName, StartOpts, Fun) From 1a8b77c8bed437f0f630f4e5e44c55a2c79e282f Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 16 Feb 2022 19:49:12 -0700 Subject: [PATCH 05/13] api: support for just status code in set_status api --- .../test/opentelemetry_SUITE.erl | 21 +++++++++++++++---- .../opentelemetry_api/include/otel_tracer.hrl | 4 ++-- apps/opentelemetry_api/lib/open_telemetry.ex | 6 ++++++ apps/opentelemetry_api/src/opentelemetry.erl | 6 ++++++ apps/opentelemetry_api/src/otel_span.erl | 5 +++++ .../test/open_telemetry_test.exs | 14 +++++++++++++ .../test/opentelemetry_api_SUITE.erl | 3 +++ 7 files changed, 53 insertions(+), 6 deletions(-) diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index af69f8d0..d07b6e51 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -380,9 +380,22 @@ update_span_data(Config) -> Events = opentelemetry:events([{opentelemetry:timestamp(), <<"event-name">>, []}]), - Status = opentelemetry:status(?OTEL_STATUS_ERROR, <<"status">>), - - ?assert(otel_span:set_status(SpanCtx1, Status)), + ErrorStatus = opentelemetry:status(?OTEL_STATUS_ERROR, <<"status">>), + ?assertMatch(#status{code=?OTEL_STATUS_ERROR, + message = <<"status">>}, ErrorStatus), + + OkStatus = opentelemetry:status(?OTEL_STATUS_OK, <<"will be ignored">>), + ?assertMatch(#status{code=?OTEL_STATUS_OK, + message = <<>>}, OkStatus), + ?assertEqual(OkStatus, opentelemetry:status(?OTEL_STATUS_OK)), + + UnsetStatus = opentelemetry:status(?OTEL_STATUS_UNSET, <<"will be ignored">>), + ?assertMatch(#status{code=?OTEL_STATUS_UNSET, + message = <<>>}, UnsetStatus), + ?assertEqual(UnsetStatus, opentelemetry:status(?OTEL_STATUS_UNSET)), + + ?assert(otel_span:set_status(SpanCtx1, ErrorStatus)), + ?assertNot(otel_span:set_status(SpanCtx1, ?OTEL_STATUS_ERROR)), %% returns false if called with something that isn't a status record ?assertNot(otel_span:set_status(SpanCtx1, notastatus)), @@ -397,7 +410,7 @@ update_span_data(Config) -> links=L, events=E}] = ?UNTIL_NOT_EQUAL([], ets:match_object(Tid, #span{trace_id=TraceId, span_id=SpanId, - status=Status, + status=ErrorStatus, _='_'})), diff --git a/apps/opentelemetry_api/include/otel_tracer.hrl b/apps/opentelemetry_api/include/otel_tracer.hrl index cee2f4cf..936a46e4 100644 --- a/apps/opentelemetry_api/include/otel_tracer.hrl +++ b/apps/opentelemetry_api/include/otel_tracer.hrl @@ -45,8 +45,8 @@ -define(set_status(Code, Message), otel_span:set_status(?current_span_ctx, Code, Message)). --define(set_status(Status), - otel_span:set_status(?current_span_ctx, Status)). +-define(set_status(StatusOrCode), + otel_span:set_status(?current_span_ctx, StatusOrCode)). -define(update_name(Name), otel_span:update_name(?current_span_ctx, Name)). diff --git a/apps/opentelemetry_api/lib/open_telemetry.ex b/apps/opentelemetry_api/lib/open_telemetry.ex index 0e7d8c8d..fa5c8e82 100644 --- a/apps/opentelemetry_api/lib/open_telemetry.ex +++ b/apps/opentelemetry_api/lib/open_telemetry.ex @@ -190,6 +190,12 @@ defmodule OpenTelemetry do @spec events(list()) :: [event()] defdelegate events(event_list), to: :opentelemetry + @doc """ + Creates a Status with an empty description. + """ + @spec status(:opentelemetry.status_code()) :: status() + defdelegate status(code), to: :opentelemetry + @doc """ Creates a Status. """ diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index 185b6f9b..f81952b3 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -52,6 +52,7 @@ event/2, event/3, events/1, + status/1, status/2, verify_and_set_term/3]). @@ -381,6 +382,11 @@ events(List) -> false end, List). +-spec status(Code) -> status() | undefined when + Code :: status_code(). +status(Code) -> + status(Code, <<>>). + -spec status(Code, Message) -> status() | undefined when Code :: status_code(), Message :: unicode:unicode_binary(). diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index d50c0df7..4caa48f8 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -257,6 +257,11 @@ record_exception(SpanCtx, Class, Term, Message, Stacktrace, Attributes) -> SpanCtx :: opentelemetry:span_ctx(). set_status(SpanCtx=#span_ctx{span_sdk={Module, _}}, Status) when ?is_recording(SpanCtx) -> Module:set_status(SpanCtx, Status); +set_status(SpanCtx=#span_ctx{span_sdk={Module, _}}, Code) when ?is_recording(SpanCtx) andalso + (Code =:= ?OTEL_STATUS_UNSET orelse + Code =:= ?OTEL_STATUS_OK orelse + Code =:= ?OTEL_STATUS_ERROR)-> + Module:set_status(SpanCtx, opentelemetry:status(Code)); set_status(_, _) -> false. diff --git a/apps/opentelemetry_api/test/open_telemetry_test.exs b/apps/opentelemetry_api/test/open_telemetry_test.exs index af966868..0a24eb6a 100644 --- a/apps/opentelemetry_api/test/open_telemetry_test.exs +++ b/apps/opentelemetry_api/test/open_telemetry_test.exs @@ -10,6 +10,9 @@ defmodule OpenTelemetryTest do @fields Record.extract(:span_ctx, from_lib: "opentelemetry_api/include/opentelemetry.hrl") Record.defrecordp(:span_ctx, @fields) + @fields Record.extract(:status, from_lib: "opentelemetry_api/include/opentelemetry.hrl") + Record.defrecordp(:status, @fields) + test "current_span tracks last set_span" do span_ctx1 = Tracer.start_span("span-1") assert :undefined == Tracer.current_span_ctx() @@ -48,6 +51,17 @@ defmodule OpenTelemetryTest do event2 = OpenTelemetry.event("event-2", []) Tracer.add_events([event1, event2]) + + error_status = OpenTelemetry.status(:error, "This is an error!") + assert status(code: :error, message: "This is an error!") = error_status + unset_status = OpenTelemetry.status(:unset, "This is ignored") + assert status(code: :unset, message: "") = unset_status + + ok_status = OpenTelemetry.status(:ok, "This is ignored") + assert status(code: :ok, message: "") = ok_status + + Tracer.set_status(error_status) + Tracer.set_status(:error, "this is not ok") end end end diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index 7abc7f22..f1412fd4 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -205,10 +205,13 @@ update_span_data(_Config) -> ErrorStatus = opentelemetry:status(?OTEL_STATUS_ERROR, <<"This is an error!">>), ?assertMatch(#status{code = ?OTEL_STATUS_ERROR, message = <<"This is an error!">>}, ErrorStatus), + %% for unset and ok status status/2 and status/1 should return the same record UnsetStatus = opentelemetry:status(?OTEL_STATUS_UNSET, <<"This is a message">>), + UnsetStatus = opentelemetry:status(?OTEL_STATUS_UNSET), ?assertMatch(#status{code = ?OTEL_STATUS_UNSET, message = <<"">>}, UnsetStatus), Status = opentelemetry:status(?OTEL_STATUS_OK, <<"This is Ok">>), + Status = opentelemetry:status(?OTEL_STATUS_OK), ?assertMatch(#status{code = ?OTEL_STATUS_OK, message = <<"">>}, Status), otel_span:set_status(SpanCtx1, Status), From 7fb6167e130bb85af88aebb52a9fb929c018c31f Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 17 Feb 2022 10:23:50 -0700 Subject: [PATCH 06/13] set_status: follow spec which defines precedence of values If set_status has already been called and set to OK then another set_status can not change the value after that. If it was set to ERROR then it can only updated to OK. --- apps/opentelemetry/src/otel_span_ets.erl | 19 +++++++++++++++++-- .../test/opentelemetry_SUITE.erl | 8 +++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/apps/opentelemetry/src/otel_span_ets.erl b/apps/opentelemetry/src/otel_span_ets.erl index d424cc7b..f688e54c 100644 --- a/apps/opentelemetry/src/otel_span_ets.erl +++ b/apps/opentelemetry/src/otel_span_ets.erl @@ -39,6 +39,7 @@ -include_lib("opentelemetry_api/include/opentelemetry.hrl"). -include("otel_span.hrl"). -include("otel_span_ets.hrl"). +-include_lib("stdlib/include/ms_transform.hrl"). -record(state, {}). @@ -136,8 +137,22 @@ add_events(#span_ctx{span_id=SpanId}, NewEvents) -> end. -spec set_status(opentelemetry:span_ctx(), opentelemetry:status()) -> boolean(). -set_status(#span_ctx{span_id=SpanId}, Status) when is_record(Status, status) -> - ets:update_element(?SPAN_TAB, SpanId, {#span.status, Status}); +set_status(#span_ctx{span_id=SpanId}, Status=#status{code=NewCode}) -> + MS = ets:fun2ms(fun(Span=#span{span_id=Id, + status=#status{code=?OTEL_STATUS_ERROR}}) when Id =:= SpanId , + NewCode =:= ?OTEL_STATUS_OK -> + %% can only set status to OK if it has been set to ERROR before + Span#span{status=#status{code=?OTEL_STATUS_OK}}; + (Span=#span{span_id=Id, + status=#status{code=?OTEL_STATUS_UNSET}}) when Id =:= SpanId -> + %% if UNSET then the status can be updated to OK or ERROR + Span#span{status=Status}; + (Span=#span{span_id=Id, + status=undefined}) when Id =:= SpanId -> + %% if undefined then the status can be updated to anything + Span#span{status=Status} + end), + ets:select_replace(?SPAN_TAB, MS) =:= 1; set_status(_, _) -> false. diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index d07b6e51..7a2bb086 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -394,9 +394,11 @@ update_span_data(Config) -> message = <<>>}, UnsetStatus), ?assertEqual(UnsetStatus, opentelemetry:status(?OTEL_STATUS_UNSET)), - ?assert(otel_span:set_status(SpanCtx1, ErrorStatus)), + ?assert(otel_span:set_status(SpanCtx1, OkStatus)), + %% spec does not allow setting status to error/unset after it is ok + ?assertNot(otel_span:set_status(SpanCtx1, ErrorStatus)), ?assertNot(otel_span:set_status(SpanCtx1, ?OTEL_STATUS_ERROR)), - %% returns false if called with something that isn't a status record + %% %% returns false if called with something that isn't a status record ?assertNot(otel_span:set_status(SpanCtx1, notastatus)), %% returning not false means it successfully called the SDK @@ -410,7 +412,7 @@ update_span_data(Config) -> links=L, events=E}] = ?UNTIL_NOT_EQUAL([], ets:match_object(Tid, #span{trace_id=TraceId, span_id=SpanId, - status=ErrorStatus, + status=OkStatus, _='_'})), From a9e16f6080210e40965660d1a3593a0ce120d843 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 17 Feb 2022 15:21:04 -0700 Subject: [PATCH 07/13] update changelog with entry about span status fix --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d600e250..1306b1d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [Simpler configuration of span processors](https://github.com/open-telemetry/opentelemetry-erlang/pull/357) +#### Fixed + +- Span Status: Ignore status changes that don't follow the [define precedence in + the spec](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#set-status) + ### [Zipkin Exporter] #### Fixed From a809e995728e8449e5c77e46cf345ddb79831da8 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 21 Feb 2022 14:40:18 -0700 Subject: [PATCH 08/13] Update apps/opentelemetry_api/README.md Co-authored-by: Fred Hebert --- apps/opentelemetry_api/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/opentelemetry_api/README.md b/apps/opentelemetry_api/README.md index c0cd245e..106924f9 100644 --- a/apps/opentelemetry_api/README.md +++ b/apps/opentelemetry_api/README.md @@ -81,7 +81,7 @@ it as the currently active Span in the Context stored in the process dictionary and ending the Span when the `Fun` or body of the Elixir macro finish, even if an exception is thrown -- however, the exception is not caught, so it does not change how user code should deal with raised exceptions. After the Span is -end'ed the Context in the process dictionary is reset to its value before the +ended the Context in the process dictionary is reset to its value before the newly started Span was set as the active Span. ``` erlang From 4035a43c181c25205f29ee3266d5433771969f52 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 17 Feb 2022 17:27:12 -0700 Subject: [PATCH 09/13] elixir api: remove unused deps and mix.exs code --- apps/opentelemetry_api/mix.exs | 50 ++++----------------------------- apps/opentelemetry_api/mix.lock | 11 ++++---- 2 files changed, 11 insertions(+), 50 deletions(-) diff --git a/apps/opentelemetry_api/mix.exs b/apps/opentelemetry_api/mix.exs index bbd150fb..281e2e08 100644 --- a/apps/opentelemetry_api/mix.exs +++ b/apps/opentelemetry_api/mix.exs @@ -3,59 +3,25 @@ defmodule OpenTelemetry.MixProject do def project do {app, desc} = load_app() - config = load_config() [ app: app, - version: version(Keyword.fetch!(desc, :vsn)), + version: to_string(Keyword.fetch!(desc, :vsn)), description: to_string(Keyword.fetch!(desc, :description)), elixir: "~> 1.8", start_permanent: Mix.env() == :prod, - # We should never have dependencies - deps: deps(Keyword.fetch!(config, :deps)), - # Docs + deps: [ + {:dialyxir, "~> 1.0", only: [:dev], runtime: false}, + {:covertool, ">= 0.0.0", only: :test} + ], name: "OpenTelemetry API", - # source_url: "https://github.com/USER/PROJECT", - # homepage_url: "http://YOUR_PROJECT_HOMEPAGE", test_coverage: [tool: :covertool], - docs: [ - main: "readme" - # logo: "path/to/logo.png", - ], - aliases: [ - # when build docs first build edocs with rebar3 - docs: ["cmd rebar3 edoc", "docs"] - ], package: package() ] end - defp version(version) when is_list(version) do - List.to_string(version) - end - - defp version({:file, path}) do - path - |> File.read!() - |> String.trim() - end - - # Run "mix help compile.app" to learn about applications. def application, do: [] - defp deps(rebar) do - rebar - |> Enum.map(fn - {dep, version} -> {dep, to_string(version)} - dep when is_atom(dep) -> {dep, ">= 0.0.0"} - end) - |> Enum.concat([ - {:ex_doc, "0.21.0", only: :dev, runtime: false}, - {:dialyxir, "~> 1.0", only: [:dev], runtime: false}, - {:covertool, ">= 0.0.0", only: :test} - ]) - end - defp package() do [ description: "OpenTelemetry API", @@ -69,12 +35,6 @@ defmodule OpenTelemetry.MixProject do ] end - defp load_config do - {:ok, config} = :file.consult('rebar.config') - - config - end - defp load_app do {:ok, [{:application, name, desc}]} = :file.consult('src/opentelemetry_api.app.src') diff --git a/apps/opentelemetry_api/mix.lock b/apps/opentelemetry_api/mix.lock index d8004ddb..dac90e81 100644 --- a/apps/opentelemetry_api/mix.lock +++ b/apps/opentelemetry_api/mix.lock @@ -3,11 +3,12 @@ "covertool": {:hex, :covertool, "2.0.3", "5d1ca6958482b9b7e718daf61f398e382426ed0f4689d5c8698a60ae3b5ba521", [:rebar3], [], "hexpm", "5c13170a55dbd6bd9efc722bc7fa32caff6f3c9cde9c692bd4a88bfc9ac4f029"}, "dialyxir": {:hex, :dialyxir, "1.1.0", "c5aab0d6e71e5522e77beff7ba9e08f8e02bad90dfbeffae60eaf0cb47e29488", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "07ea8e49c45f15264ebe6d5b93799d4dd56a44036cf42d0ad9c960bc266c0b9a"}, "earmark": {:hex, :earmark, "1.4.14", "d04572cef64dd92726a97d92d714e38d6e130b024ea1b3f8a56e7de66ec04e50", [:mix], [{:earmark_parser, ">= 1.4.12", [hex: :earmark_parser, repo: "hexpm", optional: false]}], "hexpm", "df338b8b1852ee425180b276c56c6941cb12220e04fe8718fe4acbdd35fd699f"}, - "earmark_parser": {:hex, :earmark_parser, "1.4.12", "b245e875ec0a311a342320da0551da407d9d2b65d98f7a9597ae078615af3449", [:mix], [], "hexpm", "711e2cc4d64abb7d566d43f54b78f7dc129308a63bc103fbd88550d2174b3160"}, + "earmark_parser": {:hex, :earmark_parser, "1.4.19", "de0d033d5ff9fc396a24eadc2fcf2afa3d120841eb3f1004d138cbf9273210e8", [:mix], [], "hexpm", "527ab6630b5c75c3a3960b75844c314ec305c76d9899bb30f71cb85952a9dc45"}, "elixir_make": {:hex, :elixir_make, "0.6.2", "7dffacd77dec4c37b39af867cedaabb0b59f6a871f89722c25b28fcd4bd70530", [:mix], [], "hexpm", "03e49eadda22526a7e5279d53321d1cced6552f344ba4e03e619063de75348d9"}, "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"}, - "ex_doc": {:hex, :ex_doc, "0.21.0", "7af8cd3e3df2fe355e99dabd2d4dcecc6e76eb417200e3b7a3da362d52730e3c", [:mix], [{:earmark, "~> 1.3", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm", "ef679a81de63385c7e72597e81ca1276187505eeacb38281a672d2822254ff1a"}, - "makeup": {:hex, :makeup, "1.0.5", "d5a830bc42c9800ce07dd97fa94669dfb93d3bf5fcf6ea7a0c67b2e0e4a7f26c", [:mix], [{:nimble_parsec, "~> 0.5 or ~> 1.0", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cfa158c02d3f5c0c665d0af11512fed3fba0144cf1aadee0f2ce17747fba2ca9"}, - "makeup_elixir": {:hex, :makeup_elixir, "0.15.1", "b5888c880d17d1cc3e598f05cdb5b5a91b7b17ac4eaf5f297cb697663a1094dd", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.1", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "db68c173234b07ab2a07f645a5acdc117b9f99d69ebf521821d89690ae6c6ec8"}, - "nimble_parsec": {:hex, :nimble_parsec, "1.1.0", "3a6fca1550363552e54c216debb6a9e95bd8d32348938e13de5eda962c0d7f89", [:mix], [], "hexpm", "08eb32d66b706e913ff748f11694b17981c0b04a33ef470e33e11b3d3ac8f54b"}, + "ex_doc": {:hex, :ex_doc, "0.28.0", "7eaf526dd8c80ae8c04d52ac8801594426ae322b52a6156cd038f30bafa8226f", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "e55cdadf69a5d1f4cfd8477122ebac5e1fadd433a8c1022dafc5025e48db0131"}, + "makeup": {:hex, :makeup, "1.1.0", "6b67c8bc2882a6b6a445859952a602afc1a41c2e08379ca057c0f525366fc3ca", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "0a45ed501f4a8897f580eabf99a2e5234ea3e75a4373c8a52824f6e873be57a6"}, + "makeup_elixir": {:hex, :makeup_elixir, "0.15.2", "dc72dfe17eb240552857465cc00cce390960d9a0c055c4ccd38b70629227e97c", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.1", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "fd23ae48d09b32eff49d4ced2b43c9f086d402ee4fd4fcb2d7fad97fa8823e75"}, + "makeup_erlang": {:hex, :makeup_erlang, "0.1.1", "3fcb7f09eb9d98dc4d208f49cc955a34218fc41ff6b84df7c75b3e6e533cc65f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "174d0809e98a4ef0b3309256cbf97101c6ec01c4ab0b23e926a9e17df2077cbb"}, + "nimble_parsec": {:hex, :nimble_parsec, "1.2.2", "b99ca56bbce410e9d5ee4f9155a212e942e224e259c7ebbf8f2c86ac21d4fa3c", [:mix], [], "hexpm", "98d51bd64d5f6a2a9c6bb7586ee8129e27dfaab1140b5a4753f24dac0ba27d2f"}, } From 705133db6184830f62893e0b5bf1460c26727468 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 17 Feb 2022 17:28:17 -0700 Subject: [PATCH 10/13] use --no-start to fix tls_certificate_check warning in mix test --- .github/workflows/elixir.yml | 2 +- test/test_helper.exs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/elixir.yml b/.github/workflows/elixir.yml index ff153043..e8469c03 100644 --- a/.github/workflows/elixir.yml +++ b/.github/workflows/elixir.yml @@ -56,7 +56,7 @@ jobs: - name: Compile run: rebar3 as test compile - name: ExUnit - run: mix test test/otel_tests.exs + run: mix test --no-start test/otel_tests.exs api_tests: runs-on: ${{ matrix.os }} diff --git a/test/test_helper.exs b/test/test_helper.exs index c5faaf2a..31c300f7 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -5,7 +5,6 @@ Application.put_env(:opentelemetry, :processors, [ {:otel_batch_processor, %{scheduled_delay_ms: 1}} ]) -Application.load(:opentelemetry_exporter) -Application.ensure_all_started(:opentelemetry) +{:ok, _} = Application.ensure_all_started(:opentelemetry_exporter) ExUnit.start() From fd90b62657e9032e0ddfdcaa5be35ece024eae15 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 21 Feb 2022 14:49:33 -0700 Subject: [PATCH 11/13] add getting started link to top level readme --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index e6ebb37e..b456cb23 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,16 @@ slack](https://slack.cncf.io/). Please join us for more informal discussions. You can also find us in the #opentelemetry channel on [Elixir Slack](https://elixir-slackin.herokuapp.com/). +## Getting Started + +You can find a getting started guide on [opentelemetry.io](https://opentelemetry.io/docs/instrumentation/erlang/getting-started/). + +To start capturing distributed traces from your application it first needs to be +instrumented. The easiest way to do this is by using an instrumentation library, +there are a number of [officially supported instrumentation +libraries](https://github.com/open-telemetry/opentelemetry-erlang-contrib) for +popular Erlang and Elixir libraries and frameworks. + ## Design The [OpenTelemetry From b85e739b98da13ef075c3edb935a519c8906f3ce Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 22 Feb 2022 10:37:11 -0700 Subject: [PATCH 12/13] add explainer about resetting context after with_span --- apps/opentelemetry_api/README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/opentelemetry_api/README.md b/apps/opentelemetry_api/README.md index 106924f9..f4c3018a 100644 --- a/apps/opentelemetry_api/README.md +++ b/apps/opentelemetry_api/README.md @@ -82,7 +82,9 @@ and ending the Span when the `Fun` or body of the Elixir macro finish, even if an exception is thrown -- however, the exception is not caught, so it does not change how user code should deal with raised exceptions. After the Span is ended the Context in the process dictionary is reset to its value before the -newly started Span was set as the active Span. +newly started Span was set as the active Span. This handling of the active Span +in the process dictionary ensures proper lineage of Spans is kept when starting +and ending child Spans. ``` erlang ?with_span(SpanName, StartOpts, Fun) @@ -100,8 +102,10 @@ end [SpanKind](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/api.md#spankind) defines the relationship between the Span, its parents, and its children in a Trace. Possible values: `internal`, `server`, `client`, `producer` and -`consumer`. -- `attributes`: [Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/common/common.md#attributes) +`consumer`. Defaults to `internal` if not specified. +- `attributes`: See + [Attributes](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/common/common.md#attributes) + for details about Attributes. Default is an empty list of attributes. - `links`: List of [Links](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/overview.md#links-between-spans) to causally related Spans from the same or a different Trace. - `start_time`: The start time of the Span operation. Defaults to the current time. The option should only be set if the start of the operation described by From 7e45530c3ed2c761de053c5d485516344b866444 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 22 Feb 2022 11:35:56 -0700 Subject: [PATCH 13/13] elixir sdk tests: no need to start exporter bc tests use pid exporter --- test/test_helper.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_helper.exs b/test/test_helper.exs index 31c300f7..b339830c 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -5,6 +5,6 @@ Application.put_env(:opentelemetry, :processors, [ {:otel_batch_processor, %{scheduled_delay_ms: 1}} ]) -{:ok, _} = Application.ensure_all_started(:opentelemetry_exporter) +{:ok, _} = Application.ensure_all_started(:opentelemetry) ExUnit.start()