-
Notifications
You must be signed in to change notification settings - Fork 18
Feature: trace diffs sent to browser #801
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since some of the code will change after the refactoring of traces I think it is ok. (only left some nitpicks). Works well. Good Job 🐬
Enables tracing for a specific process. | ||
""" | ||
@spec process(pid(), flags :: list()) :: {:ok, term()} | {:error, term()} | ||
def process(pid, flags) when is_list(flags) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: you could also add is_pid
It stores the trace in table associated with `pid` given in `Trace` struct. | ||
""" | ||
@spec insert(Trace.t()) :: true | ||
def insert(%Trace{} = trace) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about adding
defgaurd is_trace(trace) when is_struct(trace, Trace) or is_struct(trace, DiffTrace)
@type continuation() :: TracesStorage.continuation() | ||
@type ets_table_id() :: TracesStorage.ets_table_id() | ||
@type table_identifier() :: TracesStorage.table_identifier() | ||
@type trace() :: Trace.t() | DiffTrace.t() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use LiveDebugger.API.TracesStorage.trace()
?
trace_diffs = Keyword.get(opts, :trace_diffs, false) | ||
|
||
match_spec(node_id, functions, execution_times) | ||
match_spec(node_id, functions, execution_times, trace_diffs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just do this
match_spec(node_id, functions, execution_times, trace_diffs) | |
match_spec(node_id, functions, execution_times) | |
|> maybe_attach_diff_spec(trace_diffs) |
In this PR I introduced a new trace associated with LiveView diffs that are sent to the browser from LiveView via WebSocket. They can be accessed via global traces view, after selecting corresponding option in the filters form
Traces as a structure needs some unification, but since this PR is already big, I decided to refactor it in the next PR
Screen.Recording.2025-10-16.at.12.14.19.mov