Bug, double conversion.

[sactre]

in #2860 line: 163, file: logging/google/cloud/logging/logger.py

the function _make_entry_resource return a resource(dict), but de timestamp element has to be a datetime not a string. Because in the next step the timestamp has a conversion from datetime to string (file: _gax.py, function: _log_entry_mapping_to_pb).

[Fkawala]

@sactre that's wired ! from the source I get the same understanding than you, it should fail. However, I currently use the timestamp parameter in the log_struct function and it works like a charm.

Do you have a stack trace to investigate on ?

[plowman]

@Fkawala I'm seeing the same thing that @sactre sees. Specifically, if I specify a timestamp, it fails.

Using a string timestamp

    from google.cloud import logging as google_logging
    one_week_ago = datetime.utcnow() - timedelta(days=7)
    one_week_ago = one_week_ago.strftime("%Y-%m-%dT%H:%M:%S.%fZ")

    logging_client = google_logging.Client(project="my-project")
    logger = logging_client.logger("mobile-logger")

    logger.log_text("My Logs Here", timestamp=one_week_ago)

String timestamp stack trace:

  File "/project/project3/project/servers/apis/mobile/api_helpers.py", line 261, in _inner_method
    return api_method(api_instance, request, *args, **kwargs)
  File "/project/project3/project/servers/apis/mobile/resources/logs.py", line 144, in write_bulk
    logger.log_text("My Logs Here", timestamp=one_week_ago)
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/logging/logger.py", line 201, in log_text
    http_request=http_request, timestamp=timestamp)
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/logging/logger.py", line 163, in _make_entry_resource
    resource['timestamp'] = _datetime_to_rfc3339(timestamp)
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/_helpers.py", line 329, in _datetime_to_rfc3339
    return value.strftime(_RFC3339_MICROS)
AttributeError: 'str' object has no attribute 'strftime'

Using a datetime timestamp

    from google.cloud import logging as google_logging
    one_week_ago = datetime.utcnow() - timedelta(days=7)

    logging_client = google_logging.Client(project="my-project")
    logger = logging_client.logger("mobile-logger")

    logger.log_text("My Logs Here", timestamp=one_week_ago)

Datetime timestamp stack trace:

  File "/project/project3/project/servers/apis/mobile/api_helpers.py", line 261, in _inner_method
    return api_method(api_instance, request, *args, **kwargs)
  File "/project/project3/project/servers/apis/mobile/resources/logs.py", line 143, in write_bulk
    logger.log_text("My Logs Here", timestamp=one_week_ago)
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/logging/logger.py", line 202, in log_text
    client.logging_api.write_entries([entry_resource])
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/logging/_gax.py", line 128, in write_entries
    entry_pbs = [_log_entry_mapping_to_pb(entry) for entry in entries]
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/logging/_gax.py", line 456, in _log_entry_mapping_to_pb
    mapping['timestamp'] = _datetime_to_rfc3339(mapping['timestamp'])
  File "/home/vagrant/.virtualenvs/project3/local/lib/python2.7/site-packages/google/cloud/_helpers.py", line 329, in _datetime_to_rfc3339
    return value.strftime(_RFC3339_MICROS)
AttributeError: 'str' object has no attribute 'strftime'

[Fkawala]

@plowman Thanks for the MRE! We observe different behaviors because I use the Batch class instead of Logger class. A Batch instance is created by the batch method of the Logger class.

Methods log_text, log_structand log_proto from Batch simply stack entries for further use. These methods, contrary to their counterpart from Logger do not call to the _make_entry_resource method. The reason why my code works it that the commit method of Batch does not convert the datetime timestamp to string.

From the docs. I understand that is it correct to convert datetime into string in _make_entry_resource to convert the datetime into string. Therefore, I guess that the fix should be done on the "gax side". If we go this way we would have to fix the commit method from Batch. That being said I don't know what could break if we go this way.

Maybe a better way would be to refactor so that commit method from Batch shares code with the _make_entry_resource method.

@daspecster Could you flag this as bug ? Could you advise me about the right way to fix that ?

[daspecster]

@plowman, @Fkawala, @sactre this is great information.

@Fkawala, I believe the best place to fix is where @sactre mentioned. I'll take a stab at it right now, I think it should be pretty simple to fix.

[Fkawala]

@daspecster great !