Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

callsite parameter fix while operating under asyncio #565

Merged
merged 13 commits into from
Oct 30, 2023
Merged

Conversation

pahrohfit
Copy link
Contributor

Summary

Added support for proper callsite parameters while operating under asyncio

Pull Request Check List

  • Added tests for changed code.
    • The CI fails with less than 100% coverage.
  • New APIs are added to api.py.
  • Updated documentation for changed code.
    • New functions/classes have to be added to docs/api.rst by hand.

    • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.

      The next version is the second number in the current release + 1. The first number represents the current year. So if the current version on PyPI is 23.1.0, the next version is gonna be 23.2.0. If the next version is the first in the new year, it'll be 24.1.0.

  • Documentation in .rst and .md files is written using semantic newlines.
  • Changes (and possible deprecations) are documented in the changelog.
  • Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

@pahrohfit
Copy link
Contributor Author

Sample:

import asyncio
import structlog

structlog.configure(
    processors=[
        structlog.processors.CallsiteParameterAdder(
            {
                structlog.processors.CallsiteParameter.FILENAME,
                structlog.processors.CallsiteParameter.FUNC_NAME,
                structlog.processors.CallsiteParameter.LINENO,
            },
        ),
        structlog.processors.JSONRenderer(),
    ],
    context_class=dict,
    cache_logger_on_first_use=True,
)
logger = structlog.get_logger()


def sync_logging():
    logger.info("hello sync")

async def async_logging():
    await logger.ainfo("hello async via ainfo")

if __name__ == "__main__":
    sync_logging()

    loop = asyncio.get_event_loop()
    loop.run_until_complete(async_logging())
    loop.close()

Produces:

$ python structlog_test.py
{"event": "hello sync", "lineno": 22, "func_name": "sync_logging", "filename": "structlog_test.py"}
{"event": "hello async via ainfo", "lineno": 25, "func_name": "async_logging", "filename": "structlog_test.py"}

Copy link
Owner

@hynek hynek left a comment

Choose a reason for hiding this comment

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

Thanks this is cool! needs some work, but should be straight-forward.

src/structlog/_frames.py Outdated Show resolved Hide resolved
src/structlog/_log_levels.py Outdated Show resolved Hide resolved
src/structlog/_log_levels.py Outdated Show resolved Hide resolved
src/structlog/contextvars.py Outdated Show resolved Hide resolved
src/structlog/_log_levels.py Outdated Show resolved Hide resolved
@pahrohfit pahrohfit requested a review from hynek October 21, 2023 16:55
@pahrohfit
Copy link
Contributor Author

How’s recent commit looking?

@hynek
Copy link
Owner

hynek commented Oct 28, 2023

Hm I tried to push changes into your branch but I'm getting a permission denied. Are you sure you haven't forgotten to grant the permission for that or am I just failing at git here?

src/structlog/contextvars.py Outdated Show resolved Hide resolved
tests/test_processors.py Outdated Show resolved Hide resolved
tests/test_processors.py Outdated Show resolved Hide resolved
@pahrohfit
Copy link
Contributor Author

pahrohfit commented Oct 29, 2023

Hm I tried to push changes into your branch but I'm getting a permission denied. Are you sure you haven't forgotten to grant the permission for that or am I just failing at git here?

image

It shows the relevant box checked ... I just added you as a collaborator to the fork as well for the hell of it

@pahrohfit pahrohfit requested a review from hynek October 29, 2023 22:30
@hynek
Copy link
Owner

hynek commented Oct 30, 2023

hm no if the checkmark is set, it should work. maybe it's because you opened your PR from your own main branch which has protections. You should avoid doing that in general, none-the-least because once this PR is merged, you'll have a bad time syncing your fork due to squash and merge.

sadly that's an information that comes too late for the PR checklist ;)

Copy link
Owner

@hynek hynek left a comment

Choose a reason for hiding this comment

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

Thanks!

@hynek hynek merged commit 7883d04 into hynek:main Oct 30, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants