Skip to content

Conversation

@msailes
Copy link
Contributor

@msailes msailes commented Aug 18, 2020

…the 'LOG_LEVEL' environment variable is set to a Log4J Level (INFO, ERROR etc...)

Issue #, if available:

Description of changes:

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@pankajagrawal16 pankajagrawal16 left a comment

Choose a reason for hiding this comment

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

One suggestion, else looks neat 🎉

@Aspect
public final class LambdaLoggingAspect {
private static final ObjectMapper mapper = new ObjectMapper();
private static final String LOG_LEVEL = System.getenv("LOG_LEVEL");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use a example in one of the sample lambda function in example folder ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@msailes I believe after my PR #41 goes inn, we can actually validate this part as well ? Even though it might feel like testing log4j itself, but will be good to validate if those 3 lines of magic does works 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, lets merge #41 first, then re-review this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes sure. You can take it as another PR as well, fine by me.

@pankajagrawal16
Copy link
Contributor

hmm what just happend.. i just tried rebasing 🤔

@msailes msailes deleted the log-level-env-var branch August 31, 2020 09:46
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