googleapis / python-logging

Apache License 2.0
130 stars 55 forks source link

The docstring comment to describe type of severity field in LogEntry is not up to date #739

Closed ymotongpoo closed 1 year ago

ymotongpoo commented 1 year ago

Environment details

Steps to reproduce

  1. call google.cloud.logging.Logger#log_entry or google.cloud.logging.Batch#log_entry with named arguments, and assign severity level in string as described in the official doc.

Code example

batch.log_struct(
    {
        "duration": d,
        "service": "profile_handler",
        "version": version,
    },
    timestamp=start,
    severity="info",
)

Stack trace

Traceback (most recent call last):
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 812, in _ConvertScalarFieldValue
    number = int(value)
ValueError: invalid literal for int() with base 10: 'info'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 815, in _ConvertScalarFieldValue
    raise ParseError('Invalid enum value {0} for enum type {1}'.format(
google.protobuf.json_format.ParseError: Invalid enum value info for enum type google.logging.type.LogSeverity

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 633, in _ConvertFieldValuePair
    _ConvertScalarFieldValue(value, field,
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 825, in _ConvertScalarFieldValue
    raise ParseError('{0} at {1}'.format(e, path)) from e
google.protobuf.json_format.ParseError: Invalid enum value info for enum type google.logging.type.LogSeverity at LogEntry.severity

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/dummylog.py", line 47, in <module>
    main()
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/dummylog.py", line 43, in main
    batch.commit(partial_success=True)
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/cloud/logging_v2/logger.py", line 467, in commit
    client.logging_api.write_entries(
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/cloud/logging_v2/_gapic.py", line 154, in write_entries
    log_entry_pbs = [_log_entry_mapping_to_pb(entry) for entry in entries]
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/cloud/logging_v2/_gapic.py", line 154, in <listcomp>
    log_entry_pbs = [_log_entry_mapping_to_pb(entry) for entry in entries]
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/cloud/logging_v2/_gapic.py", line 564, in _log_entry_mapping_to_pb
    ParseDict(mapping, entry_pb)
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 471, in ParseDict
    parser.ConvertMessage(js_dict, message, '')
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 512, in ConvertMessage
    self._ConvertFieldValuePair(value, message, path)
  File "/usr/local/google/home/yoshifumi/corp/friction/20230406/venv/lib/python3.10/site-packages/google/protobuf/json_format.py", line 637, in _ConvertFieldValuePair
    raise ParseError(
google.protobuf.json_format.ParseError: Failed to parse severity field: Invalid enum value info for enum type google.logging.type.LogSeverity at LogEntry.severity.
daniel-sanche commented 1 year ago

Hmm I'm not sure this is just a docstring issue. It is the intention of the library to support string severity for the hand-written, user-facing API surface

Do you see this issue for both logger and batch? And across log_struct, log_text, log_empty, log, etc?

I haven't dug too deep into this, but we should have tests covering this use-case, so I'm curious how general this is

ymotongpoo commented 1 year ago

Closing this as #744 (the fix to #743) is approved to merge.