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 CIDER Log Mode #3352

Merged
merged 1 commit into from
Jul 5, 2023
Merged

Add CIDER Log Mode #3352

merged 1 commit into from
Jul 5, 2023

Conversation

r0man
Copy link
Contributor

@r0man r0man commented Jun 12, 2023

Hello CIDER team,

this PR adds CIDER log mode. CIDER Log Mode allows you to capture, debug, inspect and view log events emitted by Java logging frameworks. The captured log events can be searched, streamed to the client, pretty printed and are integrated with the CIDER Inspector and Stacktrace Mode.

How to try it out?

Install the CIDER middleware

  • Checkout the cider-log branch of cider-nrepl
  • Install the middleware with lein do clean, inline-deps && lein with-profile +plugin.mranderson/config install

Install the CIDER Emacs mode

  • Checkout the cider-log branch of cider
  • Configure CIDER to use the middleware we installed above with:
(setq cider-injected-middleware-version "0.0.0")
(setq cider-required-middleware-version "0.0.0")

Using the Emacs mode

  • Go into the cider-nrepl project and M-x cider-jack-in
  • Run M-x cider-log to open the main menu
  • You will be asked to choose a log framework (select logback)
  • After selecting the framework, you will see a message that a log appender has been added to the framework and you end up in the main menu
  • Type in es to search log events, followed by s to perform a search
  • The *cider-log* buffer will open. This is initially empty but will receive log events once you start logging something
  • Open the cider.nrepl.middleware.log-test and run the tests to emit log events
  • Log events are now streamed to the *cider-log* buffer
  • Log events can be inspected RET, pretty-printed F, and opened in the stacktrace navigator E (if there are any exceptions)
  • That's it for the basic usage. You can do more like configure appenders and consumers and search events. Checkout the logging.adoc file for more information.

Open questions

  • I think we can move everything under the cider.log namespace to a separate repository. Similar to what we did with haystack. So we need a good name for it. Any ideas?
  • Right now the Emacs Mode is based on Logview Mode https://github.com/doublep/logview Logview Mode does the coloring and provides some additional functionality like narrowing the log events to certain types. I think it's quite useful but we can try to get rid of it, or make it optional.
  • I'm using Transient (which now comes with Emacs) for the menus. It is a nice way to select complex filters and re-apply them by bringing them up via the Transient history feature. So I would not like to drop that dependency.
  • Some of the transient menus are a bit repetitive, but it's the only way I could make the main cider-log menu working. I was thinking of even getting rid of the main menu and only use the cider-log-framework, cider-log-appender, cider-log-consumer and cider-log-event menus. On the other hand the main menu makes things more discover-able. What do you think?
  • Right now this works with Logback (which I am using) and Java Util Logging. There is some code for Log4j2 that passes some of the tests, but I couldn't make it working reliably. The issue seems that Log4j2 reloads its configuration when a new appender is added, and wipes out the changes. I tried most of the suggestions on Stackoverflow to get it working, but I'm giving up on this one. I already spent too much time on this. So, I propose to drop that code into a branch of the separated library. If anyone knows how to get it working, I'm willing to help, but I won't investigate this beast any further.

Thanks

Finally a big thanks to @rafaeldff who showed me the log capturing in his private devtools and supported me on this.


Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • [?] All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

Thanks!

If you're just starting out to hack on CIDER you might find this section of its
manual
extremely useful.

@r0man
Copy link
Contributor Author

r0man commented Jun 12, 2023

This PR is for the NREPL side: clojure-emacs/cider-nrepl#773

@r0man
Copy link
Contributor Author

r0man commented Jun 12, 2023

I will look into the failing tests soon ... (or once we decided what to do with Logview mode)

@bbatsov
Copy link
Member

bbatsov commented Jun 13, 2023

I think we can move everything under the cider.log namespace to a separate repository. Similar to what we did with haystack. So we need a good name for it. Any ideas?

We discussed this over Slack and agreed it's a good idea. Naming - pending. :-)

Right now the Emacs Mode is based on Logview Mode https://github.com/doublep/logview Logview Mode does the coloring and provides some additional functionality like narrowing the log events to certain types. I think it's quite useful but we can try to get rid of it, or make it optional.

It's fine. We can tweak this later if needed.

I'm using Transient (which now comes with Emacs) for the menus. It is a nice way to select complex filters and re-apply them by bringing them up via the Transient history feature. So I would not like to drop that dependency.

I haven't used it, but if it's built-in I don't have issues with it.

Some of the transient menus are a bit repetitive, but it's the only way I could make the main cider-log menu working. I was thinking of even getting rid of the main menu and only use the cider-log-framework, cider-log-appender, cider-log-consumer and cider-log-event menus. On the other hand the main menu makes things more discover-able. What do you think?

Both approaches have their pros and cos. I don't mind the main menu myself, but I also I rarely use menus myself. They are handy when exploring something new, though, that's why I always try to make them detailed. Again, it's not something very important right now. I'm a big believer in small iteration over trying to get everything perfect from the first try.

@vemv
Copy link
Member

vemv commented Jun 13, 2023

I think we can move everything under the cider.log namespace to a separate repository. Similar to what we did with haystack. So we need a good name for it. Any ideas?

We discussed this over Slack and agreed it's a good idea. Naming - pending. :-)

I propose Glass 🍺

One can view things (logs here) through a window, or magnifying glass, telescope... seems semantically close.

@@ -12,7 +12,7 @@
;; Maintainer: Bozhidar Batsov <bozhidar@batsov.dev>
;; URL: http://www.github.com/clojure-emacs/cider
;; Version: 1.7.0
;; Package-Requires: ((emacs "26") (clojure-mode "5.16.0") (parseedn "1.0.6") (queue "0.2") (spinner "1.7") (seq "2.22") (sesman "0.3.2"))
;; Package-Requires: ((emacs "26") (clojure-mode "5.16.0") (parseedn "1.0.6") (queue "0.2") (spinner "1.7") (seq "2.22") (sesman "0.3.2") (logview "0.16.1") (transient "0.4.1"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bbatsov I added this for now here to make the tests pass. Is there a better way to optionally add them and have it installed on CircleCI? I added transient here as well. It comes with newer Emacsen, but the tests running on the older versions need it installed as well.

Copy link
Member

Choose a reason for hiding this comment

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

For CI purposes you can just add deps to Eldev, but I think it's fine to have them here given that those are runtime deps. (e.g. seq is also built-in)

@r0man
Copy link
Contributor Author

r0man commented Jun 28, 2023

@bbatsov @vemv I'm trying to fix the lint issues. The last ones left are:

https://app.circleci.com/pipelines/github/clojure-emacs/cider/1575/workflows/8bca6016-7dde-4948-86f1-fdce5f5cdcf7/jobs/7916

[00:01.537]  Loading file `cider-log.el' before byte-compiling it...

[00:01.937]  
             In cider-log--do-search-events:
             cider-log.el:843:12: Error: `cider-log--do-add-consumer' is for interactive
                 use only.
[00:01.940]  
             In cider-log-event-next-line:
             cider-log.el:923:36: Error: `cider-log-event-show-stacktrace' is for
                 interactive use only.
[00:01.940]  cider-log.el:925:36: Error: `cider-log-event-inspect' is for interactive use
                 only.
[00:01.941]  cider-log.el:927:36: Error: `cider-log-event-pretty-print' is for interactive
                 use only.
[00:01.972]  
             In cider-log--ensure-initialized:
             cider-log.el:1112:50: Error: `cider-log--do-add-appender' is for interactive
                 use only.
[00:01.982]  Saving target dependencies to file `.eldev/28.2/target-dependencies.build'...

[00:01.983]  Failed to byte-compile `cider-log.el'
[00:01.983]  Finished erroneously on Wed Jun 28 07:37:50 2023

Do you know what is the best way to fix them?

Should I create a set of non-interactive functions for all of those? This seems a bit tedious.

@vemv
Copy link
Member

vemv commented Jun 28, 2023

Do you know what is the best way to fix them?

(IDK)

@bbatsov
Copy link
Member

bbatsov commented Jun 28, 2023

@vemv We've discussed this with @r0man over Slack.

cider-log.el Outdated Show resolved Hide resolved
@r0man
Copy link
Contributor Author

r0man commented Jun 29, 2023

@bbatsov Alright, I fixed the lint issues. Since I moved some things around, let me run with this for a couple of days and let's merge it next week.

@bbatsov
Copy link
Member

bbatsov commented Jun 29, 2023

Sure!

@r0man
Copy link
Contributor Author

r0man commented Jul 5, 2023

@bbatsov I did not notice anything suspicious in the last days running this, so I would say let's merge it?

@bbatsov bbatsov merged commit ada5df5 into clojure-emacs:master Jul 5, 2023
11 checks passed
@bbatsov
Copy link
Member

bbatsov commented Jul 5, 2023

Done!

@r0man
Copy link
Contributor Author

r0man commented Jul 5, 2023

Perfect. Thank you and @vemv once again!

@bbatsov
Copy link
Member

bbatsov commented Jul 5, 2023

@r0man Thank you for another fantastic contribution! 🙇

Btw, I noticed the keybindings a bit too late, but we can't really use something like C-c l, as such keybindings are reserved for users and no modes should utilize them. Try to find something with C-x or C-c M-/C-c C-x prefix instead.

@bbatsov
Copy link
Member

bbatsov commented Jul 5, 2023

Right now the Emacs Mode is based on Logview Mode https://github.com/doublep/logview Logview Mode does the coloring and provides some additional functionality like narrowing the log events to certain types. I think it's quite useful but we can try to get rid of it, or make it optional.

I didn't realize this dependency is only on MELPA (see #3365), which breaks the CIDER build on NonGNU ELPA. We might have to make this dep optional until that problem gets resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants