Skip to content

Fix structlog handling of dict arguments in logging calls#62258

Open
SakshamSinghal20 wants to merge 5 commits intoapache:mainfrom
SakshamSinghal20:nor
Open

Fix structlog handling of dict arguments in logging calls#62258
SakshamSinghal20 wants to merge 5 commits intoapache:mainfrom
SakshamSinghal20:nor

Conversation

@SakshamSinghal20
Copy link
Copy Markdown
Contributor

Description

Fixes #62201: structlog configuration fails with dict input

When a dictionary is passed as a positional argument to a logging message
(e.g., logging.getLogger(__name__).warning('Info message %s', {'a': 10})),
Airflow's structlog processor incorrectly treats the dict as structured context
instead of a value for string interpolation, causing a crash.

This fix modifies the structlog processor to convert dict arguments to their
string representation before processing, ensuring backward compatibility with
existing structured logging functionality.

Changes Made

  • Modified structlog processor in airflow/utils/log/ to handle dict arguments
  • Added type conversion for positional arguments containing dictionaries
  • Ensured backward compatibility with existing structured logging (kwargs)

Testing

  • Verified fix with reproduction script: logging.getLogger(__name__).warning('Info message %s', {'a': 10})
  • Tested with nested dictionaries, empty dictionaries, and various data types
  • Confirmed no regression in existing logging functionality

Related Issues


Was generative AI tooling used to co-author this PR?
  • Yes (used for understanding the issue and analyzing the logic)

Copy link
Copy Markdown
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

Looking at this more closely, this is not a valid way of logging dictionaries.

Since we use underneath stdlib logging, it will crash which is expected. Running this:

import logging

logging.basicConfig(level=logging.DEBUG)
logger = logging.getLogger(__name__)

logger.warning("Info message %s", {"a": 10})

would crash too. The right way to log a dict would be: logger.warning("Info message %s", str({"a": 10}))

@SakshamSinghal20
Copy link
Copy Markdown
Contributor Author

Looking at this more closely, this is not a valid way of logging dictionaries.

Since we use underneath stdlib logging, it will crash which is expected. Running this:

import logging

logging.basicConfig(level=logging.DEBUG)
logger = logging.getLogger(__name__)

logger.warning("Info message %s", {"a": 10})

would crash too. The right way to log a dict would be: logger.warning("Info message %s", str({"a": 10}))

Thank You @amoghrajesh for letting me know I will change this ASAP.

@anyapriya
Copy link
Copy Markdown

Looking at this more closely, this is not a valid way of logging dictionaries.

Since we use underneath stdlib logging, it will crash which is expected. Running this:

import logging

logging.basicConfig(level=logging.DEBUG)
logger = logging.getLogger(__name__)

logger.warning("Info message %s", {"a": 10})

would crash too. The right way to log a dict would be: logger.warning("Info message %s", str({"a": 10}))

@amoghrajesh Wait, I'm a little confused. You're saying this example should fail, but it doesn't seem to when I test it out, screenshot below. Would be curious if it's a version difference, or if I'm misunderstanding.

Screenshot 2026-02-24 at 10 03 00 AM

@SakshamSinghal20
Copy link
Copy Markdown
Contributor Author

@amoghrajesh I’ve addressed your feedback and pushed the necessary changes. Could you please review it again when you get a chance?

@SakshamSinghal20
Copy link
Copy Markdown
Contributor Author

@potiuk Would you like to look at this once and tell if everything is correctly implemented or not?

@jscheffl
Copy link
Copy Markdown
Contributor

@SakshamSinghal20 This PR has a few issues that need to be addressed before it can be reviewed — please see our Pull Request quality criteria.

Issues found:

  • Merge conflicts: This PR has merge conflicts with the main branch. Your branch is 722 commits behind main. Please rebase your branch (git fetch origin && git rebase origin/main), resolve the conflicts, and push again. See contributing quick start.

Note: Your branch is 722 commits behind main. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.

What to do next:

  • The comment informs you what you need to do.
  • Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
  • Maintainers will then proceed with a normal review.

Please address the issues above and push again. If you have questions, feel free to ask on the Airflow Slack.

@potiuk potiuk marked this pull request as draft April 2, 2026 16:42
@potiuk
Copy link
Copy Markdown
Member

potiuk commented Apr 2, 2026

@SakshamSinghal20 This PR has been converted to draft because it does not yet meet our Pull Request quality criteria.

Issues found:

  • Merge conflicts: This PR has merge conflicts with the main branch. Your branch is 1185 commits behind main. Please rebase your branch (git fetch origin && git rebase origin/main), resolve the conflicts, and push again. See contributing quick start.

Note: Your branch is 1185 commits behind main. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.

What to do next:

  • The comment informs you what you need to do.
  • Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
  • There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
  • Maintainers will then proceed with a normal review.

Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack.

@SakshamSinghal20 SakshamSinghal20 marked this pull request as ready for review April 3, 2026 07:42
@SakshamSinghal20
Copy link
Copy Markdown
Contributor Author

@potiuk sorry for the inactivity, i have updated my branch.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

structlog configuration fails with dict input

5 participants