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

Wip level and stacktrace output #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lnostdal
Copy link

Only briefly tested, but perhaps useful.

  • (log/error (Exception. "hi")) will yield a stacktrace just like the standard timbre appender.
  • Log event messages now include the log level which is pretty standard I guess.

We'll rely on the timbre/stacktrace Fn to do this -- so the output
should more or less match what's seen in the default :println
appender.
.. so things sort of match the default :println appender in Timbre.
@@ -1,5 +1,6 @@
(ns clj-journal.timbre
(:require [clj-journal.log :refer [jsend]]))
(:require [clojure.string :as str]
Copy link
Author

Choose a reason for hiding this comment

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

Whops, I think the :as str part could be removed.

@runejuhl
Copy link
Owner

runejuhl commented Feb 1, 2020

Thank you for your PR @lnostdal!

I think I actually made a conscious decision to not have the log level be part of the message as journald already have that information, but I think it'd be a good idea to have the option to more easily customize the format, e.g. to prefix the error level like you've done.

Since you're the first active user I know of outside of my day job I'd be really glad to hear your thoughts; what would make this library better/easier/more flexible?

@lnostdal
Copy link
Author

lnostdal commented Feb 1, 2020

Yup, the meta-data always contains the level. I suppose the :output-fn needs to be flexible then; could do a (merge .. ..) somewhere perhaps so users can override this; it might already be possible.

[ need to look into this a bit later .. :) ]

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.

2 participants