Merge pull request #641 from SergeTupchiy/fix-processor-kill-runner

fix(otel_processor): wait for runner process termination

Author: bryannaegele

apps/opentelemetry/src/otel_batch_processor.erl

@@ -367,10 +367,15 @@ complete_exporting(Data) ->
                                  handed_off_table=undefined}}.
 
 kill_runner(Data=#data{runner_pid=RunnerPid}) when RunnerPid =/= undefined ->
+    Mon = erlang:monitor(process, RunnerPid),
     erlang:unlink(RunnerPid),
     erlang:exit(RunnerPid, kill),
-    Data#data{runner_pid=undefined,
-              handed_off_table=undefined}.
+    %% Wait for the runner process termination to be sure that
+    %% the export table is destroyed and can be safely recreated
+    receive
+        {'DOWN', Mon, process, RunnerPid, _} ->
+            Data#data{runner_pid=undefined, handed_off_table=undefined}
+    end.
 
 new_export_table(Name) ->
      ets:new(Name, [public,

apps/opentelemetry/src/otel_simple_processor.erl

@@ -195,10 +195,15 @@ complete_exporting(Data=#data{current_from=From,
      [{reply, From, ok}]}.
 
 kill_runner(Data=#data{runner_pid=RunnerPid}) when RunnerPid =/= undefined ->
+    Mon = erlang:monitor(process, RunnerPid),
     erlang:unlink(RunnerPid),
     erlang:exit(RunnerPid, kill),
-    Data#data{runner_pid=undefined,
-              handed_off_table=undefined}.
+    %% Wait for the runner process termination to be sure that
+    %% the export table is destroyed and can be safely recreated
+    receive
+        {'DOWN', Mon, process, RunnerPid, _} ->
+            Data#data{runner_pid=undefined, handed_off_table=undefined}
+    end.
 
 new_export_table(Name) ->
      ets:new(Name, [public,

apps/opentelemetry/test/otel_batch_processor_SUITE.erl

@@ -10,7 +10,8 @@
 
 all() ->
     [exporting_timeout_test,
-     check_table_size_test].
+     check_table_size_test,
+     exporting_runner_timeout_test].
 
 %% verifies that after the runner has to be killed for taking too long
 %% that everything is still functional and the exporter does not crash
@@ -60,12 +61,36 @@ check_table_size_test(_Config) ->
     timer:sleep(CheckTableSizeMs * 10),
     true = otel_batch_processor:on_end(generate_span(), #{reg_name => RegName}).
 
+exporting_runner_timeout_test(_Config) ->
+    process_flag(trap_exit, true),
+
+    {ok, Pid, #{reg_name := RegName}} = otel_batch_processor:start_link(
+                                          #{name => test_processor1,
+                                            resource => otel_resource:create([]),
+                                            exporter => ?MODULE,
+                                            exporting_timeout_ms => 1,
+                                            scheduled_delay_ms => 1}),
+
+    %% Insert a few spans to make sure runner process will be spawned and killed
+    %% because it hangs for 10 minutes (see export/4 below)
+    true = otel_batch_processor:on_end(generate_span(), #{reg_name => RegName}),
+    true = otel_batch_processor:on_end(generate_span(), #{reg_name => RegName}),
+
+    receive
+        {'EXIT', Pid, _} ->
+            %% test is to ensure we don't hit this
+            ct:fail(batch_processor_crash)
+    after
+        200 ->
+            ok
+    end.
+
 %% exporter behaviour
 
 init(_) ->
     {ok, []}.
 
-export(_, _) ->
+export(_, _, _, _) ->
     timer:sleep(timer:minutes(10)).
 
 shutdown(_) ->