Skip to content

refactor(api-service): logger to pinologger #8128

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 15 commits into from
Apr 16, 2025

Conversation

djabarovgeorge
Copy link
Contributor

@djabarovgeorge djabarovgeorge commented Apr 14, 2025

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

…ion' of github.com:novuhq/novu into nv-5651-investigate-why-log-levels-dont-work-in-production

Copy link

linear bot commented Apr 14, 2025

@djabarovgeorge djabarovgeorge changed the title chore(app-gen): rename cache file refactor(api-service): logger to pinologger Apr 15, 2025
Base automatically changed from nv-5651-investigate-why-log-levels-dont-work-in-production to next April 16, 2025 08:10
import { createNestLoggingModuleOptions, PinoLogger } from '@novu/application-generic';
import packageJson from '../../../../package.json';

export const getLogger = (context: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this come from the application generic so we can reuse in the worker if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will definitely move it once we do the same to the apps, for now i kept it close.


if (process.env.LOGGING_LEVEL || process.env.LOG_LEVEL) {
logLevel = process.env.LOGGING_LEVEL || process.env.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.

shouldn't this be reversed? like

Suggested change
logLevel = process.env.LOGGING_LEVEL || process.env.LOG_LEVEL;
logLevel = process.env.LOG_LEVEL || process.env.LOGGING_LEVEL;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its not about priority here, so any value should be fine, it's meant to be only kept for some time to support backward the old environment variable.

Copy link

linear bot commented Apr 16, 2025

Copy link

netlify bot commented Apr 16, 2025

Deploy Preview for dashboard-v2-novu-staging canceled.

Name Link
🔨 Latest commit 144e5ed
🔍 Latest deploy log https://app.netlify.com/sites/dashboard-v2-novu-staging/deploys/67ffa7d30b0fad000844a8c4

@djabarovgeorge djabarovgeorge merged commit 0422b27 into next Apr 16, 2025
8 checks passed
@djabarovgeorge djabarovgeorge deleted the nv-5651-api-refactor-logger-to-pinologger branch April 16, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants