-
-
Notifications
You must be signed in to change notification settings - Fork 172
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
Provide line number info to CLJS Chrome dev tools, etc. #132
Comments
Hi Dom! Timbre logging calls are done via a macro, in part to capture available ns + line-number info. The appender call itself is intentionally a fn call for simplicity, flexibility, and expansion size. Expanding all appenders in place would balloon a ClojureScript source file quite quickly. The ideal solution is to convey the line number from the macro call to the appender (fn) call. That way you can include the line number in the log output. That's what the As noted there though, support for this is unfortunately still waiting on http://dev.clojure.org/jira/browse/CLJ-865 If 865 gets resolved, we can update the default Will keep this issue open. Haven't looked into this in a while so not sure if any other possibilities/hacks have since come up (esp. on the cljs side). I've got my hands full atm, but would be happy to hear ideas if you felt like doing the digging yourself? Cheers! :-) |
Hi Peter :) I missed that However, even with CLJ-865 resolved, I don't think that this will really achieve what I'm looking for, though please correct me if I'm wrong. Even if logs include correct line numbers, source maps will still point to Timbre, right? Also, is macro expansion size a real issue since logging macros can be elided in production? Though unrelated to the function vs. macro question, I prefer objects to be printed with |
Hey :-)
Source maps will still point to the appender, that's correct. Idea is if you have the line numbers in the log output, you don't need source maps. Actually, may be a way to support source maps too if we wanted that for convenience - haven't investigated. Would be happy if you wanted to dig.
Well, you rarely elide everything in production- you'll normally elide specific levels. In any case, macro expansion size isn't the only reason to prefer functions here. Functions are just a lot more flexible + easier to work with. There'd need to be pretty compelling reasons to go the macro route with all you'd need to give up for it - if that makes sense?
Sure, totally! You can just add your own little appender for this if you like and disable the default console appender. Would be happy to see a PR if you felt like including one? Could add it to the bottom of https://github.com/ptaoussanis/timbre/blob/master/src/taoensso/timbre/appenders/core.cljx#L135 and add an alias at https://github.com/ptaoussanis/timbre/blob/master/src/taoensso/timbre.cljx#L51 I haven't used cljs-devtools myself, so would be awesome if you could handle that since you've experience with it. Cheers! :-) |
Just took a quick look at cljs-devtools btw. Looks like you actually might not need/want a different appender at all?: Timbre already includes all raw logging args under the I'd suggest checking the config docstring at https://github.com/ptaoussanis/timbre#configuration to see what info your appenders are given to work with. Hope that helps? :-) |
Hey Peter, Sorry for my slow response. I don't know how you manage to always respond so quickly, politely, and thoroughly, but I truly admire you for it. :) I'll take a look at the appender documentation that you included. Thanks for that. I'll also look into whether it's possible to fix line numbers without resorting to macros. Thanks again for your help with this and for everything you do for the community. :) |
No problem, you're very welcome! Thanks for saying so :-) Feel free to ping if you have any follow-up Q's or if you turn up anything that Timbre could handle better. Like I say, it's quite possible- haven't looked into the line number situation for a while (and never looked on the cljs side). Cheers! :-) |
Using this I was able to create an appender that displayed the line number on which the appender was called instead of a line number from within the appender. That solved the biggest problem caused by appenders being functions. The problem now is that line numbers originate from within |
Hey, sorry for the delay replying to this - have been crazy swamped. Haven't had a chance to look at this in detail yet, but would be happy to see a PR with what you had in mind! |
Hey Dom, any update on this? |
Hey Peter, sorry, I've been swamped. I'll get around to this eventually but it probably won't be that soon so I'll close the issue for now. Thanks for your help! |
Zero problem, please take your time. Do prefer to keep this open in the meanwhile though so that I don't forget about it :-) |
Hi Peter, I investigated the issue how to get correct file names with timbre and Chrome Dev Tools. It turns out that new Chrome Dev Tools has a feature called blackboxing. You can select files to be ignored by Chrome Dev Tools. For logging using timbre, you should blackbox:
I used it today with cljs-devtools and it works great (including jump to the CLJS file with sourcemaps). Developer's heaven. My setup: (def devtools-level-to-fn
{:fatal js/console.error,
:error js/console.error,
:warn js/console.warn,
:info js/console.info,
:debug js/console.debug,
:trace js/console.trace})
(def devtools-appender
"Simple js/console appender which avoids pr-str and uses cljs-devtools
to format output"
{:enabled? true
:async? false
:min-level nil
:rate-limit nil
:output-fn nil
:fn
(fn [data]
(let [{:keys [level ?ns-str vargs_]} data
vargs (list* (str ?ns-str ":") (force vargs_))
f (devtools-level-to-fn level js/console.log)]
(.apply f js/console (to-array vargs))))})
(when (= "Google Inc." js/navigator.vendor)
(set! log/*config*
(-> log/*config*
(assoc-in [:appenders :console] devtools-appender)))) I am open to suggestions how to improve it and contribute it as a pull request. |
@danskarda That's awesome, thank you Daniel! May I ask for you to create a PR for a Please include a docstring mention of your https://developer.chrome.com/devtools/docs/blackboxing reference, and the vendor check as a comment block. (I'll bring that into the appender impl. myself). Just focused on something else at the moment, but will clean up the PR + merge as soon as I have some time. Edit: Oh, and you're very welcome re: the libs! :-) |
Hey @domkm, any chance you could show me a snippet of what that appender looks like? Want to take a stab at getting this in on the next release. Thanks! |
Hi @ptaoussanis, I can't seem to find the code. Sorry. If memory serves, it was a direct JS-to-CLJS port of https://gist.github.com/bgrins/5108712#file-log-portable-js. |
Hi Dom, thanks for the reply. Do you happen to recall if some kind of a port of that code definitely solved the problem? I.e. when used as a Timbre appender, the appender was producing accurate line numbers w/o the need for any blackboxing? Was fiddling with that |
This lets folks more easily use Chrome blackboxing but it'd still be nice to get a proper general-case line number solution. Seems like our options are: 1. Waiting on CLJ-865 (which'd still leave us w/o accurate source map links?). 2. Something like https://gist.github.com/bgrins/5108712 (doesn't seem possible if we want a more elaborate wrapper?) 3. Other ideas??
This is a bit of a pita (and still doesn't solve issues when user adds logging calls w/in her own macros); but it's better than nothing while we wait on some kind of resolution to CLJ-865.
I remember modifying the built-in |
No problem- if you happen to remember something, feel free to update here - otherwise have instructions in the v4.3 appender for enabling Chrome Blackboxing. CLJ-865 will also hopefully get resolved some time which should make this less important. Cheers! :-) |
I'm not a timbre user (yet), but I think OP's request can be implemented by a rather simple wrapper macro (in user code) and a modified console appender. For example we could take @danskarda's (do (set! *last-vargs* nil) (call-timbre-leading-to-devtools-appender-execution ...) (.apply js/console.log js/console *last-vargs*) nil) This would work out-of-the-box without blackboxing setup. It relies on the fact that javascript has a single thread and nothing could step in between (assuming timbre does everything synchronously). Also the generated code at call sites would not balloon that much because possibly complex timbre code is still behind a function call. Of course it could be implemented less dirty using some kind of log queue or "service" instead of setting a global variable directly. But you get the idea. |
@darwin Hi Antonin, would be happy to look over a PR if you have one - thanks! |
…nstructions The linked page was dead, and Chrome seems to now offer script exclusion via git Chrome seems to have removed
…nstructions The linked page was dead, and Chrome seems to now offer script exclusion via git Chrome seems to have removed
In ClojureScript, Chrome Developer Tools shows that Timbre logs originate from a line within
taoensso.timbre.appenders.core/console-?appender
because that is whereconsole.log
|warn
|error
is called. It would be great if logs showed the correct line number. This could be achieved by using macros instead of functions.The text was updated successfully, but these errors were encountered: