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

Allow logging of None type values on logging context via a config value #4524

Closed
2 tasks done
CaptainDriftwood opened this issue Jun 14, 2024 · 4 comments
Closed
2 tasks done
Labels
feature-request feature request help wanted Could use a second pair of eyes/hands need-customer-feedback Requires more customers feedback before making or revisiting a decision

Comments

@CaptainDriftwood
Copy link

Use case

The built in logging formatter provides very good defaults, however is there a potential work around to avoid stripping logging context whose value is None? This method call appears to remove any key value pairs on the log context whose value is None:

formatted_log = self._strip_none_records(records=formatted_log)

This would allow log messages to show context that is set to None which would be valuable in certain circumstances.

Solution/User Experience

Allow the passing in of a keyword argument that allows None type values to be logged, as well as an environment variable that configures this behavior.

Alternative solutions

No response

Acknowledgment

@heitorlessa
Copy link
Contributor

giving you an answer until tomorrow as to why; we've been heads down on putting controls to prevent future regressions due to optional dependencies we had in the last release, hence why we missed.

@heitorlessa heitorlessa added help wanted Could use a second pair of eyes/hands need-customer-feedback Requires more customers feedback before making or revisiting a decision and removed triage Pending triage from maintainers labels Jun 21, 2024
@heitorlessa
Copy link
Contributor

hey @CaptainDriftwood appreciate your patience while we resolved a higher priority - created a complete workaround for you at the bottom.

I've added help wanted and need-customer-feedback to help attract +1, and anyone interested in making a PR for this -- we didn't do it earlier to not make assumptions about certain scenarios (please share yours to help out :)).

To work around it, you can provide your own Formatter and overriding _strip_none_records static method.

The side effect of allowing a None value is that it will include other possibly unwanted keys like exception: null, polluting your logs (and $ however small depending on log volume).

To save you time, I've covered that side effect in the example at the bottom along with the option for you to extend whatever key you don't want to be present if None too.

Hope that that still helps

Side effect example
{
   "level":"INFO",
   "location":"<module>:19",
   "message":"Key with None will not be dropped",
   "timestamp":"2024-06-21 09:21:22,482+0200",
   "service":"service_undefined",
   "sampling_rate":null,
   "customer_id":null,
   "exception":null,
   "exception_name":null,
   "stack_trace":null,
   "xray_trace_id":null
}

from typing import Dict, Any

from aws_lambda_powertools.logging import Logger
from aws_lambda_powertools.logging.formatter import LambdaPowertoolsFormatter


class AllowNoneFormatter(LambdaPowertoolsFormatter):
    POWERTOOLS_OPTIONAL_FIELDS = (
        "exception",
        "exception_name",
        "stack_trace",
        "xray_trace_id",
        "sampling_rate",
    )

    @staticmethod
    def _strip_none_records(records: Dict[str, Any]) -> Dict[str, Any]:
        return {
            k: v
            for k, v in records.items()
            if k not in AllowNoneFormatter.POWERTOOLS_OPTIONAL_FIELDS or v is not None
        }

logger = Logger(logger_formatter=AllowNoneFormatter())

logger.info("Key with None will not be dropped", customer_id=None)

@heitorlessa heitorlessa moved this from Triage to Pending customer in Powertools for AWS Lambda (Python) Jun 21, 2024
@leandrodamascena
Copy link
Contributor

Hi @CaptainDriftwood! Thanks for opening this issue! I have a few additional comments in addition to what @heitorlessa has already made.

We can't predict all the use cases our customers might have and how they are using Powertools. I'm sure in some use cases logging None values ​​might make sense as it sounds like you have one. But it brings some concerns, especially in the cost perspective, if we add that in Powertools as the default behavior or even as a flag to enable/disable.

I created a small examples where I removed the formatted_log = self._strip_none_records(records=formatted_log) line and create some logs.

This is a log record created with strip_none function - 169 bytes

{
   "level":"INFO",
   "location":"<module>:5",
   "message":"Logging with keys None",
   "timestamp":"2024-10-08 17:04:19,341+0100",
   "service":"service_undefined"
}

This is a log record created without strip_none function - 307 bytes

{
   "level":"INFO",
   "location":"<module>:5",
   "message":"Logging with keys None",
   "timestamp":"2024-10-08 17:04:16,825+0100",
   "service":"service_undefined",
   "sampling_rate":null,
   "my_key":null,
   "exception":null,
   "exception_name":null,
   "stack_trace":null,
   "xray_trace_id":null
}

The log containing None keys is nearly twice the size of the log without them, potentially leading to significantly higher CloudWatch costs for our customers. Also, from an observability perspective, having None values ​​doesn't really add much to observe the application and some behaviors, what I see is customers logging empty strings, which sometimes make sense to catch edge case situations.

If you still need to log with None keys, you can create a custom Logger Formatter by extending the LambdaPowertoolsFormatter class, as demonstrated by @heitorlessa. We designed this custom formatting class to help customers address specific use cases like yours, providing flexibility while maintaining the core functionality of our logger.

I'm closing this issue as not planned and please reopen if you have any additional question.

@leandrodamascena leandrodamascena closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2024
@github-project-automation github-project-automation bot moved this from Pending customer to Coming soon in Powertools for AWS Lambda (Python) Oct 8, 2024
Copy link
Contributor

github-actions bot commented Oct 8, 2024

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request feature request help wanted Could use a second pair of eyes/hands need-customer-feedback Requires more customers feedback before making or revisiting a decision
Projects
Status: Coming soon
Development

No branches or pull requests

3 participants