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

Add live_component update telemetry event #2914

Merged
merged 5 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 33 additions & 14 deletions lib/phoenix_live_view/diff.ex
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,6 @@ defmodule Phoenix.LiveView.Diff do
{{pending, diffs, components}, seen_ids} =
Enum.reduce(pending, acc, fn {component, entries}, acc ->
{{pending, diffs, components}, seen_ids} = acc
update_many? = function_exported?(component, :update_many, 1)
entries = maybe_preload_components(component, Enum.reverse(entries))

{assigns_sockets, metadata, components, seen_ids} =
Expand All @@ -653,26 +652,46 @@ defmodule Phoenix.LiveView.Diff do
put_cid(components, component, id, cid)}
end

assigns_sockets =
if update_many? do
[{new_assigns, socket} | assigns_sockets]
else
[Utils.maybe_call_update!(socket, component, new_assigns) | assigns_sockets]
end

assigns_sockets = [{new_assigns, socket} | assigns_sockets]
metadata = [{cid, id, new?} | metadata]
seen_ids = Map.put(seen_ids, [component | id], true)
{assigns_sockets, metadata, components, seen_ids}
end)

assigns_sockets = Enum.reverse(assigns_sockets)
metadata = Enum.reverse(metadata)
update_many? = function_exported?(component, :update_many, 1)

component_data=
assigns_sockets
|> Enum.zip(metadata)
|> Enum.map(fn {{assigns, socket}, {cid, _id, new?}} ->
%{
assigns: assigns,
socket: socket,
cid: cid,
new?: new?
}
end)
Copy link
Member

Choose a reason for hiding this comment

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

This code can be very performance sensitive. Doing all of these traversals for telemetry purposes is going to be expensive. We can start with a single telemetry passing :component and :assigns_sockets as metadata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, I thought about passing metadata (the internal "variable") , but that would probably be leaking an internal structure, so I did this.
I'll change to just pass the assigns_sockets. On the stop event, do you think I should replace sockets or just add a new metadata field with the new sockets?

Copy link
Member

Choose a reason for hiding this comment

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

Just return the new sockets and the component, yeah.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 😄


telemetry_metadata = %{socket: socket, component: component, component_data: component_data, update_many?: update_many?}

sockets =
if update_many? do
component.update_many(Enum.reverse(assigns_sockets))
else
Enum.reverse(assigns_sockets)
end
:telemetry.span([:phoenix, :live_component, :update], telemetry_metadata, fn ->
sockets =
if update_many? do
component.update_many(assigns_sockets)
else
Enum.map(assigns_sockets, fn {assigns, socket} ->
Utils.maybe_call_update!(socket, component, assigns)
end)
end

component_data = sockets |> Enum.zip(component_data) |> Enum.map(fn {socket, data} -> %{data | socket: socket} end)

{sockets, Map.put(telemetry_metadata, :component_data, component_data)}
end)

metadata = Enum.reverse(metadata)
triplet = zip_components(sockets, metadata, component, cids, {pending, diffs, components})
{triplet, seen_ids}
end)
Expand Down
39 changes: 39 additions & 0 deletions test/phoenix_live_view/integrations/telemetry_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,45 @@ defmodule Phoenix.LiveView.TelemtryTest do
assert metadata.component == Phoenix.LiveViewTest.StatefulComponent
assert metadata.params == %{"op" => "boom"}
end

test "emits telemetry events for update calls", %{conn: conn} do
attach_telemetry([:phoenix, :live_component, :update])

{:ok, view, _html} = live(conn, "/multi-targets")

assert_receive {:event, [:phoenix, :live_component, :update, :start], %{system_time: _},
metadata}

assert metadata.socket
assert metadata.component == Phoenix.LiveViewTest.StatefulComponent
refute metadata.update_many?
josevalim marked this conversation as resolved.
Show resolved Hide resolved

assert [
%{
cid: cid,
assigns: %{id: _id, name: name},
socket: component_socket,
new?: true
},
_
] = metadata.component_data

assert is_integer(cid)

assert_receive {:event, [:phoenix, :live_component, :update, :stop], %{duration: _},
metadata}

assert metadata.socket
assert metadata.component == Phoenix.LiveViewTest.StatefulComponent
assert [%{socket: updated_component_socket}, _] = metadata.component_data

assert updated_component_socket != component_socket

render_click(view, "disable", %{"name" => name})

assert_receive {:event, [:phoenix, :live_component, :update, :start], %{system_time: _},
%{component_data: [%{new?: false, assigns: %{name: ^name, disabled: true}} | _]}}
end
end

describe "logging configuration" do
Expand Down
4 changes: 4 additions & 0 deletions test/support/live_views/components.ex
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ defmodule Phoenix.LiveViewTest.WithMultipleTargets do
def handle_event("transform", %{"op" => _op}, socket) do
{:noreply, assign(socket, :message, "Parent was updated")}
end

def handle_event("disable", %{"name" => name}, socket) do
{:noreply, assign(socket, :disabled, Enum.uniq([name | socket.assigns.disabled]))}
end
end

defmodule Phoenix.LiveViewTest.WithLogOverride do
Expand Down
Loading