Skip to content

feat: added 'LOGGER_TRANSPORT' env key#29

Open
finkinfridom wants to merge 2 commits into
Aengz:masterfrom
finkinfridom:feature/logger-transport
Open

feat: added 'LOGGER_TRANSPORT' env key#29
finkinfridom wants to merge 2 commits into
Aengz:masterfrom
finkinfridom:feature/logger-transport

Conversation

@finkinfridom

@finkinfridom finkinfridom commented Dec 20, 2023

Copy link
Copy Markdown

PR for issue #28
Added a new LOGGER_TRANSPORT env variable to easily setup pino loggerTransport property

fyi: it seems the tests are failing (and it happens into master too)

@finkinfridom finkinfridom force-pushed the feature/logger-transport branch from ec36b99 to 2f835cb Compare December 20, 2023 09:06
Comment thread src/adapters/logger.ts
...(process.env.LOGGER_TRANSPORT
? {
transport: {
target: process.env.LOGGER_TRANSPORT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would be nice to have a default transport to pino-pretty if LOGGER_TRANSPORT is not defined

@finkinfridom finkinfridom Jan 9, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If I'm not wrong, by default Pino uses stdout as the default transport (as per right now).
We are using a different transport in our source code because we are ingesting logs through OpenTelemetry (which could be considered a standard) as it prefers JSON format (similarly to datadog and other products).
I could change the code tomorrow to provide JSON as the default if you want.

Pino by default uses JSON as standard transport (if nothing is specified as in the changes I made).
In your current source code, you're forcing pino-pretty which is cool for console output but not useful when ingesting data through DataDog (or similar products).
I think providing a fallback value would make it impossible to "reset" the behavior to the default one.

As a mid-term improvements, there could be the possibility to specify loggerOptions (similarly to how payload does) on an init level

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