Skip to content

Commit

Permalink
Merge pull request #286 from fr0stbyte/radu/span_end_time2
Browse files Browse the repository at this point in the history
allows for optional end span timestamp
  • Loading branch information
tsloughter authored Oct 5, 2021
2 parents b70723b + 4b018e8 commit a7ddda6
Show file tree
Hide file tree
Showing 13 changed files with 77 additions and 45 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/elixir.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- uses: actions/checkout@v2
- uses: erlef/setup-beam@v1
with:
otp-version: '24.0.1'
otp-version: '24.1.2'
elixir-version: '1.12.1'
- uses: actions/cache@v2
name: Cache
Expand All @@ -33,7 +33,7 @@ jobs:
name: Test on Elixir ${{ matrix.elixir_version }} (OTP ${{ matrix.otp_version }}) and ${{ matrix.os }}
strategy:
matrix:
otp_version: ['24.0.2', '23.3.4.2', '22.3.4.20']
otp_version: ['24.1.2', '23.3.4.2', '22.3.4.20']
elixir_version: ['1.12.1', '1.11.4']
rebar3_version: ['3.16.1']
os: [ubuntu-18.04]
Expand Down Expand Up @@ -66,7 +66,7 @@ jobs:
name: Test on Elixir ${{ matrix.elixir_version }} (OTP ${{ matrix.otp_version }}) and ${{ matrix.os }}
strategy:
matrix:
otp_version: ['24.0.2', '23.3.4.2', '22.3.4.20']
otp_version: ['24.1.2', '23.3.4.2', '22.3.4.20']
elixir_version: ['1.12.1', '1.11.4']
rebar3_version: ['3.16.1']
os: [ubuntu-18.04]
Expand Down Expand Up @@ -116,7 +116,7 @@ jobs:
name: Dialyze on Elixir ${{ matrix.elixir_version }} (OTP ${{ matrix.otp_version }}) and ${{ matrix.os }}
strategy:
matrix:
otp_version: ['24.0.2']
otp_version: ['24.1.2']
elixir_version: ['1.12.1']
os: [ubuntu-18.04]
env:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/erlang.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
otp_version: ['24.0.2', '23.3.4.2', '22.3.4.20']
otp_version: ['24.1.2', '23.3.4.2', '22.3.4.20']
rebar3_version: ['3.16.1']
os: [ubuntu-18.04]
include:
Expand Down Expand Up @@ -75,7 +75,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
otp_version: ['24.0.2', '23.3.4.2', '22.3.4.20']
otp_version: ['24.1.2', '23.3.4.2', '22.3.4.20']
rebar3_version: ['3.16.1']
os: [ubuntu-18.04]
include:
Expand Down
13 changes: 8 additions & 5 deletions apps/opentelemetry/src/otel_span_ets.erl
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,25 @@ start_span(Ctx, Name, Opts, Processors, InstrumentationLibrary) ->
end.

end_span(SpanCtx=#span_ctx{span_sdk={_, OnEndProcessors}}) ->
end_span(SpanCtx, OnEndProcessors).
end_span(SpanCtx, undefined, OnEndProcessors).

end_span(SpanCtx=#span_ctx{span_sdk={_, OnEndProcessors}}, Timestamp) ->
end_span(SpanCtx, Timestamp, OnEndProcessors).

%% @doc End a span based on its context and send to exporter.
-spec end_span(opentelemetry:span_ctx(), fun()) -> boolean() | {error, term()}.
-spec end_span(opentelemetry:span_ctx(), integer() | undefined, fun()) -> boolean() | {error, term()}.
end_span(#span_ctx{span_id=SpanId,
is_recording=true,
tracestate=Tracestate}, Processors) ->
tracestate=Tracestate}, Timestamp, Processors) ->
case ets:take(?SPAN_TAB, SpanId) of
[Span] ->
Span1 = otel_span_utils:end_span(Span#span{tracestate=Tracestate,
is_recording=false}),
is_recording=false}, Timestamp),
Processors(Span1);
_ ->
false
end;
end_span(_, _) ->
end_span(_, _, _) ->
true.

-spec get_ctx(opentelemetry:span()) -> opentelemetry:span_ctx().
Expand Down
11 changes: 8 additions & 3 deletions apps/opentelemetry/src/otel_span_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
-module(otel_span_utils).

-export([start_span/3,
end_span/1]).
end_span/1,
end_span/2]).

-include_lib("opentelemetry_api/include/opentelemetry.hrl").
-include("otel_sampler.hrl").
Expand Down Expand Up @@ -86,11 +87,15 @@ root_span_ctx() ->
-spec end_span(opentelemetry:span()) -> opentelemetry:span().
end_span(Span=#span{end_time=undefined,
trace_flags=TraceFlags}) when ?IS_SAMPLED(TraceFlags) ->
EndTime = opentelemetry:timestamp(),
Span#span{end_time=EndTime};
end_span(Span, undefined);
end_span(Span) ->
Span.

-spec end_span(opentelemetry:span(), integer() | undefined) -> opentelemetry:span().
end_span(Span, Timestamp) when is_integer(Timestamp) ->
Span#span{end_time=Timestamp};
end_span(Span, _) ->
Span#span{end_time=opentelemetry:timestamp()}.
%%

sample(Ctx, Sampler, TraceId, Links, SpanName, Kind, Attributes) ->
Expand Down
22 changes: 11 additions & 11 deletions apps/opentelemetry/test/opentelemetry_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
all() ->
[disable_auto_registration, registered_tracers, with_span, macros, child_spans,
update_span_data, tracer_instrumentation_library, tracer_previous_ctx, stop_temporary_app,
reset_after, attach_ctx, default_sampler, non_recording_ets_table,
root_span_sampling_always_on, root_span_sampling_always_off,
reset_after, attach_ctx, default_sampler, non_recording_ets_table,
root_span_sampling_always_on, root_span_sampling_always_off,
record_but_not_sample, record_exception_works, record_exception_with_message_works,
propagator_configuration, propagator_configuration_with_os_env].

Expand Down Expand Up @@ -142,7 +142,7 @@ macros(Config) ->
?set_current_span(SpanCtx2),

?assertMatch(SpanCtx2, ?current_span_ctx),
?end_span(SpanCtx2),
otel_span:end_span(SpanCtx2),

?set_current_span(SpanCtx1),
?assertMatch(SpanCtx1, ?current_span_ctx),
Expand All @@ -151,7 +151,7 @@ macros(Config) ->
AttrValue1 = <<"attr-value-1">>,
?set_attribute(Attr1, AttrValue1),

?end_span(SpanCtx1),
otel_span:end_span(SpanCtx1),

[Span1] = assert_exported(Tid, SpanCtx1),

Expand Down Expand Up @@ -197,7 +197,7 @@ child_spans(Config) ->

%% end the 3rd span
?assertMatch(SpanCtx3, ?current_span_ctx),
?end_span(SpanCtx3),
otel_span:end_span(SpanCtx3),

assert_exported(Tid, SpanCtx3),

Expand All @@ -214,18 +214,18 @@ child_spans(Config) ->
?assertMatch(SpanCtx4, ?current_span_ctx),

%% end 4th span and 2nd should be current
?end_span(SpanCtx4),
otel_span:end_span(SpanCtx4),

?set_current_span(SpanCtx2),
?assertMatch(SpanCtx2, ?current_span_ctx),

%% end 2th span and 1st should be current
?end_span(SpanCtx2),
otel_span:end_span(SpanCtx2),
?set_current_span(SpanCtx1),
?assertMatch(SpanCtx1, ?current_span_ctx),

%% end first and no span should be current ctx
?end_span(SpanCtx1),
otel_span:end_span(SpanCtx1),
?set_current_span(undefined),
?assertMatch(undefined, ?current_span_ctx),

Expand Down Expand Up @@ -257,7 +257,7 @@ update_span_data(Config) ->
otel_span:add_events(SpanCtx1, Events),

?assertMatch(SpanCtx1, ?current_span_ctx),
?end_span(SpanCtx1),
otel_span:end_span(SpanCtx1),

?UNTIL_NOT_EQUAL([], ets:match(Tid, #span{trace_id=TraceId,
span_id=SpanId,
Expand Down Expand Up @@ -519,7 +519,7 @@ record_exception_works(Config) ->
catch
Class:Term:Stacktrace ->
otel_span:record_exception(SpanCtx, Class, Term, Stacktrace, [{"some-attribute", "value"}]),
?end_span(SpanCtx),
otel_span:end_span(SpanCtx),
[Span] = assert_exported(Tid, SpanCtx),
[Event] = Span#span.events,
?assertEqual(<<"exception">>, Event#event.name),
Expand All @@ -539,7 +539,7 @@ record_exception_with_message_works(Config) ->
catch
Class:Term:Stacktrace ->
otel_span:record_exception(SpanCtx, Class, Term, "My message", Stacktrace, [{"some-attribute", "value"}]),
?end_span(SpanCtx),
otel_span:end_span(SpanCtx),
[Span] = assert_exported(Tid, SpanCtx),
[Event] = Span#span.events,
?assertEqual(<<"exception">>, Event#event.name),
Expand Down
4 changes: 2 additions & 2 deletions apps/opentelemetry/test/otel_propagation_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ propagation(Config) ->
{<<"existing-header">>, <<"I exist">>} |
trace_context(Propagator, EncodedTraceId, EncodedSpanId)], Headers),

?end_span(SpanCtx),
otel_span:end_span(SpanCtx),

?assertEqual(#{<<"key-1">> => {<<"value=1">>, []},
<<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}},
Expand Down Expand Up @@ -140,7 +140,7 @@ override_propagators(_Config) ->
?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata;md-k-1=md-v-1,key-1=value%3D1">>},
{<<"existing-header">>, <<"I exist">>}], Headers),

?end_span(SpanCtx),
otel_span:end_span(SpanCtx),

?assertEqual(#{<<"key-1">> => {<<"value=1">>, []},
<<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}},
Expand Down
8 changes: 4 additions & 4 deletions apps/opentelemetry/test/otel_sweeper_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ drop(_Config) ->
?assertEqual(ChildSpanName1, ChildSpanData#span.name),
?assertEqual(SpanCtx#span_ctx.span_id, ChildSpanData#span.parent_span_id),

?end_span(ChildSpanCtx),
otel_span:end_span(ChildSpanCtx),

%% wait until the sweeper sweeps away the parent span
?UNTIL(ets:tab2list(?SPAN_TAB) =:= []),

?end_span(SpanCtx),
otel_span:end_span(SpanCtx),

receive
{span, S=#span{name=Name}} when Name =:= ChildSpanName1 ->
Expand Down Expand Up @@ -137,7 +137,7 @@ end_span(_Config) ->
ChildSpanCtx = ?start_span(ChildSpanName1),
otel_tracer:set_current_span(ChildSpanCtx),

?end_span(SpanCtx),
otel_span:end_span(SpanCtx),

%% wait until the sweeper sweeps away the parent span
?UNTIL(ets:tab2list(?SPAN_TAB) =:= []),
Expand Down Expand Up @@ -166,7 +166,7 @@ failed_attribute_and_end_span(_Config) ->
?assertEqual(ChildSpanName1, ChildSpanData#span.name),
?assertEqual(SpanCtx#span_ctx.span_id, ChildSpanData#span.parent_span_id),

?end_span(ChildSpanData),
otel_span:end_span(ChildSpanData),

%% wait until the sweeper sweeps away the parent span
?UNTIL(ets:tab2list(?SPAN_TAB) =:= []),
Expand Down
6 changes: 3 additions & 3 deletions apps/opentelemetry_api/include/otel_tracer.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
%% updates the current context with the updated span context that
%% has `is_recording' set to `false'
-define(end_span(),
otel_tracer:set_current_span(otel_span:end_span(?current_span_ctx))).
?end_span(undefined)).

-define(end_span(SpanCtx),
otel_span:end_span(SpanCtx)).
-define(end_span(Timestamp),
otel_tracer:set_current_span(otel_span:end_span(?current_span_ctx, Timestamp))).

-define(is_recording(),
otel_span:is_recording(?current_span_ctx)).
Expand Down
8 changes: 8 additions & 0 deletions apps/opentelemetry_api/lib/open_telemetry/span.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ defmodule OpenTelemetry.Span do
"""
defdelegate end_span(span_ctx), to: :otel_span

@doc """
End the Span. Sets the end timestamp for the currently active Span using the passed in timestamp if valid, current timestamp otherwise. This has no effect on any
child Spans that may exist of this Span.
The Span Context is returned with `is_recording` set to `false`.
"""
defdelegate end_span(span_ctx, timestamp), to: :otel_span

@doc """
"""
Expand Down
6 changes: 4 additions & 2 deletions apps/opentelemetry_api/lib/open_telemetry/tracer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ defmodule OpenTelemetry.Tracer do
The Span in the current Context has its `is_recording` set to `false`.
"""
def end_span() do
non_recording_span = :otel_span.end_span(:otel_tracer.current_span_ctx())
@spec end_span(:opentelemetry.timestamp() | :undefined) ::
:opentelemetry.span_ctx() | :undefined
def end_span(timestamp \\ :undefined) do
non_recording_span = :otel_span.end_span(:otel_tracer.current_span_ctx(), timestamp)
_ = :otel_tracer.set_current_span(non_recording_span)
non_recording_span
end
Expand Down
18 changes: 16 additions & 2 deletions apps/opentelemetry_api/src/otel_span.erl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
record_exception/6,
set_status/2,
update_name/2,
end_span/1]).
end_span/1,
end_span/2]).

-include("opentelemetry.hrl").

Expand Down Expand Up @@ -162,7 +163,20 @@ update_name(_, _) ->
-spec end_span(SpanCtx) -> SpanCtx when
SpanCtx :: opentelemetry:span_ctx().
end_span(SpanCtx=#span_ctx{span_sdk={Module, _}}) when ?is_recording(SpanCtx) ->
_ = Module:end_span(SpanCtx),
_ = Module:end_span(SpanCtx, undefined),
SpanCtx#span_ctx{is_recording=false};
end_span(SpanCtx) ->
SpanCtx.

-spec end_span(SpanCtx, Timestamp) -> SpanCtx when
SpanCtx :: opentelemetry:span_ctx(),
Timestamp :: integer() | undefined.
end_span(SpanCtx=#span_ctx{span_sdk={Module, _}}, Timestamp) when ?is_recording(SpanCtx)
, is_integer(Timestamp) ->
_ = Module:end_span(SpanCtx, Timestamp),
SpanCtx#span_ctx{is_recording=false};
end_span(SpanCtx=#span_ctx{span_sdk={Module, _}}, _Timestamp) when ?is_recording(SpanCtx) ->
_ = Module:end_span(SpanCtx),
SpanCtx#span_ctx{is_recording=false};
end_span(SpanCtx, _) ->
SpanCtx.
12 changes: 6 additions & 6 deletions apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ can_create_link_from_span(_Config) ->
Tracestate = otel_span:tracestate(SpanCtx),

%% end span, so there's no current span set
?end_span(),
?end_span(opentelemetry:timestamp()),

Attributes = [{<<"attr-1">>, <<"value-1">>}],

Expand Down Expand Up @@ -75,7 +75,7 @@ noop_tracer(_Config) ->
%% set to current and then end the 3rd span
?set_current_span(SpanCtx3),
?assertMatch(SpanCtx3, ?current_span_ctx),
?end_span(SpanCtx3),
otel_span:end_span(SpanCtx3),

?set_current_span(SpanCtx2),
?assertMatch(SpanCtx2, ?current_span_ctx),
Expand All @@ -87,19 +87,19 @@ noop_tracer(_Config) ->
?assertMatch(SpanCtx4, ?current_span_ctx),

%% end 4th span
?end_span(SpanCtx4),
otel_span:end_span(SpanCtx4),

?set_current_span(SpanCtx2),
?assertMatch(SpanCtx2, ?current_span_ctx),

%% end 2th span
?end_span(),
?end_span(opentelemetry:timestamp()),

?set_current_span(SpanCtx1),
?assertMatch(SpanCtx1, ?current_span_ctx),

%% end first and no span should be current ctx
?end_span(),
?end_span(opentelemetry:timestamp()),

%% 1st span is ended but still current
?assertMatch(SpanCtx1, ?current_span_ctx).
Expand Down Expand Up @@ -132,7 +132,7 @@ update_span_data(_Config) ->
otel_span:add_events(SpanCtx1, Events),

?assertMatch(SpanCtx1, ?current_span_ctx),
?end_span(),
?end_span(opentelemetry:timestamp()),

?assertMatch(#span_ctx{is_recording=false}, ?current_span_ctx),

Expand Down
2 changes: 1 addition & 1 deletion interop/w3c_trace_context_interop.erl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ do(Req) ->
headers_to_list(InjectedHeaders),
"application/json",
jsone:encode(Arguments)}, [], [{body_format, binary}]),
?end_span()
?end_span(opentelemetry:timestmap())
end, List),

{proceed, [{response, {200, "ok"}}]}.
Expand Down

0 comments on commit a7ddda6

Please sign in to comment.