From 30af1111fea8a461fc3ee0203961ec6c566be419 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 6 Nov 2023 06:20:27 -0700 Subject: [PATCH] create unique processor name in otel_tracer_server Each processor needs a unique name to reference but always creating it in start_link means everytime the process restarts it creates a new atom and new persistent term key. This is a leak. This patch moves the atom creation to otel_tracer_server so it is only done once. --- apps/opentelemetry/src/otel_batch_processor.erl | 16 +++++----------- apps/opentelemetry/src/otel_simple_processor.erl | 16 +++++----------- apps/opentelemetry/src/otel_tracer_server.erl | 9 ++++++++- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/apps/opentelemetry/src/otel_batch_processor.erl b/apps/opentelemetry/src/otel_batch_processor.erl index d1374c55..433dea95 100644 --- a/apps/opentelemetry/src/otel_batch_processor.erl +++ b/apps/opentelemetry/src/otel_batch_processor.erl @@ -90,17 +90,11 @@ current_tab_to_list(RegName) -> ets:tab2list(?CURRENT_TABLE(RegName)). -endif. -start_link(Config) -> - Name = case maps:find(name, Config) of - {ok, N} -> - N; - error -> - %% use a unique reference to distiguish multiple batch processors while - %% still having a single name, instead of a possibly changing pid, to - %% communicate with the processor - erlang:ref_to_list(erlang:make_ref()) - end, - +%% require a unique name to distiguish multiple batch processors while +%% still having a single name, instead of a possibly changing pid, to +%% communicate with the processor +-spec start_link(#{name := atom() | list()}) -> {ok, pid(), map()}. +start_link(Config=#{name := Name}) -> RegisterName = ?REG_NAME(Name), Config1 = Config#{reg_name => RegisterName}, {ok, Pid} = gen_statem:start_link({local, RegisterName}, ?MODULE, [Config1], []), diff --git a/apps/opentelemetry/src/otel_simple_processor.erl b/apps/opentelemetry/src/otel_simple_processor.erl index eaa5fd83..bb877c4b 100644 --- a/apps/opentelemetry/src/otel_simple_processor.erl +++ b/apps/opentelemetry/src/otel_simple_processor.erl @@ -64,17 +64,11 @@ -define(DEFAULT_EXPORTER_TIMEOUT_MS, timer:minutes(5)). -define(NAME_TO_ATOM(Name, Unique), list_to_atom(lists:concat([Name, "_", Unique]))). -start_link(Config) -> - Name = case maps:find(name, Config) of - {ok, N} -> - N; - error -> - %% use a unique reference to distiguish multiple batch processors while - %% still having a single name, instead of a possibly changing pid, to - %% communicate with the processor - erlang:ref_to_list(erlang:make_ref()) - end, - +%% require a unique name to distiguish multiple simple processors while +%% still having a single name, instead of a possibly changing pid, to +%% communicate with the processor +-spec start_link(#{name := atom() | list()}) -> {ok, pid(), map()}. +start_link(Config=#{name := Name}) -> RegisterName = ?NAME_TO_ATOM(?MODULE, Name), Config1 = Config#{reg_name => RegisterName}, {ok, Pid} = gen_statem:start_link({local, RegisterName}, ?MODULE, [Config1], []), diff --git a/apps/opentelemetry/src/otel_tracer_server.erl b/apps/opentelemetry/src/otel_tracer_server.erl index 4b536906..00e367e5 100644 --- a/apps/opentelemetry/src/otel_tracer_server.erl +++ b/apps/opentelemetry/src/otel_tracer_server.erl @@ -159,7 +159,14 @@ init_processor(SpanProcessorSup, ProcessorModule, Config) -> %% start_link is an optional callback for processors case lists:member({start_link, 1}, ProcessorModule:module_info(exports)) of true -> - try supervisor:start_child(SpanProcessorSup, [ProcessorModule, Config]) of + try supervisor:start_child(SpanProcessorSup, + [ProcessorModule, + %% use a unique reference to distiguish multiple processors of the same type while + %% still having a single name, instead of a possibly changing pid, to + %% communicate with the processor + maps:merge(#{name => erlang:ref_to_list(erlang:make_ref())}, + Config)]) + of {ok, _Pid, Config1} -> {true, {ProcessorModule, Config1}}; {error, Reason} ->