Feature Request: add Telemetry events for `assign_async`

Hey Andy!

This looks like a very welcome addition. Is there a chance that an issue in the LV repo would get a better attention to the matter?

Hi! Something in that direction sounds like a reasonable ask!

I’m commenting to point out that depending on what you want to measure, the actual instrumentation needs to happen at different places/points in time.

The other telemetry events are centered around callbacks defined in the Phoenix.LiveView module. So they measure your callback implementations.

The assign_async function is a convenience built on top of assign, AsyncResult, Task, start_async and the handle_async callback.

One might be interested in measuring:

• How long the Task took. This relates to the function you pass to assign_async or start_async.
• How long the handle_async callback took. This measures what is done after the task returns.
• The total time from start to new values assigned.

If this doesn’t get added to LiveView, or in any case before it does, you could add your own telemetry events by implementing your own version of assign_async which emits events at the points you care about. You could then update your myapp_web.ex project file to shadow LiveView’s function with yours an with this small change add instrumentation to all your LiveViews without touching their source code.

This forum is the recommended venue for proposals like this. The GitHub issue tracker is used for bugs.

See

• phoenix/CONTRIBUTING.md at main · phoenixframework/phoenix · GitHub
• phoenix_live_view/.github/ISSUE_TEMPLATE/config.yml at main · phoenixframework/phoenix_live_view · GitHub

@rhcarvalho thanks for your thoughtful reply!

I agree that it would be possible to implement my own version of assign_async that could track the time the Task takes to run, in fact, the NewRelic agent already implements NewRelic.Instrumented.Task that I think would do the trick.

I was nervous about this approach because I am going to end up duplicating core framework behaviour that I would prefer to avoid, and also that I would need to make sure that everyone in the team migrates to the instrumented version.

With that in mind, you mention:

You could then update your myapp_web.ex project file to shadow LiveView’s function with yours and with this small change add instrumentation to all your LiveViews without touching their source code.

This sounds very interesting and could be a good option, as it solves the second problem.

Where can I find an example of this “shadowing” approach? My background is in Ruby so I’m used to patching open classes, I didn’t know that was possible in Elixir?

Hey, @andypearson I share a hackish starting point for you to play with:

1. Create a module with your new code, which can reference the Phoenix.LiveView implementation and augment it:

defmodule MyAppWeb.Helpers do
  require Phoenix.LiveView

  def assign_async(socket, key_or_keys, func, opts \\ []) do
    dbg(binding())
    Phoenix.LiveView.assign_async(socket, key_or_keys, func, opts)
  end
end

2. Update your “MyAppWeb” module with the override:

defmodule MyAppWeb do
  #... existing code ...

  def live_view do
    quote do
      use Phoenix.LiveView

      # ... existing code ...

      # Override assign_async/3 and assign_async/4
      import Phoenix.LiveView, except: [assign_async: 3, assign_async: 4]
      import MyAppWeb.Helpers, only: [assign_async: 3, assign_async: 4]

      unquote(html_helpers())
    end
  end
  
  #... existing code ...
end

Relevant docs: Kernel.SpecialForms — Elixir v1.20.2

Don’t let me discourage you to pursue changes in Phoenix/LiveView. Considering assign_async is a convenience on top of other primitives, I think telemetry events targeting those would be more generally applicable and solve for both assign_async and start_async use cases.

Of those I’d guess Task instrumentation would be the most interesting. I haven’t used NewRelic with Elixir, maybe making sure your tasks are instrumented would be the biggest win?

@rhcarvalho I haven’t had a chance to play with this yet, but thanks so much for the detailed write up.

I’ve never thought about using import except: to change what functions get pulled in from a “core” module. Really nice!

I think the approach shown by @rhcarvalho is a good solution. I’m not against adding telemetry events, but I’m hesitant because the async operations we have are assign_async, start_async and stream_async, and adding separate events for all three feels a bit much? But if we’d only have them for handle_async, it would also feel incomplete, although it would fit to other events being tied to callbacks (mount, handle_event). So yeah, maybe explicitly wrapping when necessary is the best approach for now.