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: log_event does not serialize classes #947

Closed
kishaningithub opened this issue Jan 11, 2022 · 4 comments · Fixed by #984
Closed

Logger: log_event does not serialize classes #947

kishaningithub opened this issue Jan 11, 2022 · 4 comments · Fixed by #984
Labels
feature-request feature request

Comments

@kishaningithub
Copy link

What were you trying to accomplish?
I was trying to log the received S3 event. Please note that i am using the data classes present in this library for reading the event

@event_source(data_class=S3Event)
@log.inject_lambda_context(
    log_event=True
)
def lambda_handler(event: S3Event, context: LambdaContext):

Expected Behavior

The logged event should have all the information from the S3 event

Current Behavior

This is the output i get in the log (trimmed)

{
    "level": "INFO",
    "message": "<aws_lambda_powertools.utilities.data_classes.s3_event.S3Event object at 0x7f0be7efb2b0>",
    "timestamp": "2022-01-11 06:36:20,111+0000",
}

It looks to be that it was unable to properly represent the S3Event object as a string

Possible Solution

Implement repr and str methods in S3Event class or in the parent DictWrapper class

Steps to Reproduce (for bugs)

  1. Implement a lambda which receives and S3 event like the following
@event_source(data_class=S3Event)
@log.inject_lambda_context(
    log_event=True
)
def lambda_handler(event: S3Event, context: LambdaContext):
   pass
  1. Setup the lambda trigger as S3 object creation event
  2. Upload a file in the S3 bucket where trigger is setup
  3. See the logs in cloud watch

Environment

  • Powertools version used: 1.24.0
  • Packaging format (Layers, PyPi): PyPi
  • AWS Lambda function runtime: 3.9
  • Debugging logs

How to enable debug mode**

# paste logs here
@kishaningithub kishaningithub added bug Something isn't working triage Pending triage from maintainers labels Jan 11, 2022
@michaelbrewer
Copy link
Contributor

michaelbrewer commented Jan 13, 2022

Thanks @kishaningithub . I will take a look at that. We had a related fix for idempotency.

For now inverting the decorators:

@log.inject_lambda_context(log_event=True)
@event_source(data_class=S3Event)
def lambda_handler(event: S3Event, context: LambdaContext):
   pass

@heitorlessa heitorlessa changed the title log_event does not log content of event when using typing Logger: log_event does not serialize data classes before logging them Jan 20, 2022
@heitorlessa
Copy link
Contributor

hey @kishaningithub thanks for raising that. The main reason we didn't do this is that these are untrusted strings. We weren't certain of the potential attack vectors this could cause (similar but not the same to Log4j).

Besides the fix of inverting the decorator, I'd be cautious on doing this in production as you might dump sensitive data into the logs.

All that being said, you can bring your own JSON serializer for Logger via LambdaPowertoolsFormatter - this would be an explicit way to opt-in to serialize any data you want.

Keen to hear your thoughts

Thank you!

@heitorlessa heitorlessa added feature-request feature request and removed bug Something isn't working labels Jan 20, 2022
@heitorlessa heitorlessa changed the title Logger: log_event does not serialize data classes before logging them Logger: log_event does not serialize classes Jan 20, 2022
@heitorlessa heitorlessa added need-more-information Pending information to continue and removed triage Pending triage from maintainers labels Jan 25, 2022
michaelbrewer added a commit to gyft/aws-lambda-powertools-python that referenced this issue Jan 29, 2022
Changes:
- Add dict data from raw_event when available

closes aws-powertools#947
@heitorlessa
Copy link
Contributor

Updating here as @michaelbrewer promptly created a PR to only log IF they're an instance of our built-in event source data classes.

I'm fine with this compromise since the only vector here would be a customer to override our data classes __repr__. Any other classes should be best dealt via the serializer option in the Formatter.

@heitorlessa heitorlessa reopened this Jan 31, 2022
@heitorlessa heitorlessa added pending-release Fix or implementation already in dev waiting to be released and removed need-more-information Pending information to continue labels Jan 31, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 9, 2022

This is now released under 1.25.0 version!

@github-actions github-actions bot closed this as completed Feb 9, 2022
@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants