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 dissapears in Calva in versions from 2.0.437 with cidr-nrepl 0.47.1 - possible cause is logjam #18

Closed
ieugen opened this issue Apr 12, 2024 · 12 comments

Comments

@ieugen
Copy link

ieugen commented Apr 12, 2024

Hello,

This issue is mostly to discuss options.

We are using Calva and we noticed logs are no longer shown from a day or two.
We are using tools.logging facade with https://github.com/amperity/dialog as a SLF4j implementation.

We are directing all logging capture to slf4j.
It seems that logjam tries to do the same and this creates an issue where no logging is shown.

We got the recommendation to use an older version of cider but IMO that will work only for a limited amount of time.
How many years can we use an older version of cidr-nrepl before we are forced to upgrade ?!

We have traced this to a Calva upgrade https://github.com/BetterThanTomorrow/calva/blob/published/CHANGELOG.md#20437---2024-04-11

See discussion https://clojurians.slack.com/archives/CBE668G4R/p1712950069143409

I am still investigating the issue - we might be able to remove a dependency and make things work but I have an un-easy fealing about logjam taking over logging from the app.
IMO each app should be in control of it's logging.

This might be related to #12 as well ?!

@vemv
Copy link
Member

vemv commented Apr 12, 2024

Logjam is an opt-in feature related to object logging (as opposed to capturing text streams) that most likely isn't used by Calva at all.

You probably should be looking at https://github.com/clojure-emacs/cider-nrepl/blob/31a3d02e13ae1daff0637330c27d83f5943a8486/src/cider/nrepl/middleware/out.clj instead

How many years can we use an older version of cidr-nrepl before we are forced to upgrade ?!

We try hard to keep backwards compatibility, that's all I can say.

Cheers - V

@vemv
Copy link
Member

vemv commented Apr 12, 2024

@ieugen worth noting, cider-nrepl is a composition of middleware. One can remove the out middleware entirely and probably things will keep working - worth a shot

@bbatsov
Copy link
Member

bbatsov commented Apr 13, 2024

Let's also cc @PEZ here, as might need more context about recent changes made in Calva that might be related to this.

@bbatsov
Copy link
Member

bbatsov commented Apr 13, 2024

I did notice that Calva used an ancient version of cider-nrepl until recently - 0.28.5. So much has happened since then. Btw, I'll note the "out" middleware is an opt-in feature as well. Not sure if Calva is using it at all.

@ieugen
Copy link
Author

ieugen commented Apr 13, 2024

Thank you for looking at this as well @bbatsov .

I will try to make a reproducer repo.
See if just using amperity dialog logging is enough to reproduce.
I suspect it is since it does come with libs that try to redirect logging to slf4j: https://github.com/amperity/dialog/blob/main/deps.edn

I tried latest Calva with a smaller project that used tools.looging and slf4j-simple and it worked ok.

 :deps
 {org.clojure/clojure {:mvn/version "1.11.1"}
  org.clojure/data.json {:mvn/version "2.4.0"}
  org.slf4j/slf4j-api {:mvn/version "2.0.7"}
  org.slf4j/jul-to-slf4j {:mvn/version "2.0.7"}
  org.slf4j/jcl-over-slf4j {:mvn/version "2.0.7"}
  org.slf4j/log4j-over-slf4j {:mvn/version "2.0.7"}
  io.aviso/pretty {:mvn/version "1.3"}
  aero/aero {:mvn/version "1.1.6"}}

@PEZ
Copy link

PEZ commented Apr 15, 2024

👋

I should have testet much more before I upgraded. But I ran with the newer version for long and everything seemed to work. But I am not every user, so this part I missed. I am not all that clear about what “this part” even is, tbh.

Not sure if Calva is using the out middleware. The user can opt-in on the out-subscribe op, if that's part of it? That's the extent of it, if so, I think. And we've been doing that for a while. The update of the default nrepl + cider-nrepl dependencies is unrelated to this. And we haven't changed anything around our use of nrepl + cider-nrepl for quite a while.

@vemv
Copy link
Member

vemv commented Apr 15, 2024

Thanks much for the useful info!

Yes, out-subscribe is part of out. If you've performed out-subscribe at least once, then the following is performed:

https://github.com/clojure-emacs/cider-nrepl/blob/31a3d02e13ae1daff0637330c27d83f5943a8486/src/cider/nrepl/middleware/out.clj#L158

Which means that *out* output might 'disappear' in that it won't be visible anywhere but from subscribers.

(Then again, you indicate that you are subscribing - I assume that you make good use of these subscriptions?)

We might need a repro to proceed.

@ieugen
Copy link
Author

ieugen commented Apr 15, 2024

So I tried to create a repro but did not maanged to do that yet.

I have created a project with a deps.edn

{:paths ["src"]
 :deps {org.clojure/clojure {:mvn/version "1.11.2"}
            org.clojure/tools.logging {:mvn/version "1.3.0"}
            com.amperity/dialog {:mvn/version "2.0.115"}}}

And a src/user.clj

(ns user
  (:require [clojure.tools.logging :as log]))
(log/info "Hello")

And this works,

  • I evaluate the ns and I see the logs in calva.output
    image

Will try to figure out why Calva does not work in our project .

@vemv
Copy link
Member

vemv commented Apr 15, 2024

Kudos for working on the repro.

I reckon that you should replicate how com.amperity/dialog is setup.

Perhaps what's happening is that it starts working (i.e. creating an *out* binding and printing to it) before cider-nrepl does?

https://github.com/amperity/dialog/blob/6089a3e1b79da23652deb8f57b2c04b3667dd5d2/src/clojure/dialog/output/print.clj

@ieugen
Copy link
Author

ieugen commented Apr 15, 2024

Did not manage to reproduce yet but seems to be something that happens in our code.
We use polylith and have a lot of deps there => this makes it a bit more difficult to spot the issue - happens in :dev .

I started an nrepl server in another project using the classpath computed for :dev alias in our project - 45kb of text :) .
Unfortunately I did not manage to reproduce the issue.

Can you please help me check I am doing things ritght?

  • I get the class path in my project by running this in workspace/ :
clojure -Sdeps '{:deps {nrepl/nrepl {:mvn/version,"1.1.1"},cider/cider-nrepl {:mvn/version,"0.47.1"}}}' -Spath -M:dev 
  • I copy the output text and run the following command:
clojure -Scp CP_FROM_ABOVE_HERE  -M:dev -m nrepl.cmdline --middleware "[cider.nrepl/cider-middleware]"  

Would be nice to load CP from file.

@vemv
Copy link
Member

vemv commented Apr 15, 2024

For being extra sure, you can copy the exact classpath from ps aux. Once you save it to a file you can:

clojure -Scp $(cat the_classpath) -M:dev -m nrepl.cmdline --middleware "[cider.nrepl/cider-middleware]"

@vemv
Copy link
Member

vemv commented Apr 24, 2024

Feel free to continue the conversation.

@vemv vemv closed this as completed Apr 24, 2024
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

No branches or pull requests

4 participants