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

logger.append_keys() is not thread-safe #2040

Closed
ovcharenko opened this issue Mar 23, 2023 · 5 comments
Closed

logger.append_keys() is not thread-safe #2040

ovcharenko opened this issue Mar 23, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@ovcharenko
Copy link

Expected Behaviour

No failed tests.

Current Behaviour

Some of the tests will fail at random.

Code snippet

import concurrent.futures

import pytest
from aws_lambda_powertools import Logger

logger = Logger()


class Entity(object):
    __slots__ = [
        "class_dict",
        "id1",
        "id2",
        "extra",
    ]

    def __init__(self, class_dict):
        logger.remove_keys(self.__slots__)

        # Default values
        for key in self.__slots__:
            setattr(self, key, None)

        self.class_dict = class_dict

    def parse(self):
        self.id1 = self.class_dict.get("id1")
        self.id2 = self.class_dict.get("id2")

        self.__update_logger_keys()
        return self

    def __update_logger_keys(self):
        extra = {}

        for attribute in self.__slots__:
            # Skip meaningless data
            if attribute not in [
                "class_dict",
                "extra",
            ]:
                extra[attribute] = getattr(self, attribute)
        self.extra = extra
        logger.append_keys(extra=extra)

    @staticmethod
    def print_ok():
        logger.info("Success!")


def process(sample_dict):
    sample_entity = Entity(sample_dict).parse()
    sample_entity.print_ok()

    id_value = sample_dict.get("id1")

    # This never fails
    assert sample_entity.id1 == id_value

    # This fails randomly
    assert logger.registered_formatter.log_format.get("extra").get("id1") == id_value


def logging_in_threads():
    futures = []

    with concurrent.futures.ThreadPoolExecutor() as pool_executor:
        for i in range(10):
            sample_dict = {"id1": i, "id2": i}
            futures.append(pool_executor.submit(process, sample_dict))

        [f.result() for f in concurrent.futures.as_completed(futures)]


@pytest.mark.parametrize("run_number", range(100))
def test_logging_in_threads(run_number):
    logging_in_threads()

Possible Solution

No response

Steps to Reproduce

Run pytest with the snippet above. Some of the tests will fail at random.

AWS Lambda Powertools for Python version

latest

AWS Lambda function runtime

3.9

Packaging format used

PyPi

Debugging logs

No response

@ovcharenko ovcharenko added bug Something isn't working triage Pending triage from maintainers labels Mar 23, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Mar 23, 2023

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our AWS Lambda Powertools Discord: Invite link

@heitorlessa
Copy link
Contributor

Closing as duplicated - here's the RFC #991

We'd need to implement contextvars now that we're finally on 3.7+. We'd appreciate a PR if you have the bandwidth.

Meanwhile, you can use additional kwargs in any log statement (logger.info("test", order_id="uuid", payment_id="uuid")

Thank you!

@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@heitorlessa heitorlessa removed the triage Pending triage from maintainers label Mar 24, 2023
@heitorlessa
Copy link
Contributor

heitorlessa commented Mar 24, 2023

Adding a warning docs banner in append_keys method on not being thread-safe. I was somehow sure this was added back then but I didn't, our apologies.

image

@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Mar 24, 2023
@github-actions
Copy link
Contributor

This is now released under 2.11.0 version!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants