Add a timestamp parameter to the logger. #2860

Fkawala · 2016-12-13T18:02:35Z

The timestamp parameter is useful when one sends batches of log records. In this case, every log record in a batch will have the same timestamp. The timestamp equals the date of insertion in the google cloud logging API.

If one uses the asctime attribute from the LogRecord instances as timestamp value, a log record would have a propre date.

… one sends batches of log records. In this case, each log record in a batch will have the same timestamp. This timestamp would be the timestamp of insertion in the google cloud logging API.

googlebot · 2016-12-13T18:02:38Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

Fkawala · 2016-12-13T18:07:43Z

I signed it!

googlebot · 2016-12-13T18:07:45Z

CLAs look good, thanks!

daspecster · 2016-12-13T18:20:53Z

@Fkawala, I noticed that timestamp is always str in the docs. Do you think it would actually be a datetime.datetime at some point?

There are helper functions in google.cloud.core._helpers that might make things easier too.

tseaver · 2016-12-13T19:53:22Z

@Fkawala, @daspecster The timestamp paramter to google.cloud.logging.entries._BaseEntry is expected to be an instance of datetime.datetime. It would be best if the equivalent parameter for Logger.log_* was also an instance of datetime.datetime.

dhermes · 2016-12-14T06:14:48Z

@waprin PTAL

Fkawala · 2016-12-14T09:11:14Z

@daspecster @tseaver I'll update the PR to use datetime.datime instances instead of str. I have a question though. Where should we convert the datetime.datime to RFC3339 UTC "Zulu" format (docs)? I guess that the most appropriate would be in the _make_entry_resource. Do you confirm ?

… timestamp to a RFC3339 string representation in the helper function _make_entry_resource.

daspecster · 2016-12-14T13:48:38Z

@Fkawala I think you should convert it as close to receiving it as possible.

@tseaver in thinking about this more, should the log_* methods support both str timestamps and datetime.datetime and just properly convert if needed?

Otherwise the example would be something like

from datetime import datetime

logger.log_text("A simple entry", timestamp=datetime.utcnow())

Or something similar to that.

Fkawala · 2016-12-14T14:03:33Z

@daspecster Can I get closer than I did in 7ac9c0a ?

daspecster · 2016-12-14T14:25:05Z

@Fkawala if a datetime object gets passed to the log_* methods then I think it's good. If a str ends up getting accepted in the log_* methods then we may want to convert them from str->datetime right in those methods.

As I mentioned above, this depends on what we want our usage to look like.

tseaver · 2016-12-14T15:05:04Z

@Fkawala The _make_entry_resource helper method is the right place to convert from datetime.datetime to a string: its job is to convert "native" Python objects to their JSON equivalents before putting the entry's repr on the wire.

Fkawala · 2016-12-14T17:48:16Z

@daspecster ok got it, thanks for the explanation. I'll update the PR If the string compatibility is needed for the log_* methods.

Is there something I could do to for the PR to pass the CI tests ?

daspecster · 2016-12-14T17:50:32Z

@Fkawala I think the issue with the Python 3 tests has been resolved. I'll restart the build and see if they clear up.

waprin · 2016-12-14T19:12:11Z

lgtm