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 timestamp to the KLoggingEvent #425

Open
vdshb opened this issue May 5, 2024 · 4 comments
Open

Add timestamp to the KLoggingEvent #425

vdshb opened this issue May 5, 2024 · 4 comments

Comments

@vdshb
Copy link

vdshb commented May 5, 2024

When I comparing LoggingEvent from slf4j with KLoggingEvent I can see the last important missing part - TimeStamp (Maybe ThreadName is also important, but I don't really know how threads in Native/WASM world work. Maybe current coroutine name/dispatcher is worth to add as well).

It's a proposal to add a timestamp into KLoggingEvent.

I think the Instant from https://github.com/Kotlin/kotlinx-datetime is a good candidate for this field type. I don't know your thoughts about adding dependencies to the library, though. On the other hand, it's a decent and small KMP library by JB which might do the trick as adding oneliner:

public val timestamp: Instant? = Clock.System.now(),

It might also help with date/time formatting inside Formatter.

I assume payload might be another place to put timestamp into by custom logger... I haven't really get payload field's architecture purpose yet. But it's still worth to discuss separated field anyway, as using Map with String key for such popular field might cause performance issues (as well as adding Clock.System.now() in every logging event. I haven't really measured its performance, but I assume it's fast and not a problem).

Copy link

github-actions bot commented May 5, 2024

Thank you for reporting an issue. See the wiki for documentation and slack for questions.

@oshai
Copy link
Owner

oshai commented May 9, 2024

PR for adding the capability is welcome. few comments:

  • it's better not to add dependencies. if the lib is small maybe we can reimplement the small parts required.
  • timestamp should be added only to direct impl (not delegating loggers).
  • adding something to every log event might get expensive :-) but if other libs did it, I guess we can do something similar.

@imherrera
Copy link
Contributor

I'm curious what would be the purpose or use case for this field?

@vdshb
Copy link
Author

vdshb commented Jun 17, 2024

@imherrera Saving exact event time for logging (there might be a time gap between event and putting event into log)

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

3 participants