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

LogSink::ToString has no microsecond precision. #307

Closed
bmahler opened this issue Apr 11, 2018 · 3 comments · Fixed by #441
Closed

LogSink::ToString has no microsecond precision. #307

bmahler opened this issue Apr 11, 2018 · 3 comments · Fixed by #441
Milestone

Comments

@bmahler
Copy link

bmahler commented Apr 11, 2018

I noticed a custom LogSink was logging without microsecond precision:

I0330 16:05:21.000000 21188 master.cpp:8832] ...
I0330 16:05:21.000000 21188 master.cpp:8832] ...
I0330 16:05:21.000000 21188 master.cpp:8832] ...

Currently LogSink::ToString always uses 0 for the microsecond component of the timestamp. There is a FIXME related to this:

  // FIXME(jrvb): Updating this to use the correct value for usecs
  // requires changing the signature for both this method and
  // LogSink::send().  This change needs to be done in a separate CL
  // so subclasses of LogSink can be updated at the same time.
  int usecs = 0;

https://github.com/google/glog/blob/v0.3.5/src/logging.cc#L1631-L1635

Per the FIXME, it looks like the only way to fix this is by updating the signature of LogSink::send. Any third party LogSink implementations would need to have their signature adjusted. Would such a PR be acceptable?

@bmahler
Copy link
Author

bmahler commented Aug 14, 2018

@sergiud I'm happy to send a PR for this, would you be willing to help me get it merged?

@sergiud
Copy link
Collaborator

sergiud commented Aug 15, 2018

@bmahler Sure, go ahead!

@sergiud
Copy link
Collaborator

sergiud commented Oct 31, 2019

Fixed in 5792d60.

@sergiud sergiud closed this as completed Oct 31, 2019
@sergiud sergiud linked a pull request Apr 1, 2021 that will close this issue
@sergiud sergiud added this to the 0.5 milestone Apr 1, 2021
@sergiud sergiud removed the PR welcome label Apr 1, 2021
@sergiud sergiud mentioned this issue May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants