Skip to content

Commit

Permalink
separate application tracer registration from user named tracers
Browse files Browse the repository at this point in the history
Because `persistent_term:put` slows down as the number of terms
grows a project with thousands of modules can cause startup to
timeout as it sets a term for every module in it. This patch
replaces that term for each module with a single term containing
a map of `module => tracer`.

Users must be able to register named tracers at runtime and we
want to not trigger a global GC by updating the tracer map
used for module to tracer mapping. So any user registered tracer
will have its own `persistent_term`.

The macro for looking up a tracer used in macros like `?with_span`
now looks up the tracer by looking up the module in the
"application tracers" (the persistent term containing the map).
While a user looking up a named tracer will do so with the function
`opentelemetry:get_tracer/1` which looks directly for a term with
that name.

This means a user could register a tracer with the same name as a
module. Whether this is good or bad or needs a check to guard
against is not covered by this PR.
  • Loading branch information
Tristan Sloughter committed Oct 8, 2021
1 parent a7ddda6 commit a24a60d
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 89 deletions.
7 changes: 1 addition & 6 deletions apps/opentelemetry/include/otel_span.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
%% @end
%%%-------------------------------------------------------------------------

%% Holds information about the instrumentation library specified when
%% getting a Tracer from the TracerProvider.
-record(instrumentation_library, {name :: unicode:unicode_binary() | undefined,
version :: unicode:unicode_binary() | undefined}).

%% The name, version and language of this OpenTelemetry library
-record(telemetry_library, {name :: unicode:unicode_binary() | undefined,
language :: unicode:unicode_binary() | undefined,
Expand Down Expand Up @@ -70,5 +65,5 @@
%% trace flags lowest bit is 1 but simply not propagated.
is_recording :: boolean() | undefined,

instrumentation_library :: #instrumentation_library{} | undefined
instrumentation_library :: opentelemetry:instrumentation_library() | undefined
}).
3 changes: 2 additions & 1 deletion apps/opentelemetry/src/opentelemetry_app.erl
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ register_loaded_application_tracers(Opts) ->
register_loaded_applications_(RegisterLoadedApplications).

register_loaded_applications_(true) ->
%% TODO: filter out OTP apps that will not have any instrumentation
LoadedApplications = application:loaded_applications(),
[opentelemetry:register_application_tracer(Name) || {Name, _, _} <- LoadedApplications],
opentelemetry:register_application_tracers(LoadedApplications),
ok;
register_loaded_applications_(_) ->
ok.
21 changes: 7 additions & 14 deletions apps/opentelemetry/src/otel_tracer_server.erl
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
-behaviour(otel_tracer_provider).

-export([init/1,
register_tracer/4,
register_tracer/3,
get_tracer/2,
resource/1,
code_change/1]).

Expand Down Expand Up @@ -86,18 +86,11 @@ init(Opts) ->
resource(#state{resource=Resource}) ->
Resource.

register_tracer(Name, Vsn, Modules, #state{shared_tracer=Tracer,
deny_list=DenyList}) ->
%% TODO: support semver constraints in denylist
case proplists:is_defined(Name, DenyList) of
true ->
opentelemetry:set_tracer(Name, {otel_tracer_noop, []});
false ->
InstrumentationLibrary = otel_utils:instrumentation_library(Name, Vsn),
TracerTuple = {Tracer#tracer.module,
Tracer#tracer{instrumentation_library=InstrumentationLibrary}},
[opentelemetry:set_tracer(M, TracerTuple) || M <- Modules]
end.
get_tracer(InstrumentationLibrary, #state{shared_tracer=Tracer,
deny_list=_DenyList}) ->

{Tracer#tracer.module,
Tracer#tracer{instrumentation_library=InstrumentationLibrary}}.

register_tracer(Name, Vsn, #state{shared_tracer=Tracer,
deny_list=DenyList}) ->
Expand All @@ -106,7 +99,7 @@ register_tracer(Name, Vsn, #state{shared_tracer=Tracer,
true ->
opentelemetry:set_tracer(Name, {otel_tracer_noop, []});
false ->
InstrumentationLibrary = otel_utils:instrumentation_library(Name, Vsn),
InstrumentationLibrary = opentelemetry:instrumentation_library(Name, Vsn),
TracerTuple = {Tracer#tracer.module,
Tracer#tracer{instrumentation_library=InstrumentationLibrary}},
opentelemetry:set_tracer(Name, TracerTuple)
Expand Down
29 changes: 0 additions & 29 deletions apps/opentelemetry/src/otel_utils.erl

This file was deleted.

10 changes: 8 additions & 2 deletions apps/opentelemetry/test/opentelemetry_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,25 @@ end_per_testcase(_, _Config) ->
ok.

disable_auto_registration(_Config) ->
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_tracer(kernel),
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_application_tracer(kernel),
?assertEqual(undefined, Library),
ok.

registered_tracers(_Config) ->
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_tracer(kernel),
{_, #tracer{instrumentation_library=Library}} = opentelemetry:get_application_tracer(kernel),
?assertEqual(<<"kernel">>, Library#instrumentation_library.name),

%% register a new tracer with the same name but different version
opentelemetry:register_tracer(kernel, <<"fake-version">>),
{_, #tracer{instrumentation_library=NewLibrary}} = opentelemetry:get_tracer(kernel),
?assertEqual(<<"kernel">>, NewLibrary#instrumentation_library.name),
?assertEqual(<<"fake-version">>, NewLibrary#instrumentation_library.version),

%% tracer registered on startup for a particular application is the same
{_, #tracer{instrumentation_library=Library1}} = opentelemetry:get_application_tracer(kernel),
?assertEqual(<<"kernel">>, Library1#instrumentation_library.name),
?assertNotEqual(<<"fake-version">>, Library1#instrumentation_library.version),

ok.

propagator_configuration(_Config) ->
Expand Down
5 changes: 5 additions & 0 deletions apps/opentelemetry_api/include/opentelemetry.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
-define(OTEL_STATUS_OK, ok).
-define(OTEL_STATUS_ERROR, error).

%% Holds information about the instrumentation library specified when
%% getting a Tracer from the TracerProvider.
-record(instrumentation_library, {name :: unicode:unicode_binary() | undefined,
version :: unicode:unicode_binary() | undefined}).

-record(span_ctx, {
%% 128 bit int trace id
trace_id :: opentelemetry:trace_id(),
Expand Down
2 changes: 1 addition & 1 deletion apps/opentelemetry_api/include/otel_tracer.hrl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
%% macros for tracing
%% register a tracer for an application with opentelemetry:register_application_tracer(AppName)

-define(current_tracer, opentelemetry:get_tracer(?MODULE)).
-define(current_tracer, opentelemetry:get_application_tracer(?MODULE)).
-define(current_span_ctx, otel_tracer:current_span_ctx()).

-define(start_span(SpanName),
Expand Down
90 changes: 72 additions & 18 deletions apps/opentelemetry_api/src/opentelemetry.erl
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,18 @@
-module(opentelemetry).

-export([set_default_tracer/1,
set_tracer/2,
register_tracer/2,
register_application_tracer/1,
register_application_tracers/1,
get_tracer/0,
get_tracer/1,
set_tracer/2,
get_application_tracer/1,
set_text_map_propagator/1,
set_text_map_extractor/1,
get_text_map_extractor/0,
set_text_map_injector/1,
get_text_map_injector/0,
instrumentation_library/2,
timestamp/0,
timestamp_to_nano/1,
convert_timestamp/2,
Expand All @@ -51,7 +53,6 @@
events/1,
status/2,
verify_and_set_term/3,
verify_and_set_term/4,
generate_trace_id/0,
generate_span_id/0]).

Expand Down Expand Up @@ -84,6 +85,9 @@
text_map/0]).

-type tracer() :: {module(), term()}.
%% -type named_tracer_map() :: #{atom() => tracer()}.

-type instrumentation_library() :: #instrumentation_library{}.

-type trace_id() :: non_neg_integer().
-type span_id() :: non_neg_integer().
Expand Down Expand Up @@ -129,48 +133,77 @@

-type text_map() :: [{unicode:unicode_binary(), unicode:unicode_binary()}].

-define(TRACER_KEY(Name), {?MODULE, tracer, Name}).
-define(DEFAULT_TRACER_KEY, ?TRACER_KEY('$__default_tracer')).
-define(APPLICATION_TRACER_KEY, {?MODULE, otel_application_tracer_key}).
-define(TEXT_MAP_EXTRACTOR_KEY, {?MODULE, text_map_extractor}).
-define(TEXT_MAP_INJECTOR_KEY, {?MODULE, text_map_injector}).

-spec set_default_tracer(tracer()) -> boolean().
set_default_tracer(Tracer) ->
verify_and_set_term(Tracer, default_tracer, otel_tracer).
verify_and_set_term(Tracer, ?DEFAULT_TRACER_KEY, otel_tracer).

-spec set_tracer(atom(), tracer()) -> boolean().
set_tracer(Name, Tracer) ->
verify_and_set_term(Tracer, Name, otel_tracer).
verify_and_set_term(Tracer, ?TRACER_KEY(Name), otel_tracer).

-spec register_tracer(atom(), string() | binary()) -> boolean().
register_tracer(Name, Vsn) when is_atom(Name) and is_binary(Vsn) ->
otel_tracer_provider:register_tracer(Name, Vsn);
register_tracer(Name, Vsn) when is_atom(Name) and is_list(Vsn) ->
otel_tracer_provider:register_tracer(Name, list_to_binary(Vsn)).

-spec register_application_tracer(atom()) -> boolean().
register_application_tracer(Name) ->
otel_tracer_provider:register_application_tracer(Name).
-spec register_application_tracers([atom()]) -> boolean().
register_application_tracers(Applications) ->
TracerMap = lists:foldl(fun(Application, Acc) ->
maps:merge(Acc, application_tracer(Application))
end, #{}, Applications),
persistent_term:put(?APPLICATION_TRACER_KEY, TracerMap).

%% creates a map of modules to tracers for a particular application
application_tracer({Name, _Description, Version}) ->
try
{ok, Modules} = application:get_key(Name, modules),
InstrumentationLibrary = instrumentation_library(Name, Version),
TracerTuple = otel_tracer_provider:get_tracer(InstrumentationLibrary),
lists:foldl(fun(M, Acc) ->
Acc#{M => TracerTuple}
end, #{}, Modules)
catch exit:{noproc, _} ->
%% ignore because noproc here means no provider
%% likely since the SDK has been included and started
#{}
end.

-spec get_tracer() -> tracer().
get_tracer() ->
persistent_term:get({?MODULE, default_tracer}, {otel_tracer_noop, []}).
persistent_term:get(?DEFAULT_TRACER_KEY, {otel_tracer_noop, []}).

-spec get_tracer(atom()) -> tracer().
get_tracer(Name) ->
persistent_term:get({?MODULE, Name}, get_tracer()).
persistent_term:get(?TRACER_KEY(Name), get_tracer()).

-spec get_application_tracer(atom()) -> tracer().
get_application_tracer(ModuleName) ->
Map = persistent_term:get(?APPLICATION_TRACER_KEY, #{}),
maps:get(ModuleName, Map, get_tracer()).

%% setting the propagator is the same as setting the same injector and extractor
set_text_map_propagator(Propagator) ->
set_text_map_injector(Propagator),
set_text_map_extractor(Propagator).

set_text_map_extractor(Propagator) ->
persistent_term:put({?MODULE, text_map_extractor}, Propagator).
persistent_term:put(?TEXT_MAP_EXTRACTOR_KEY, Propagator).

set_text_map_injector(Propagator) ->
persistent_term:put({?MODULE, text_map_injector}, Propagator).
persistent_term:put(?TEXT_MAP_INJECTOR_KEY, Propagator).

get_text_map_extractor() ->
persistent_term:get({?MODULE, text_map_extractor}, otel_propagator_text_map_noop).
persistent_term:get(?TEXT_MAP_EXTRACTOR_KEY, otel_propagator_text_map_noop).

get_text_map_injector() ->
persistent_term:get({?MODULE, text_map_injector}, otel_propagator_text_map_noop).
persistent_term:get(?TEXT_MAP_INJECTOR_KEY, otel_propagator_text_map_noop).

%% @doc A monotonically increasing time provided by the Erlang runtime system in the native time unit.
%% This value is the most accurate and precise timestamp available from the Erlang runtime and
Expand Down Expand Up @@ -341,12 +374,9 @@ uniform(X) ->

-spec verify_and_set_term(module() | {module(), term()}, term(), atom()) -> boolean().
verify_and_set_term(Module, TermKey, Behaviour) ->
verify_and_set_term(?MODULE, Module, TermKey, Behaviour).

verify_and_set_term(AppKey, Module, TermKey, Behaviour) ->
case verify_module_exists(Module) of
true ->
persistent_term:put({AppKey, TermKey}, Module),
persistent_term:put(TermKey, Module),
true;
false ->
?LOG_WARNING("Module ~p does not exist. "
Expand Down Expand Up @@ -376,3 +406,27 @@ link_or_false(TraceId, SpanId, Attributes, TraceState) ->
_ ->
false
end.

instrumentation_library(Name, Vsn) ->
#instrumentation_library{name=name_to_binary(Name),
version=vsn_to_binary(Vsn)}.

%% Vsn can't be an atom or anything but a list or binary
%% so return `undefined' if it isn't a list or binary.
vsn_to_binary(Vsn) when is_list(Vsn) ->
list_to_binary(Vsn);
vsn_to_binary(Vsn) when is_binary(Vsn) ->
Vsn;
vsn_to_binary(_) ->
undefined.

%% name can be atom, list or binary. But atom `undefined'
%% must stay as `undefined' atom.
name_to_binary(undefined)->
undefined;
name_to_binary(T) when is_atom(T) ->
atom_to_binary(T, utf8);
name_to_binary(T) when is_list(T) ->
list_to_binary(T);
name_to_binary(T) when is_binary(T) ->
T.
23 changes: 11 additions & 12 deletions apps/opentelemetry_api/src/otel_tracer_provider.erl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

-export([start_link/2,
resource/0,
register_application_tracer/1,
get_tracer/1,
register_tracer/2]).

-export([init/1,
Expand All @@ -33,6 +33,7 @@

-callback init(term()) -> {ok, cb_state()}.
-callback register_tracer(atom(), binary(), cb_state()) -> boolean().
-callback get_tracer(opentelemetry:instrumentation_library()) -> opentelemetry:tracer() | undefined.
-callback resource(cb_state()) -> term() | undefined.

-record(state, {callback :: module(),
Expand All @@ -59,15 +60,13 @@ register_tracer(Name, Vsn) ->
false
end.

-spec register_application_tracer(atom()) -> boolean().
register_application_tracer(Name) ->
-spec get_tracer(opentelemetry:instrumentation_library()) -> opentelemetry:tracer() | undefined.
get_tracer(InstrumentationLibrary) ->
try
{ok, Vsn} = application:get_key(Name, vsn),
{ok, Modules} = application:get_key(Name, modules),
gen_server:call(?MODULE, {register_tracer, Name, Vsn, Modules})
gen_server:call(?MODULE, {get_tracer, InstrumentationLibrary})
catch exit:{noproc, _} ->
%% ignore register_tracer because no SDK has been included and started
false
%% ignore because likely no SDK has been included and started
{otel_tracer_noop, []}
end.

init([ProviderModule, Opts]) ->
Expand All @@ -79,10 +78,10 @@ init([ProviderModule, Opts]) ->
Other
end.

handle_call({register_tracer, Name, Vsn, Modules}, _From, State=#state{callback=Cb,
cb_state=CbState}) ->
_ = Cb:register_tracer(Name, Vsn, Modules, CbState),
{reply, true, State};
handle_call({get_tracer, InstrumentationLibrary}, _From, State=#state{callback=Cb,
cb_state=CbState}) ->
Tracer = Cb:get_tracer(InstrumentationLibrary, CbState),
{reply, Tracer, State};
handle_call({register_tracer, Name, Vsn}, _From, State=#state{callback=Cb,
cb_state=CbState}) ->
_ = Cb:register_tracer(Name, Vsn, CbState),
Expand Down
Loading

0 comments on commit a24a60d

Please sign in to comment.