6.0.1. breaks in GraalVM native-image about timbre HOT 11 CLOSED

Hi, I raised the question around this in the Clojurians slack after running into this issue, and 6.0.2-SNAPSHOT has fixed it for me. Thanks for the (extremely) quick turnaround on this!

from timbre.

In the context of #354, using a global counter would also allow the change to also work for cljs

from timbre.

The likelihood of a ~64bit collision is small with the number of callsites, especially since the callsite id also incorporates other sources of entropy like the namespace, line, arg form, etc.

But a counter would indeed be simpler. This was a case of complexity growing organically over time :-)
The initial choice of rand was quick+simple. Then the issue of determinism came up, then the GraalVM issue related to the determinism fix.

It was helpful of Michiel to re-check the original purpose: a counter is definitely the better choice now that we know we want determinism 👍

from timbre.

Hi @borkdude Michiel, thanks so much for the clear report and solution! Apologies for the trouble.

Could you please confirm that [com.taoensso/timbre "6.0.2-SNAPSHOT"] now works correctly on Graal?
Will deploy 6.0.2 final as soon as I get your confirmation.

Relatedly, it'd be great to get a Graal compile included in Timbre's tests (and in my other projects) but I've never used Graal myself. Would you maybe be able to point me in the direction of a good example of how to do this?

Cheers!

from timbre.

@dekelpilli Thanks a lot for the confirmation Dekel! Apologies for the trouble.

[com.taoensso/timbre "6.0.2"] is now up on Clojars 👍

Will keep this issue open until we have some kind of Graal unit test included (or we rule out that possibility).

from timbre.

Fix looks good to me. Why was the random number needed in the log macro in the first place?

from timbre.

@borkdude Good question.

Basically we want a unique id for each logging call site (/ individual logging form).
These ids can be useful for rate-limiting, etc.

But the arguments for a logging form aren't necessarily unique, so to uniquely identify each form we can use something like a counter or random number/uuid/gensym/etc.

Hope that makes sense?

from timbre.

Yes. So a global counter would have been sufficient I guess. (rand) or (.nextDouble ...) can give you twice the same number (theoretically), a strictly increasing counter doesn't, although there the issue might be that eventually you will have to move to BigInteger: (inc' Long/MAX_VALUE)

from timbre.

Ah, logging call site: those are fairly limited, so I would not expect that to hit BigInteger ever :)

from timbre.

Thanks @ptaoussanis !

from timbre.

Closing this issue, but have opened #361 re: adding Graal tests.

from timbre.