Add support for callback graph improvements and timing

[rpkyle]

This PR proposes to introduce additional context within the dev tools callback graph, as outlined previously in plotly/dash#1179, specifically

• a new callback_context.record_timing method is added, which permits capturing callback execution duration and adding it to Server-Timing headers (more info here), as seen in figure below

Most of the changes are due to updates within dash-renderer based on #1179, including:

• viz.js is replaced with cytoscape.js via react-cytoscape; the callback graph is now interactive, and its layout/organization (e.g. when dragging/reordering nodes) persists throughout the session
• CallbackGraphContainer now subscribes to layout and paths so it can introspect the current state
• new reducers are added to compute profiling information and report all changed props (not just inputs)

Sample code (included within a callback), and a snapshot of the network pane illustrating callback profiling data within the Server Timing headers:

    Sys.sleep(0.1)
    app$callback_context.record_timing('task_1', 0.1, 'The first task.')
    
    Sys.sleep(0.7)
    app$callback_context.record_timing('task_2', 0.7)
    
    Sys.sleep(0.2)
    app$callback_context.record_timing('task_3', 0.2, 'Cleanup task.')

To do:

• restrict invocation of callback_context.record_timing to callbacks, as with app$callback_context

Closes #223.

[rpkyle]

@Marc-Andre-Rivet I've completed my investigation, and as I discussed with @alexcjohnson, the request object is not easily modified within the handler (like Dash, it's an R6 class -- entitled Request -- and environments/bindings can be locked).

If you try to add a field on the object in place of what we were doing previously, i.e. app$server$set_data("timing-information"), you are likely to see this error:

Browse[1]> request$timing_information <- "test"
Error in request$timing_information <- "test" : 
  cannot add bindings to a locked environment

Similarly, unlike Response, there is no way to append_header:

... and you cannot force one in there:

Browse[1]> request$headers$Server_Timing <- "test"
Error in (function ()  : 
  unused argument (base::quote(list(c("text/html", "application/xhtml+xml", "application/xml;q=0.9

An alternative I was considering is to hash the request object, then use the hash as a way to uniquely identify timing information within self$server$get_data("timing-information"), assuming the hash is reliably unique and not mutable. We could index by the hash, or index by URL and store the hash within the entry. The former feels "safer" since you could potentially have interleaved requests which are processed out of order for the same resource, if Alex's concerns given concurrent asynchronous requests are warranted.

We already pull in digest, which has a function for computing the MD5 sum:

digest::digest(request, "md5")

I did test twice, and upon initial request the hash was unique:

Browse[1]> digest::digest(request, "md5")
[1] "d9da9c5617b829a5148351b27302f7fb"

Browse[4]> digest::digest(request, "md5")
[1] "c1b00e7fdb85aeba08a1f6f32767453a"

I haven't verified that the hash is identical between self$server$on('before-request', function(server, request, ...) and self$server$on('request', function(server, request, ...), but I'd guess that it should be.

[rpkyle]

As I discussed with @alexcjohnson and @Marc-Andre-Rivet separately, I think we're going to revert to the original implementation which doesn't fuss around with evaluating functions in other environments (and setting attributes on the request object). I'll also write a test with three callbacks that are somewhat slow-running, to attempt to validate that the information recorded during a particular callback's execution is correctly matched within the server's data store.

The appropriate long-term solution is likely to submit a PR to provide an optional request context within reqres upstream, but that's out of scope for the current release cycle. We can reconsider this alternative at some point in the future.

[rpkyle]

As discussed with @chriddyp, @Marc-Andre-Rivet, @alexcjohnson, I'll finish up with this PR and defer the integration test we discussed to the next DE release cycle. I'll open an issue to ensure it remains a priority and link back her.

[alexcjohnson]

@rpkyle this looks good! Agreed that we can defer testing the complex parallel callback timing / context test until later, but it would be nice to include a simple test that timing info makes its way to the response - just a straight port of https://github.com/plotly/dash/blob/dev/tests/integration/devtools/test_callback_timing.py, then I think we'll be good to go!

[rpkyle]

@rpkyle this looks good! Agreed that we can defer testing the complex parallel callback timing / context test until later, but it would be nice to include a simple test that timing info makes its way to the response - just a straight port of https://github.com/plotly/dash/blob/dev/tests/integration/devtools/test_callback_timing.py, then I think we'll be good to go!

Right, sorry for not including that here. I would not merge this without even a very basic test 😬 I'll handle that today. Thanks for taking a second pass!

[rpkyle]

@rpkyle this looks good! Agreed that we can defer testing the complex parallel callback timing / context test until later, but it would be nice to include a simple test that timing info makes its way to the response - just a straight port of https://github.com/plotly/dash/blob/dev/tests/integration/devtools/test_callback_timing.py, then I think we'll be good to go!

added in e0c0841

[alexcjohnson]

Non-blocking, but in principle this can be dashr_server rather than dashr, since you don't need the browser, just a server to make a request to.

[rpkyle]

Thanks for the heads-up, fixed in 9c8566e.

[alexcjohnson]

Looks great! 💃