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

Logging in notebook #437

Merged
merged 73 commits into from
Jan 17, 2022
Merged

Logging in notebook #437

merged 73 commits into from
Jan 17, 2022

Conversation

fonsp
Copy link
Owner

@fonsp fonsp commented Sep 15, 2020

Schermopname.2021-04-02.om.19.16.30.mov

Adds support for Julia logging: @info, @warn, @debug, @error and custom log messages.

(This does not relay the terminal - that is a different subject.)

TODO

  • The log should "pop out" of the overflow:hidden box when you click on it.
  • Virtualized logs (only load a viewport worth of logs into the DOM)
  • layout might not work if the cell has many lines but cell input is hidden
  • Selecting cells is difficult if there are also logs
  • Option to hide logs (Future TODO: save that propery in a comment @dralletje
  • Narrow-screen layout still needs improvement
  • show kwargs
  • rich output
  • use a lighter background for the kwargs
  • State diffing becomes slow when there are lots of logs
  • Don't also print logs to terminal?
  • Limit a crazy number of logs?
  • Dark mode

Test notebook

https://github.com/fonsp/disorganised-mess/blob/main/logging.jl

Try it out!

julia> ]
pkg> activate --temp
pkg> add Pluto#logging
julia> import Pluto; Pluto.run()

@fonsp

This comment has been minimized.

@fonsp
Copy link
Owner Author

fonsp commented Sep 15, 2020

image


# we put this in __init__ to fix a world age problem
function __init__()
Logging.global_logger(PlutoLogger())
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be better just to use Logging.with_logger() in some context which surrounds the user's code?

@c42f
Copy link
Contributor

c42f commented Sep 16, 2020

This is super cool!

Let me know if there's any help you'd like in integrating this, or subtle questions about the logging system. I'm also happy to do a call if that's helpful? (Note I am in AEST=UTC+10)

I can help out with the Julia side of the code if necessary. Figuring out the most correct way to format the log messages might be the hard bit.

@fonsp
Copy link
Owner Author

fonsp commented Sep 18, 2020

Right now the messages only appear if you had the notebook open while the logs happened, so the logs are lost when you refresh. Instead, logs need to be stored in an array inside the cell's state.

Our current state management is not very flexible - implementing this means adding a couple of new message types, handlers and state updates, and some careful thinking to ensure that the state is never corrupt. We were planning to update the state management system soon, so I think it would be nicer to wait for that.

@fonsp fonsp added this to the 🛶 v0.13 milestone Sep 19, 2020
@c42f
Copy link
Contributor

c42f commented Sep 21, 2020

Sure, no problem at all.

What are your thoughts for UI when log records arise from code defined outside Pluto, or from functions defined in other cells?

I can think of a couple of options:

  • Dynamically assign them to the line from which they originate (Note that this requires taking a stack trace for every message which will appear in the UI, which can be done by the logging backend. Could be expensive; though UI elements are also expensive so we might be ok :-) )
  • Plot them on a line separate from the code lines of the cell. This can be done with the file and line keys that every log record comes with.

I kinda favor trying the dynamic option and seeing how it pans out.

@fonsp
Copy link
Owner Author

fonsp commented Sep 21, 2020

The dynamical approach is a good idea! Could you show me how to get the stacktrace from a log?

Before your idea, I was thinking that every external log message could appear as a vertical line, since we know it's temporal position, but not the line number. Something like this:
image

The log would show in the cell that was most recently running.

Finding the backtraced line number is better of course, but we might still need this as a fallback? For async external code for example? Or can those also be backtraced?

@c42f
Copy link
Contributor

c42f commented Sep 29, 2020

Could you show me how to get the stacktrace from a log?

The idea would be to add a call to backtrace() from within handle_message(::PlutoLogger, ...) — this runs as a child function of the user's @info so the backtrace is directly accessible. From that backtrace, extract any info necessary and attach it to the log record as metadata before passing it to the frontend.

For async tasks, note that loggers are inherited from the parent task if Logging.with_logger was used. So as long as you install PlutoLogger before creating those tasks, the same considerations will apply. (Or if you're using global_logger that will be the fallback for any tasks which don't have a local logger installed.)

@fonsp fonsp removed this from the 🛶 v0.13 milestone Jan 26, 2021
Base automatically changed from master to main March 11, 2021 21:33
@LinusSch
Copy link

Quick thought on the UI: I would prefer the dots from the same iteration of the loop to be vertically aligned. That way there is a little bit more information, directly visible.

I guess this idea breaks the case where multiple logging events happen on a single line of a single iteration of the loop, but this could be solved by having two or three sectors with different colors as needed. Exactly how many logging events are happening at a single point in the code is rarely as useful as quickly realizing whether several events happening in a loop are happening on the same or on different iterations (in my experience, which is more limited than yours, so feel free to correct me if I'm wrong).

If this idea can work well or not probably depends on how you show the details of each event, that view may become unwieldy in cases where many events get bundled into a single dot. If so then the usefulness may not be worth the drawback.

@fonsp
Copy link
Owner Author

fonsp commented Apr 2, 2021

image

@fonsp fonsp added the wide audience This affects a wide audience of Pluto users and future Pluto users label Dec 13, 2021
@fonsp
Copy link
Owner Author

fonsp commented Dec 20, 2021

@dralletje Can you take a look at the state diffing performance problem in this PR? You had a solution right?

background: hsl(0deg 0% 100% / 81%);
}

/* @media screen and (max-width: calc(700px + 25px + 6px + 150px)) { */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this commented on purpose? @fonsp

@fonsp
Copy link
Owner Author

fonsp commented Jan 17, 2022

@dralletje Your recent commits (0e5f8ce and cc045d3) did the trick!

Benchmarks

Before:

Schermafbeelding 2022-01-17 om 20 36 50

After:

Schermafbeelding 2022-01-17 om 20 41 15

Script:

https://gist.github.com/fonsp/9551c8688f05b4baf21d3f242696dfcc

@fonsp fonsp merged commit 665d99a into main Jan 17, 2022
@fonsp fonsp deleted the logging branch January 17, 2022 20:27
@fonsp
Copy link
Owner Author

fonsp commented Jan 17, 2022

@fonsp fonsp added the logging About `@info`, `@warn`, etc label Mar 2, 2022
@fonsp fonsp mentioned this pull request Sep 27, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Concerning the HTML editor logging About `@info`, `@warn`, etc wide audience This affects a wide audience of Pluto users and future Pluto users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProgressLogging progress is not logged anywhere Progress for cells taking longer to execute
7 participants