Observer Improvements

[filmor]

• Allow starting the observer connected to a node
• Ignore repeated nodedown messages

[github-actions]

CT Test Results

    2 files    15 suites   15m 25s ⏱️
157 tests 155 ✔️ 2 💤 0 ❌
174 runs  172 ✔️ 2 💤 0 ❌

Results for commit bdbd83b.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[dgud]

Fix typo and test that it works as you intended because it can't right now.
Would also prefer filter_node_down instead of vacuum.

[dgud]

This works:

>$ERL_TOP/bin/erl -sname foo -run observer start node@host
But

>$ERL_TOP/bin/erl -sname foo -run observer start_and_wait node@host
Erlang/OTP 26 [DEVELOPMENT] [erts-13.1.2] [source-0800de8a81] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Runtime terminating during boot ({function_clause,[{observer,start,node@host,[{_},{_}]},{observer,start_and_wait,1,[{_},{_}]},{init,start_em,1,[]},{init,do_boot,3,[]}]})

Add test cases and update documentation with new functions,
the tests can be simple just using the new functions but I want them covered for regressions purposes.

[filmor]

It does what I want it to now, namely, I can start the observer like this now:

erl -noshell -setcookie some_cookie -sname undefined -run observer start_and_wait some_node@some_machine -run init stop

[dgud]

Can you fix the doc build problems?

[dgud]

Test fail

10:55:26.431 <0.13921.2> init_per_testcase(blocking_start) -> entry with
   Config: [{watchdog,<0.13922.2>},
            {tc_logfile,"/ldisk/daily_build/26_master-opu_o.2023-01-10_21/test/test_server/ct_run.test_server@bard.2023-01-11_10.53.26/test.observer_test.logs/run.2023-01-11_10.53.28/observer_suite.blocking_start.html"},
            {tc_group_properties,[{name,gui}]},
            {tc_group_path,[]},
            {data_dir,"/ldisk/daily_build/26_master-opu_o.2023-01-10_21/test/observer_test/observer_SUITE_data/"},
            {priv_dir,"/ldisk/daily_build/26_master-opu_o.2023-01-10_21/test/test_server/ct_run.test_server@bard.2023-01-11_10.53.26/test.observer_test.logs/run.2023-01-11_10.53.28/log_private/"},
            {nodenames,[]}]

10:55:26.431 <0.13921.2> init_per_testcase(blocking_start) -> entry


*** CT Error Notification 2023-01-11 10:55:26.432 ***[🔗](http://otp.ericsson.se/product/internal/test/test_results/progress_26/2023_01_10/otp_26_bard_linux-gnu_x86_64_64-bit_jit_s16_a1_offheapmsgq_meamin/ct_run.test_server@bard.2023-01-11_10.53.26/test.observer_test.logs/run.2023-01-11_10.53.28/observer_suite.blocking_start.html#e-30)
observer_SUITE:blocking_start failed on line 487
Reason: {observer_stopped_unexpectedly,[{observer_SUITE,...},{...}|...]}

[Full error description and stacktrace](http://otp.ericsson.se/product/internal/test/test_results/progress_26/2023_01_10/otp_26_bard_linux-gnu_x86_64_64-bit_jit_s16_a1_offheapmsgq_meamin/ct_run.test_server@bard.2023-01-11_10.53.26/test.observer_test.logs/run.2023-01-11_10.53.28/observer_suite.blocking_start.html#end)
10:55:26.432 <0.13921.2> end_per_testcase(blocking_start) -> entry with
   Config: [{tc_status,
                {failed,
                    {observer_stopped_unexpectedly,
                        [{observer_SUITE,blocking_start,1,
                             [{file,"observer_SUITE.erl"},{line,487}]},
                         {test_server,ts_tc,3,
                             [{file,"test_server.erl"},{line,1782}]},
                         {test_server,run_test_case_eval1,6,
                             [{file,"test_server.erl"},{line,1291}]},
                         {test_server,run_test_case_eval,9,
                             [{file,"test_server.erl"},{line,1223}]}]}}},
            {tc_fail_loc,
                [{observer_SUITE,blocking_start,487},
                 {test_server,ts_tc,1782},
                 {test_server,run_test_case_eval1,1291},
                 {test_server,run_test_case_eval,1223}]},
            {watchdog,<0.13923.2>},
            {watchdog,<0.13922.2>},
            {tc_logfile,
                "/ldisk/daily_build/26_master-opu_o.2023-01-10_21/test/test_server/ct_run.test_server@bard.2023-01-11_10.53.26/test.observer_test.logs/run.2023-01-11_10.53.28/observer_suite.blocking_start.html"},
            {tc_group_properties,[{name,gui}]},
            {tc_group_path,[]},
            {data_dir,
                "/ldisk/daily_build/26_master-opu_o.2023-01-10_21/test/observer_test/observer_SUITE_data/"},
            {priv_dir,
                "/ldisk/daily_build/26_master-opu_o.2023-01-10_21/test/test_server/ct_run.test_server@bard.2023-01-11_10.53.26/test.observer_test.logs/run.2023-01-11_10.53.28/log_private/"},
            {nodenames,[]}]

10:55:26.432 <0.13921.2> end_per_testcase2(blocking_start) -> entry - try cancel watchdog

10:55:26.432 <0.13921.2> end_per_testcase2(blocking_start) -> done

[filmor]

I saw that there is an undocumented function net_kernel:hidden_connect_node/1, would it be an option to use that instead of net_kernel:connect_node/1 here?

[dgud]

diff --git a/lib/observer/test/observer_SUITE.erl b/lib/observer/test/observer_SUITE.erl
index e5d9e6cc80..0aef5832d2 100644
--- a/lib/observer/test/observer_SUITE.erl
+++ b/lib/observer/test/observer_SUITE.erl
@@ -60,7 +60,7 @@ groups() ->
        process_win,
        table_win,
        port_win_when_tab_not_initiated,
-       % blocking_start,
+       blocking_start,
        remote_node
       ]
      }].
@@ -474,20 +474,21 @@ table_win(Config) when is_list(Config) ->
 remote_node(_Config) ->
     {ok, Peer, Node} = ?CT_PEER(),
     ok = observer:start(Node),
+    timer:sleep(1000),
     Node = observer_wx:get_active_node(),
     observer:stop(),
     ensure_observer_stopped(),
     peer:stop(Peer).

 blocking_start(_Config) ->
-    Pid = spawn(fun observer:start_and_wait/0),
-    SpawnerRef = monitor(process, Pid),
+    {Pid, SpawnerRef} = spawn_monitor(fun observer:start_and_wait/0),
+    timer:sleep(1000),
     ObserverRef = monitor(process, observer),
     receive
-        {'DOWN', ObserverRef, _, _, _} ->
-            error(observer_stopped_unexpectedly);
-        {'DOWN', SpawnerRef, _, _, _} ->
-            error(spawner_stopped_unexpectedly)
+        {'DOWN', ObserverRef, _, _, Reason} ->
+            error({observer_stopped_unexpectedly, Reason});
+        {'DOWN', SpawnerRef, _, _, Reason} ->
+            error({spawner_stopped_unexpectedly, Reason})
     after
         500 ->
             ok

You have some timing issues.

But looking at the code again, shouldn't the Node argument be sent observer_wx:start() function and be handled there
instead of outside the observer process where you have to introduce a function/handle_call to connect to the node?

[filmor]

Hmm, the timing issues are odd, shouldn't the startup be synchronous? Or are the 500ms just not enough for a wx app?

I figured that set_node would be a more useful function as this fits with how the application works internally. Would you want this code moved to observer_wx:start or observer_wx:init?

[dgud]

The sleep in the first testcase is needed because observers startup is not synchronous, and the code deadlocks it self when you call observer:stop().
That should probably be fixed, but is another issue.

The second sleep is needed because you do a monitor on the observer process which may not have started yet, i.e. directly after the spawn().

Regarding the set_node(), I see your point so let me ponder on that for a while before you re-write it.

[filmor]

Okay, I'll add the sleeps. I'll also have a look whether I can make the startup synchronous in a separate PR.

[filmor]

Great, thank you :)