Skip to content

fix(application-generic): logging levels #8111

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/on-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ jobs:
- name: Run Lint, Build, Test
uses: mansagroup/nrwl-nx-action@v3
env:
LOGGING_LEVEL: 'info'
LOG_LEVEL: 'info'
Copy link
Contributor

Choose a reason for hiding this comment

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

@merrcury do we need to makes some updates on TF deployment aswell?

Copy link
Member

Choose a reason for hiding this comment

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

No, We need to make this change just in secret Manager

with:
targets: lint,build,test
projects: ${{join(fromJson(needs.get-affected.outputs.test-packages), ',')}}
Expand Down
4 changes: 2 additions & 2 deletions .idea/runConfigurations/WORKER___TEST.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion apps/api/src/.env.development
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ REDIS_CLUSTER_KEY_PREFIX=
NEW_RELIC_ENABLED=true
NEW_RELIC_APP_NAME="[DEV] - api"
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info

VERCEL_REDIRECT_URI=https://dashboard.novu-staging.co/auth/login
VERCEL_BASE_URL=https://api.vercel.com
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/.env.production
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ REDIS_CLUSTER_KEY_PREFIX=

NEW_RELIC_ENABLED=true
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info

VERCEL_REDIRECT_URI=https://dashboard.novu.co/auth/login
VERCEL_BASE_URL=https://api.vercel.com
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/.env.test
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ MAX_NOVU_INTEGRATION_MAIL_REQUESTS=300
INTERCOM_IDENTITY_VERIFICATION_SECRET_KEY=
NOVU_EMAIL_INTEGRATION_API_KEY=test

LOGGING_LEVEL=error
LOG_LEVEL=error

LAUNCH_DARKLY_SDK_KEY=

Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/.example.env
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ VERCEL_BASE_URL=https://api.vercel.com
STORE_NOTIFICATION_CONTENT=true

INTERCOM_IDENTITY_VERIFICATION_SECRET_KEY=
LOGGING_LEVEL=info
LOG_LEVEL=info

LAUNCH_DARKLY_SDK_KEY=

Expand Down
2 changes: 1 addition & 1 deletion apps/inbound-mail/.example.env
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ REDIS_DB_INDEX="2"
REDIS_HOST="localhost"
REDIS_PORT="6379"

LOGGING_LEVEL=info
LOG_LEVEL=info
## This value should be set to true if it is the first time you are running the with the database
MONGO_AUTO_CREATE_INDEXES=false
2 changes: 1 addition & 1 deletion apps/inbound-mail/src/.env.development
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ REDIS_CLUSTER_KEY_PREFIX=
NEW_RELIC_ENABLED=true
NEW_RELIC_APP_NAME="[DEV] - INBOUND-MAIL"
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info
2 changes: 1 addition & 1 deletion apps/inbound-mail/src/.env.production
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ REDIS_CLUSTER_KEY_PREFIX=

NEW_RELIC_ENABLED=true
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info
2 changes: 1 addition & 1 deletion apps/inbound-mail/src/.env.test
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ REDIS_CLUSTER_KEEP_ALIVE=
REDIS_CLUSTER_FAMILY=
REDIS_CLUSTER_KEY_PREFIX=

LOGGING_LEVEL=error
LOG_LEVEL=error
2 changes: 1 addition & 1 deletion apps/webhook/src/.env.development
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ MONGO_URL=mongodb://127.0.0.1:27017/novu-db
NEW_RELIC_ENABLED=true
NEW_RELIC_APP_NAME="[DEV] - WEBHOOK"
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info
2 changes: 1 addition & 1 deletion apps/webhook/src/.env.production
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ WIDGET_BASE_URL=https://widget.novu.co

NEW_RELIC_ENABLED=true
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info
MONGO_MIN_POOL_SIZE=50
2 changes: 1 addition & 1 deletion apps/webhook/src/.env.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ MONGO_URL=mongodb://127.0.0.1:27017/novu-test
PORT=1341
NODE_ENV=test

LOGGING_LEVEL=error
LOG_LEVEL=error
2 changes: 1 addition & 1 deletion apps/webhook/src/.example.env
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ PORT=3003
MONGO_URL=mongodb://127.0.0.1:27017/novu-db
MONGO_MAX_POOL_SIZE=500

LOGGING_LEVEL=info
LOG_LEVEL=info
2 changes: 1 addition & 1 deletion apps/worker/src/.env.test
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ NEW_RELIC_APP_NAME="[TEST] - worker"
# Segment Analytics
# SEGMENT_TOKEN=

LOGGING_LEVEL=error
LOG_LEVEL=error

# Launch Darkly
LAUNCH_DARKLY_SDK_KEY=
Expand Down
2 changes: 1 addition & 1 deletion apps/ws/src/.env.development
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ WS_CONTEXT_PATH=
NEW_RELIC_ENABLED=true
NEW_RELIC_APP_NAME="[DEV] - ws"
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info

MONGO_AUTO_CREATE_INDEXES=true

Expand Down
2 changes: 1 addition & 1 deletion apps/ws/src/.env.production
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ WS_CONTEXT_PATH=

NEW_RELIC_ENABLED=true
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_ENABLED=true
LOGGING_LEVEL=info
LOG_LEVEL=info
MONGO_MIN_POOL_SIZE=50

## This value should be set to true if it is the first time you are running the with the database
Expand Down
2 changes: 1 addition & 1 deletion apps/ws/src/.env.test
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ REDIS_CACHE_SERVICE_PORT=6379
GLOBAL_CONTEXT_PATH=
WS_CONTEXT_PATH=

LOGGING_LEVEL=info
LOG_LEVEL=info
MONGO_AUTO_CREATE_INDEXES=true
2 changes: 1 addition & 1 deletion apps/ws/src/.example.env
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ GLOBAL_CONTEXT_PATH=
WS_CONTEXT_PATH=

NEW_RELIC_ENABLED=false
LOGGING_LEVEL=info
LOG_LEVEL=info
2 changes: 1 addition & 1 deletion libs/application-generic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"mixpanel": "^0.17.0",
"nanoid": "^3.1.20",
"nestjs-otel": "6.1.1",
"nestjs-pino": "4.1.0",
"nestjs-pino": "4.2.0",
"node-fetch": "^3.2.10",
"pino-http": "^8.3.3",
"pino-pretty": "^9.4.0",
Expand Down
14 changes: 4 additions & 10 deletions libs/application-generic/src/logging/LogDecorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ const DEFAULT_OPTIONS: IOptions = {
};

export const LogDecorator = (options = DEFAULT_OPTIONS) => {
return (
target: any,
propertyName: string,
descriptor: TypedPropertyDescriptor<any>,
): void => {
return (target: any, propertyName: string, descriptor: TypedPropertyDescriptor<any>): void => {
const logger = options?.logger || new Logger(target?.constructor?.name);
const method = descriptor?.value;

Expand All @@ -30,21 +26,19 @@ export const LogDecorator = (options = DEFAULT_OPTIONS) => {
...((options?.transform ? options?.transform(args) : args) || {}),
},
},
`"${target?.constructor?.name}.${propertyName}" invoke`,
`"${target?.constructor?.name}.${propertyName}" invoke`
);

const data = await method.apply(this, args);

const executeTime = options?.timestamp
? `${Date.now() - currentTime}`
: '';
const executeTime = options?.timestamp ? `${Date.now() - currentTime}` : '';

logger.debug(
{
executionTimeMs: executeTime,
result: { ...(options?.transform ? options?.transform(data) : data) },
},
`"${target?.constructor?.name}.${propertyName}" result`,
`"${target?.constructor?.name}.${propertyName}" result`
);

return data;
Expand Down
133 changes: 42 additions & 91 deletions libs/application-generic/src/logging/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable no-console */
import { NestInterceptor, RequestMethod } from '@nestjs/common';
import { getLoggerToken, Logger, LoggerErrorInterceptor, LoggerModule, Params, PinoLogger } from 'nestjs-pino';
import { storage, Store } from 'nestjs-pino/storage';
Expand All @@ -10,135 +11,85 @@ export function getErrorInterceptor(): NestInterceptor {
}
export { Logger, LoggerModule, PinoLogger, storage, Store, getLoggerToken };

const loggingLevelArr = ['error', 'warn', 'info', 'verbose', 'debug'];

const loggingLevelSet = {
error: 50,
warn: 40,
trace: 10,
debug: 20,
info: 30,
verbose: 20,
debug: 10,
warn: 40,
error: 50,
fatal: 60,
none: 70,
Comment on lines +15 to +21
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alignment with the pino logger levels with additional 'none' in case we want to silence the logs.

};

interface ILoggingVariables {
env: string;
level: string;

hostingPlatform: string;
tenant: string;
}
const loggingLevelArr = Object.keys(loggingLevelSet);

export function getLogLevel() {
let logLevel = process.env.LOGGING_LEVEL ?? 'info';

if (loggingLevelArr.indexOf(logLevel) === -1) {
// eslint-disable-next-line no-console
console.log(`${logLevel}is not a valid log level of ${loggingLevelArr}. Reverting to info.`);
let logLevel = null;

if (process.env.LOGGING_LEVEL || process.env.LOG_LEVEL) {
logLevel = process.env.LOGGING_LEVEL || process.env.LOG_LEVEL;
Comment on lines +28 to +29
Copy link
Contributor Author

Choose a reason for hiding this comment

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

backward compatibility for the old LOGGING_LEVEL environment variable.

} else {
console.log(`Environment variable LOG_LEVEL is not set. Falling back to info level.`);
logLevel = 'info';
}
// eslint-disable-next-line no-console
console.log(`Log Level Chosen: ${logLevel}`);

return logLevel;
}

// TODO: should be moved into a config framework
function getLoggingVariables(): ILoggingVariables {
const env = process.env.NODE_ENV ?? 'local';

// eslint-disable-next-line no-console
console.log(`Environment: ${env}`);

const hostingPlatform = process.env.HOSTING_PLATFORM ?? 'Docker';

// eslint-disable-next-line no-console
console.log(`Platform: ${hostingPlatform}`);

const tenant = process.env.TENANT ?? 'OS';
if (!loggingLevelArr.includes(logLevel)) {
console.log(`${logLevel}is not a valid log level of ${loggingLevelArr}. Falling back to info level.`);

// eslint-disable-next-line no-console
console.log(`Tenant: ${tenant}`);
return 'info';
}

return {
env,
level: getLogLevel(),
hostingPlatform,
tenant,
};
return logLevel;
}

export function createNestLoggingModuleOptions(settings: ILoggerSettings): Params {
const values: ILoggingVariables = getLoggingVariables();

let redactFields: string[] = sensitiveFields.map((val) => val);

let redactFields: string[] = sensitiveFields;
redactFields.push('req.headers.authorization');

const baseWildCards = '*.';
const baseArrayWildCards = '*[*].';
for (let i = 1; i <= 6; i += 1) {
redactFields = redactFields.concat(sensitiveFields.map((val) => baseWildCards.repeat(i) + val));

redactFields = redactFields.concat(sensitiveFields.map((val) => baseArrayWildCards.repeat(i) + val));
}

const transport = ['local', 'test', 'debug'].includes(process.env.NODE_ENV) ? { target: 'pino-pretty' } : undefined;

// eslint-disable-next-line no-console
console.log(loggingLevelSet);

// eslint-disable-next-line no-console
console.log(`Selected Log Transport ${!transport ? 'None' : 'pino-pretty'}`, loggingLevelSet);
const configSet = {
transport: ['local', 'test', 'debug'].includes(process.env.NODE_ENV) ? { target: 'pino-pretty' } : undefined,
platform: process.env.HOSTING_PLATFORM ?? 'Docker',
tenant: process.env.TENANT ?? 'OS',
level: getLogLevel(),
levels: loggingLevelSet,
};
console.log('Logging Configuration:', {
level: configSet.level,
environment: process.env.NODE_ENV,
transport: !configSet.transport ? 'None' : 'pino-pretty',
platform: configSet.platform,
tenant: configSet.tenant,
levels: JSON.stringify(configSet.levels),
});

return {
exclude: [{ path: '*/health-check', method: RequestMethod.GET }],
assignResponse: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

customProps is not needed because of this new flag that was introduced in pino 4.2 version.

customProps: (req: any, res: any) => ({
        user: {
          userId: req?.user?._id || null,
          environmentId: req?.user?.environmentId || null,
          organizationId: req?.user?.organizationId || null,
        },
        authScheme: req?.authScheme,
        rateLimitPolicy: res?.rateLimitPolicy,
      }),

pinoHttp: {
customLevels: loggingLevelSet,
level: values.level,
useOnlyCustomLevels: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

in the end i kept the custom levels, this flag is making sure that they are not overridden by the built-in levels in pino

customLevels: configSet.levels,
level: configSet.level,
redact: {
paths: redactFields,
censor: customRedaction,
Copy link
Contributor

Choose a reason for hiding this comment

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

What this one was doing originally?

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm, I see it's a blank function

},
base: {
pid: process.pid,
serviceName: settings.serviceName,
serviceVersion: settings.version,
platform: values.hostingPlatform,
tenant: values.tenant,
platform: configSet.platform,
tenant: configSet.tenant,
},
transport,
autoLogging: !['test', 'local'].includes(process.env.NODE_ENV),
/**
* These custom props are only added to 'request completed' and 'request errored' logs.
* Logs generated during request processing won't have these props by default.
* To include these or any other custom props in mid-request logs,
* use `PinoLogger.assign(<props>)` explicitly before logging.
*/
customProps: (req: any, res: any) => ({
user: {
userId: req?.user?._id || null,
environmentId: req?.user?.environmentId || null,
organizationId: req?.user?.organizationId || null,
},
authScheme: req?.authScheme,
rateLimitPolicy: res?.rateLimitPolicy,
}),
transport: configSet.transport,
autoLogging: !['test'].includes(process.env.NODE_ENV),
},
};
}

const customRedaction = (value: any, path: string[]) => {
/*
* Logger.
* if (obj.email && typeof obj.email === 'string') {
* obj.email = '[REDACTED]';
* }
*
* return JSON.parse(JSON.stringify(obj));
*/
};

interface ILoggerSettings {
serviceName: string;
version: string;
Expand Down
9 changes: 2 additions & 7 deletions libs/application-generic/src/logging/masking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ const passwordFields = [

const phoneFields = ['homePhone', 'workPhone', 'phone'];

const addressFields = [
'addressLine1',
'addressLine2',
'address',
'cardAddress',
];
const addressFields = ['addressLine1', 'addressLine2', 'address', 'cardAddress'];

const httpFields = ['webhookUrl', 'avatar', 'avatar_url'];

Expand All @@ -34,5 +29,5 @@ export const sensitiveFields = cardFields.concat(
phoneFields,
addressFields,
uuidFields,
httpFields,
httpFields
);
Loading
Loading