Skip to content

Always use LogRecord.getMessage to get the log body#4372

Open
pR0Ps wants to merge 3 commits intoopen-telemetry:mainfrom
pR0Ps:bugfix/record-getmessage-compatibility
Open

Always use LogRecord.getMessage to get the log body#4372
pR0Ps wants to merge 3 commits intoopen-telemetry:mainfrom
pR0Ps:bugfix/record-getmessage-compatibility

Conversation

@pR0Ps
Copy link
Copy Markdown

@pR0Ps pR0Ps commented Mar 28, 2026

Description

This change makes the log exporting always call LogRecord.getMessage() in order to format and return the record's body text. This is a port of open-telemetry/opentelemetry-python#4327 to this new repo. See that pull request for more details and context.

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 open-telemetry/opentelemetry-python#4216 for details)

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

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

Copy link
Copy Markdown
Member

@pmcollins pmcollins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. I'm in favor of this change, but because it's breaking, it probably warrants a discussion in the weekly SIG meeting. @pR0Ps not sure if you attend these already, but you are welcome to join if you are interested.

@tammy-baylis-swi tammy-baylis-swi moved this to Ready for review in Python PR digest Apr 2, 2026
Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. I agree with @pmcollins the changelog needs to indicate this is a breaking change.

PS the description references the core PR (#4327) for context, but that PR itself links to additional background that's useful for reviewers:

  • open-telemetry/opentelemetry-python#4216 — the original issue and discussion that motivated this change
  • open-telemetry/opentelemetry-python#3343 — the PR this is mostly reverting

Probably worth adding those directly to the description here so reviewers don't have to chase the chain 😄

@MikeGoldsmith
Copy link
Copy Markdown
Member

Has this been discussed at a SIG call? I agree this probably the right thing to do, but would like other @open-telemetry/python-approvers to give input as it's a breaking change.

@pmcollins
Copy link
Copy Markdown
Member

Has this been discussed at a SIG call? I agree this probably the right thing to do, but would like other @open-telemetry/python-approvers to give input as it's a breaking change.

Hey @MikeGoldsmith, ran out of time last week, but hope to discuss tomorrow.

Copy link
Copy Markdown
Member

@pmcollins pmcollins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi -- discussed this in the SIG meeting and folks seem to be in favor of this change. The fact that it has been moved out of the SDK give us some additional flexibility. Added a suggestion about the changelog entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

4 participants