Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeline Tracking #17

Merged
merged 32 commits into from
Jun 27, 2017
Merged

Timeline Tracking #17

merged 32 commits into from
Jun 27, 2017

Conversation

Hajto
Copy link
Contributor

@Hajto Hajto commented Mar 20, 2017

Added:

  • epl_timeline_observer.erl - genserver that hosts timeline changes and state changes

Edited:

  • epl_tracer - two new ets tables, one for storing pids that are tracked and one for caching changes

To test

{ok, TS} = epl_timeline_observer:start_link(list_to_pid("<13611.385.0>")).
%% will yield timeline so far
epl_timeline_observer:timeline(TS).

@Hajto
Copy link
Contributor Author

Hajto commented Mar 21, 2017

zrzut ekranu 2017-03-20 o 20 02 46

@michalslaski
Copy link
Member

Thank you very much for contributing! I will review this PR asap, hopefully tonight.

BTW, how do you like lectures on discrete mathematics? ;)

@baransu
Copy link
Contributor

baransu commented Mar 21, 2017

I know we had conversation about this plugin but I dont really remember when and how should we display time travel debug info in our UI.

@Hajto
Copy link
Contributor Author

Hajto commented Mar 21, 2017

About UI. Ideal way would be a separate pane with list of tracked timelines, and input to type in the pid manually. (That's partially why pids are compared as lists).

Very nice thing would be also a track timeline button in one of the other views in process details. The click would start trackign and move user to timeline view.

Yet another option would be a pane with all the pids in a filterable lists, user could tick interesting pids of and see their timeline on the other subpane. (left half pane list, right half timeline)

At least that's how I would imagine it.

@baransu
Copy link
Contributor

baransu commented Mar 21, 2017

I'll try to add think about something for it. Is it possible to have it as separate plugin? It can be good starting point for plugin system for frontend as well because I'm thinking about moving all core plugins to one "core/official" plugins monorepo at some point and we need plugin system for that.

@Hajto
Copy link
Contributor Author

Hajto commented Mar 21, 2017

The problem with making it Plugin is fact that I had to severely modify already existing tracer. (At stock it gives only information about amount of messages between processes)

@michalslaski
Copy link
Member

Hi @Hajto, I was thinking how we could visualise the trace information you gather and one idea is to render so-called flame graphs. It could be rendered in the 3rd level of Vizceral, when one double-clicks a process. In other words can you please review your PR and see if you gather enough information to render a flame graph? Such graphs for Erlang processes are already implemented by @proger in his eflame.

baransu added a commit that referenced this pull request Apr 10, 2017
@baransu baransu self-assigned this Apr 14, 2017
@baransu
Copy link
Contributor

baransu commented Apr 15, 2017

I've added epl_timeline_EPL plugin serving state over WebSocket's timeline-info topic.
I'll be working on UI part of that.

/cc @michalslaski @arkgil I would really appreciate your review on my Erlang code 🙂

@baransu
Copy link
Contributor

baransu commented Apr 15, 2017

You can checkout this version. Basic functionality is done, only some UI improvements and of course tracker changes if required :)

/ cc @Hajto

@arkgil
Copy link
Contributor

arkgil commented Apr 24, 2017

And to answer your question - yes, we retrieve them all, but we can't capture changing state step by step if messages come too fast. Because messages can come to process and change its state after we receive trace message but before we make sys:get_state call.

@arkgil
Copy link
Contributor

arkgil commented Apr 24, 2017

One more question - is it possible to show whole message received by the process? I can't find a way to extend the column with messages.

@baransu
Copy link
Contributor

baransu commented Apr 24, 2017

No right now. I'll add this message in state's header because side panel has limited space for that.

@baransu
Copy link
Contributor

baransu commented Apr 24, 2017

I've moved epl_timeline_observer.erl logic into epl_timeline.erl so it's more consistent.
I'll try to retrieve observed node pid to change input method as well as full message text 🙂

JSON = epl_json:encode(#{timelines => T}, <<"timeline-info">>),
handle_info({data, _, Data}, State = #state{subscribers = Subs, timelines = Times}) ->
{Timelines, NewState} =
lists:foldl(fun({Pid, {P, PidList, Changes}}, {Acc1, Acc2}) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This anonymous function is huge, so it might be a good idea to move it to private function in the module :)

JSON = epl_json:encode(#{timelines => T}, <<"timeline-info">>),
handle_info({data, _, Data}, State = #state{subscribers = Subs, timelines = Times}) ->
{Timelines, NewState} =
lists:foldl(fun({Pid, {P, PidList, Changes}}, {Acc1, Acc2}) ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rename Acc1 and Acc2 so that the reader knows what do they refer to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that was already done by @baransu.

@baransu
Copy link
Contributor

baransu commented Apr 28, 2017

@arkgil I'm retrieving node part of pid from kernel's application_master on observed node and then using it during the add WS message. I've also split input into two separate parts for second and third part of the pid. Waiting for your feedback 😊

@michalslaski
Copy link
Member

@baransu I tried the new view and I like it very much. The UX of the timeline feature is intuitive and easy to figure out.

@Hajto I tried tracing the mnesia_locker process and noticed that throughput of messages reported in the dashboard view increased to over 45,000 msg. The system is not processing any Mnesia transactions, so most of the internal traffic is between epl_tracer and mnesia_locker processes. Is it expected?

@Hajto
Copy link
Contributor Author

Hajto commented Jun 12, 2017

@michalslaski I am sorry for inactivity. It is not expected to ignore those messages. I will have to investigate it more.

@michalslaski
Copy link
Member

No worries, good to have you back :)

@baransu
Copy link
Contributor

baransu commented Jun 19, 2017

I've added resizable panes with state and messages so when messages are to long or you want to focus on state better it's now easier 🎉

@Hajto what is progress on issues with to many messages?

@baransu baransu changed the title [WIP] Timeline Tracking Timeline Tracking Jun 21, 2017
@mkacper mkacper removed this from the 0.7 milestone Jun 21, 2017
@baransu baransu added this to the 0.8 milestone Jun 22, 2017
@baransu
Copy link
Contributor

baransu commented Jun 22, 2017

@michalslaski @mkacper
I've fixed mailbox arrow navigation so right now moving through large list of messages by up/down arrows should super smooth ☺️. Waiting for your review. Let me know if you have any concerns or ideas for improvements.

@mkacper
Copy link
Contributor

mkacper commented Jun 23, 2017

@baransu moving through mailbox by up/down arrows works perfectly but if you want to see details of a message -> state pair you have to click on a particular message. Maybe it would be easier to use when you can hit enter on a highlighted message to see the details or we can just automaticly switch the detailed view when moving through the mailbox? What do you think?

@baransu
Copy link
Contributor

baransu commented Jun 23, 2017

It should automatically select message on which you're on. Did you run yarn (or any make rule including it) after pulling?

@mkacper
Copy link
Contributor

mkacper commented Jun 23, 2017

First I used only yarn start but after your comment yarn && yarn start and now it works as you said. Sorry for the confusion :) Well done! 👍

@baransu
Copy link
Contributor

baransu commented Jun 23, 2017

@mkacper It was because of old version of react-virtualized (very effective lib for large lists, grids etc) we're using for displaying mailbox 🙂

@michalslaski michalslaski merged commit e9ffe23 into erlanglab:master Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants