-
Notifications
You must be signed in to change notification settings - Fork 153
feat(logger): basic logger logic #9
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
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is so awesome to see 🤩🎉 Because it's a big PR, could you update the PR description or Logger.md with examples of usage? That would be quicker as I'd focus on the UX first, and get acquainted with the project tooling and structure as I go along ;) Thank you!!! |
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
This is a PR containing the business logic for the logger, and it's part of this list:
Usage
See readme file for examples.
Key features
All functionalities/features existing the other loggers are (or should be, unless I missed them) implemented and part of this PR.
The TypeScript logger differs from the others in these features:
1. Decorator functionality is not in this PR.
TypeScript currently only supports Decorators for Classes.
This means that to make decorators work folks need to change their code to make the Lambda handler a method of a class. This is not a common practice of how folks implement the handler logic for the Node.js runtime, as far as I know. Please feel free to add your perspective on this in the review.
2. Possibility to configure the logger settings (options) via constructor or custom configuration service
On top of the already existing configuration for the logger that can be done via environmental variables, developers can optionally bootstrap the logger settings by passing them in the constructor, or relying on a custom configuration service (that could extend the functionality of the existing one, or be of a different kind like AppConfig).
Options for the logger (log level, feature toggles on/off) are set dynamically by checking these "sources", in this specific order:
settings passed in Logger class constructor settings declared in custom configuration service settings declared in default environmental variables service.
3. BYOL: Bring Your Own Log (structure)
This powertool's logger by default prints logs having the same JSON schema as the other tools.
I extended this functionality to optionally allow developers to set their own log JSON schema, if they want to.
The reason behind this choice is that some developers, especially in big organizations, might need to comply with their company's standardized logging RFC and schema implemented company-wide for Lambda, containers, or code running on EC2. This is a necessity when logs are sent to a central logging SaaS vendor and the log key/values need to be indexed.
Another use case could be implementing a JSON schema compatible with the opentelemetry-specification standard.
4. Location field is missing
This is honestly a great feature the other tools have. But currently there is no easy or not-hacky way to get this information in Node.js/Typescript. The closest feature I found is this library here.
Shall we use it?
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.