Allow hook for LiveDashboard historical data at client connection

So, I’m loving LiveDashboard and learned a bit about telemetry, but still fairly new to LiveView for what it’s worth. The most frustrating part of the LiveDashboard experience for me is that, whether for demoing or using the dashboard for actual debugging, when you first open it up, there’s no data. Now, I know it’s out of scope for LiveDashboard to handle storing historical info from telemetry, and I have the storage of it solved easily using a ring buffer in a genserver that I already need running anyway for telemetry_poller to aggregate metrics within a tick on my customized metrics, but there are no hooks into LiveDashboard for populating a new client with a limited amount of history so that new clients can always have a little bit of history right away without waiting.

I opened this issue looking for maintainer input on the best way to structure the hook, after José indicated he was open to accepting a hook as a PR, but it’s not a priority for them right now so I’m looking for input here, both about interest level and if anyone has a good instinct from LiveView in general or LiveDashboard/Phoenix internals in particular on the best/most conventional structure and placement for such a hook. I started some work in a fork linked in the issue, but a) it’s not quite working correctly, and b) I suspect it might be preferred to have the client emit a new event (or use an existing client connection event) as the trigger, whereas right now I’m using a :telemetry.execute event as the trigger, and probably need the actual hook to be different/more conventional and similar to how the ordinary telemetry updates get dispatched to the client, but I think not as telemetry as I think each payload needs the be specifically crafted to be the history just for that client based on its moment of connection, as it will receive all future updates through ordinary means.

Look forward to hearing some feedback!

3 Likes

Generally I’d not use telemetry for retrieving historical data at all. There could be a config like:

config :live_dashboard, MyLiveDashboard,
  historical_data: %{
    [:my_app, :repo, :query] => {MyStorage, :historical_metric_data, []}
  }

Which would make the dashboard run MyStorage.historical_metric_data([:my_app, :repo, :query]) when it comes to rendering a chart for [:my_app, :repo, :query].

1 Like

I like that API! And yeah, the idea wasn’t to use telemetry for retrieving it, but only as the trigger for a hook, but I think I like your idea better since it should involve no overhead when there’s no config present, whereas mine involved a receive call and a short timeout… I’ll start playing with that next, thanks @LostKobrakai and welcome any other thoughts of course also.

Presumably in above example if the argument list were non-empty, then historical_metric_data would have arity > 1 ? I suppose having an additional argument that could be a function passed in or something could make sense. Thanks again!

I’d basically prepend the event name to the args of the config, so the actual function would need an arity of +1 to the length of arguments in the config.

Oh so you think instead of MyStorage.historical_metric_data([:my_app, :repo, :query]) it would call MyStorage.historical_metric_data(:my_app, :repo, :query) and +1 to arity for each additional argument in the config list? Yeah not sure but that makes sense/is how I first read your code before I noticed it as a list… I remember seeing a convention like this in some other library recently but blanking what it was maybe something in ecto or telemetry_poller, I forget

Nope, I meant it like your first example: prepend the full event name as the first argument.

:+1: apologies, that’s what I’d tried to convey in my first reply which made me think you were correcting a misunderstanding there.

I also wonder though, your example above looks to use Application config, but it looks like the only config live_dashboard currently takes is in the arguments in router to live_dashboard, so guessing we’d pass in historical_data as an additional optional config there, e.g.:

  scope "/live-dashboard" do
    pipe_through [:browser, :admin]
    live_dashboard "/",
      metrics: MyApp.Telemetry, 
      historical_data: %{
        [:my_app, :repo, :query] => {MyStorage, :historical_metric_data, []}
      }
  end

That would probably work as well. Skipping the app env when possible is usually a good thing.

Hopefully also can treat the keylist as a prefix rather than an explicit match so for large numbers of metrics you can just put in the namespace they’re in once, e.g. [:my_app, :repo] and get all events that start with that… probably what you intended but just stating here as I’m toying with starting this in case anyone disagrees.

For anyone interested, a quick v1 of above is pushed up here https://github.com/bglusman/phoenix_live_dashboard/tree/historical_data and I think this may work but had to break before I could start real testing, code reviews or testing suggestions welcome!

OK, pushed up a few more changes, this is working on the backend but there’s two different initialData functions in assets/js/metrics_live/index.js and I think all that’s needed is for one or both of them to be wired up to check for the hidden history div I added and parse in any spans there the same way it brings in data on liveView updates… if by any chance @LostKobrakai since it looks like you did most of the switch over from charts.js to uPlot you know the easiest/cleanest way to do that, I think that may finish this off, and I’d love your input on the implementation besides that, awesome, if you don’t have time, thanks for your help earlier anyway as it feels close now!

1 Like

I’m wondering if you actually need to send the historical data separately to the frontend? The JS doesn’t really care where the data is coming from (unless it should be marked differently somehow).

This is because they’re for different types of charts/metrics, see the classes around them.

I didn’t think so initially, but passing it in as data assigns doesn’t work/is ignored, perhaps because of the initialData function? I can play around with later or you’re welcome to/I can try and package up a little app that’s using this integration to demo/play with more easily extracted from my actual app’s usage if you think useful. I don’t think it should be marked differently, though its an interesting question, I could see perhaps wanting a marker at the dividing line, but I think that would be a future refinement if so.

Actually, you’re right, though the way I have it working without modifying JS feels a bit dirty/maybe you have an idea for a better place, it works if I do a send_update immediately after the render of live_component like so:

   <%= if @metrics do %>
      <div class="phx-dashboard-metrics-grid row">
      <%= for {metric, id} <- @metrics do %>
        <%= live_component @socket, ChartComponent, id: id, metric: metric %>
        <% send_update(ChartComponent, id: id, data: history_for(metric, @historical_data)) %>
      <% end %>
      </div>
    <% end %>

but putting a side effect there feels a bit dirty, perhaps there’s a better hook available to call immediately after render? Dunno… but locally its working beautifully this way so that’s a start…

I would’ve looked at doing it within mount of ChartComponent, so data is filled from the start.

oh yeah that makes more sense :upside_down_face: I’ll try that now, thanks!

Hmm, but I dont have access to the assigns in ChartComponent mount, I thought maybe they were in the socket on socket.assigns but that just has flash and myself at mount…

In any case, working version is pushed up, may try and refactor from there to use mount and/or add some tests or something but still welcome code reviews from anyone following this before I open PR, to both save José and company time and also increase chance of it being accepted!

Link again is https://github.com/bglusman/phoenix_live_dashboard/tree/historical_data I’ll open a PR into my master branch if I can to avoid bothering the main project until ready but allow code review…

Found a small improvement noticing that live_component accepts a do block, so can add a do block and call my send_update there and that works well enough… if I don’t hear ideas from anyone else I think I’ll accept that as reasonable place to do after-render work for a live component, since it is designed to take that block…

OK official PR is open now, added tests and more docs, hopefully its up to snuff but for anyone interested https://github.com/phoenixframework/phoenix_live_dashboard/pull/122

1 Like

So over the weekend Jose reviewed the PR and then closed it with some more feedback around some cases I hadn’t considered (I also may have gotten on his badside with some confusion/ignorance about Github alerts etc :frowning: , but ahh well, live and learn…). If anyone is using LiveDashboard with more complex metric types, tags and explicit units etc that they might like to enable history for, please let me know, as I don’t have any of these myself so I’d be making up fictitious use to try and cover the edge cases he mentions… I think the main points he raises can be handled by just passing the entire metric to the callback instead of just passing the name, but if I’m reading his comments correctly the larger issue is reworking the guide/example implementation I provided to handle those other situations with good documentation and understanding when the user might not want to play back certain events from history (this part didn’t make a ton of sense to me/maybe :telemetry_poller is already saving up some events for later playback or there’s some other side effect I’m missing?).

@LostKobrakai if you have any interest or bandwidth to discuss/weigh in on above, or @axelson @fhunleth or @sfusato since you all expressed interest (apologies if I’m failing/breaking any kind of etiquette by @'ing you guys here, no obligation to respond, apparently I’m clueless how to internet about these things :upside_down_face: ), or anyone else who has ability/desire to perhaps run my fork locally or in production and help refine the other kinds of charts/metrics José brings up to help solidify implementation and docs, contact me here or email me at brian@ my last name .me

(I pushed up the minimal changes I understand from José’s feedback into code and docs, but looking for further understanding from others on both!)