Always use LogRecord.getMessage to get the log body

[pR0Ps]

See context in #4216

This is mostly a revert of #3343

Related: #4528

Description

This change makes the log exporting always call LogRecord.getMessage() in order to format and return the record's body text. Previously in #3343 a special case was added so that the record.msg object was used directly if no args were provided. This special case was further scoped down and changed to call record.getMessage() to improve compatibility with other Python logging libraries in #4216 . This PR proposes doing away with the special case entirely and always calling record.getMessage() in order to get the body text.

The reason for preserving the original type of the record.msg given in #3343 was that the body field type in the OTel 1.22.0 Logs Data Model is supposed to be "any", not "str", so the record.msg should not be converted to a string before being set as the body.

However, this goes against the Python documentation that states that the record.msg is only supposed to be a format string[1] or an object to convert into a format string[2] and not an actual object to be logged.

[1]: From logging.Logger.debug(msg, *args, **kwargs) docs (emphasis mine):

The msg is the message format string, and the args are the arguments which are merged into msg using the string formatting operator. [...] No % formatting operation is performed on msg when no args are supplied.

[2]: From logging.LogRecord.getMessage() docs (emphasis mine):

If the user-supplied message argument to the logging call is not a string, str() is called on it to convert it to a string. This allows use of user-defined classes as messages, whose __str__ method can return the actual format string to be used.

Note that while the "Using arbitrary objects as messages" section of the logging HOWTO states:

You can pass an arbitrary object as a message, and its __str__() method will be called when the logging system needs to convert it to a string representation.

this doesn't mean that the msg itself should be an object to log out. The "when the logging system needs to convert it to a string" part happens when the LogRecord is processed by a handler, applies the logic in [2] and converts the msg to a format string that it then formats the record.args into (if any).

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I've updated and run the unit tests, as well as run manual tests of the functionality (see #4216 for details)

Does This PR Require a Contrib Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[a-recknagel]

Any updates on the status here? I think this is a good change that will make otel logging a lot more consistent. Once logging is no longer considered experimental, it'd be much harder to argue leaving a "feature" like this behind.

[lzchen]

@a-recknagel

I've added this to the logging api/sdk rc project so it is being tracked for something to be addressed before logging is stable.

[github-actions]

This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment.
If you're still working on this, please add a comment or push new commits.

[pR0Ps]

Rebased on the current main

[github-actions]

This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment.
If you're still working on this, please add a comment or push new commits.

[pmcollins]

Hi, thanks for this change. I would like to vote in favor of moving forward with it: it simplifies OTel Python code/behavior, and it's consistent with the expected usage of record.msg as mentioned in the description.

However, the LoggingHandler is migrating to a contrib instrumentation and this one in the SDK is deprecated, so I think this functionality should go there instead. @pR0Ps if you feel up to applying this change to contrib, we can continue this discussion there. I intend to approve it once in contrib but am interested in what others think.

[pR0Ps]

@pmcollins I opened a pull request on the new repo: open-telemetry/opentelemetry-python-contrib#4372