Skip to content
Open
Show file tree
Hide file tree
Changes from 8 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: 2 additions & 0 deletions apps/api/src/app/shared/shared.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
GetDecryptedSecretKey,
InvalidateCacheService,
LoggerModule,
LogLevelService,
QueuesModule,
RequestLogRepository,
StepRunRepository,
Expand Down Expand Up @@ -139,6 +140,7 @@ const PROVIDERS = [
DalServiceHealthIndicator,
featureFlagsService,
InvalidateCacheService,
LogLevelService,
storageService,
...DAL_MODELS,
CreateExecutionDetails,
Expand Down
31 changes: 21 additions & 10 deletions apps/api/src/config/env.validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,27 @@ export const envValidators = {
CLICK_HOUSE_PASSWORD: str({ default: '' }),
}),

// Feature Flags
...Object.keys(FeatureFlagsKeysEnum).reduce(
(acc, key) => {
return {
...acc,
[key as FeatureFlagsKeysEnum]: bool({ default: false }),
};
},
{} as Record<FeatureFlagsKeysEnum, ValidatorSpec<boolean>>
),
// Feature Flags - Boolean (start with IS_)
...Object.keys(FeatureFlagsKeysEnum)
.filter((key) => key.startsWith('IS_'))
.reduce<Record<string, ValidatorSpec<boolean>>>((acc, key) => {
acc[key] = bool({ default: false });

return acc;
}, {}),

// Feature Flags - Numeric (end with _NUMBER)
...Object.keys(FeatureFlagsKeysEnum)
.filter((key) => key.endsWith('_NUMBER'))
.reduce<Record<string, ValidatorSpec<number | undefined>>>((acc, key) => {
acc[key] = num({ default: undefined });

return acc;
}, {}),
Comment on lines +85 to +92
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the MAX_ENVIRONMENT_COUNT definition and any usages
rg -n "MAX_ENVIRONMENT_COUNT" --type=ts -C2

Repository: novuhq/novu

Length of output: 1239


🏁 Script executed:

#!/bin/bash
# Check the env.validators.ts file to see how feature flags are validated
cat -n apps/api/src/config/env.validators.ts | sed -n '70,100p'

Repository: novuhq/novu

Length of output: 1380


🏁 Script executed:

#!/bin/bash
# Check if MAX_ENVIRONMENT_COUNT is explicitly validated anywhere in env.validators.ts
rg -n "MAX_ENVIRONMENT_COUNT" apps/api/src/config/env.validators.ts

Repository: novuhq/novu

Length of output: 37


🏁 Script executed:

#!/bin/bash
# Get all keys in FeatureFlagsKeysEnum to see the complete list
cat -n packages/shared/src/types/feature-flags.ts | grep -A 30 "enum FeatureFlagsKeysEnum"

Repository: novuhq/novu

Length of output: 2226


🏁 Script executed:

#!/bin/bash
# Get the rest of FeatureFlagsKeysEnum to see numeric flags and MAX_ENVIRONMENT_COUNT
cat -n packages/shared/src/types/feature-flags.ts | sed -n '58,105p'

Repository: novuhq/novu

Length of output: 3772


MAX_ENVIRONMENT_COUNT won't get a numeric validator.

The filter key.endsWith('_NUMBER') will miss MAX_ENVIRONMENT_COUNT in FeatureFlagsKeysEnum. It's defined as a numeric flag but doesn't follow the _NUMBER suffix convention, so it won't receive a numeric validator.

🤖 Prompt for AI Agents
In `@apps/api/src/config/env.validators.ts` around lines 85 - 92, The numeric
validator block is skipping MAX_ENVIRONMENT_COUNT because the filter uses
key.endsWith('_NUMBER'); update the filter or add an explicit inclusion so
MAX_ENVIRONMENT_COUNT gets a numeric validator: modify the .filter(...) used on
Object.keys(FeatureFlagsKeysEnum) to also accept key === 'MAX_ENVIRONMENT_COUNT'
(or a more general pattern that matches this enum name), ensuring the reduce
still assigns acc[key] = num({ default: undefined }) for that key; reference
FeatureFlagsKeysEnum, the current .filter(key => key.endsWith('_NUMBER')), and
the reducer that sets acc[key] = num(...) when making the change.


// Feature Flags - String
CF_SCHEDULER_MODE_STR: str({ choices: ['off', 'shadow', 'live', 'complete'], default: 'off' }),
LOG_LEVEL_STR: str({ choices: ['trace', 'debug', 'info', 'warn', 'error', 'fatal', 'none'], default: undefined }),

// Azure validators
...(processEnv.STORAGE_SERVICE === 'AZURE' && {
Expand Down
4 changes: 3 additions & 1 deletion apps/webhook/src/shared/shared.module.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Module } from '@nestjs/common';
import { AnalyticsService } from '@novu/application-generic';
import { AnalyticsService, featureFlagsService, LogLevelService } from '@novu/application-generic';
import { DalService, ExecutionDetailsRepository, IntegrationRepository, MessageRepository } from '@novu/dal';

const DAL_MODELS = [ExecutionDetailsRepository, MessageRepository, IntegrationRepository];
Expand All @@ -26,6 +26,8 @@ const PROVIDERS = [
return analyticsService;
},
},
featureFlagsService,
LogLevelService,
];

@Module({
Expand Down
2 changes: 2 additions & 0 deletions apps/worker/src/app/shared/shared.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
GetTenant,
InvalidateCacheService,
LoggerModule,
LogLevelService,
MetricsModule,
ProcessTenant,
QueuesModule,
Expand Down Expand Up @@ -110,6 +111,7 @@ const PROVIDERS = [
DigestFilterSteps,
featureFlagsService,
InvalidateCacheService,
LogLevelService,
StorageHelperService,
storageService,
UpdateSubscriber,
Expand Down
4 changes: 4 additions & 0 deletions apps/ws/src/shared/shared.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { JwtModule } from '@nestjs/jwt';
import {
AnalyticsService,
DalServiceHealthIndicator,
featureFlagsService,
LogLevelService,
QueuesModule,
WebSocketsInMemoryProviderService,
} from '@novu/application-generic';
Expand Down Expand Up @@ -37,6 +39,8 @@ const PROVIDERS = [
analyticsService,
dalService,
DalServiceHealthIndicator,
featureFlagsService,
LogLevelService,
SubscriberOnlineService,
WebSocketsInMemoryProviderService,
...DAL_MODELS,
Expand Down
6 changes: 3 additions & 3 deletions libs/application-generic/src/logging/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function getErrorInterceptor(): NestInterceptor {
return new LoggerErrorInterceptor();
}

const loggingLevelSet = {
export const loggingLevelSet = {
trace: 10,
debug: 20,
info: 30,
Expand All @@ -19,7 +19,7 @@ const loggingLevelSet = {
fatal: 60,
none: 70,
};
const loggingLevelArr = Object.keys(loggingLevelSet);
export const loggingLevelArr = Object.keys(loggingLevelSet);

export function getLogLevel() {
let logLevel = null;
Expand Down Expand Up @@ -76,7 +76,7 @@ export function createNestLoggingModuleOptions(settings: {
return {
exclude: [
{ path: '*/health-check', method: RequestMethod.GET },
{ path: '/v1/internal/subscriber-online-state', method: RequestMethod.POST }
{ path: '/v1/internal/subscriber-online-state', method: RequestMethod.POST },
],
assignResponse: true,
pinoHttp: {
Expand Down
1 change: 1 addition & 0 deletions libs/application-generic/src/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export * from './content.service';
export * from './cron';
export * from './feature-flags';
export * from './in-memory-provider';
export * from './log-level';
export {
MessageInteractionResult,
MessageInteractionService,
Expand Down
1 change: 1 addition & 0 deletions libs/application-generic/src/services/log-level/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './log-level.service';
102 changes: 102 additions & 0 deletions libs/application-generic/src/services/log-level/log-level.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common';
import { FeatureFlagsKeysEnum } from '@novu/shared';
import { PinoLogger } from 'nestjs-pino';
import { getLogLevel, loggingLevelArr } from '../../logging';
import { FeatureFlagsService } from '../feature-flags';

const LOG_CONTEXT = 'LogLevelService';
const DEFAULT_POLLING_INTERVAL_MS = 60_000; // one minute

@Injectable()
export class LogLevelService implements OnModuleInit, OnModuleDestroy {
private pollingInterval: NodeJS.Timeout | null = null;
private currentLogLevel: string;
private readonly pollingIntervalMs: number;

constructor(
private featureFlagsService: FeatureFlagsService,
private logger: PinoLogger
) {
this.pollingIntervalMs = Number(process.env.LOG_LEVEL_POLLING_INTERVAL_MS) || DEFAULT_POLLING_INTERVAL_MS;
this.currentLogLevel = getLogLevel();
}

async onModuleInit(): Promise<void> {
await this.updateLogLevel();

this.pollingInterval = setInterval(async () => {
try {
await this.updateLogLevel();
} catch (error) {
Logger.error(`Failed to update log level: ${(error as Error).message}`, (error as Error).stack, LOG_CONTEXT);
}
}, this.pollingIntervalMs);

Logger.log(`Log level polling started with interval of ${this.pollingIntervalMs}ms`, LOG_CONTEXT);
}

onModuleDestroy(): void {
if (this.pollingInterval) {
clearInterval(this.pollingInterval);
this.pollingInterval = null;
Logger.log('Log level polling stopped', LOG_CONTEXT);
}
}

private async updateLogLevel(): Promise<void> {
const logLevelFromFlag = await this.getLogLevelFromFeatureFlag();

const newLogLevel = logLevelFromFlag || this.getFallbackLogLevel();

if (!this.isValidLogLevel(newLogLevel)) {
Logger.warn(
`Invalid log level "${newLogLevel}". Valid levels: ${loggingLevelArr.join(', ')}. Keeping current level: ${this.currentLogLevel}`,
LOG_CONTEXT
);

return;
}

if (newLogLevel !== this.currentLogLevel) {
this.setLogLevel(newLogLevel);
Logger.log(`Log level changed from "${this.currentLogLevel}" to "${newLogLevel}"`, LOG_CONTEXT);
this.currentLogLevel = newLogLevel;
}
}

private async getLogLevelFromFeatureFlag(): Promise<string | undefined> {
try {
const flagValue = await this.featureFlagsService.getFlag<string | undefined>({
key: FeatureFlagsKeysEnum.LOG_LEVEL_STR,
defaultValue: undefined,
user: { _id: 'system' },
});

if (flagValue && flagValue !== 'undefined') {
return flagValue;
}

return undefined;
} catch (error) {
Logger.warn(`Failed to get log level from feature flag: ${(error as Error).message}`, LOG_CONTEXT);

return undefined;
}
}

private getFallbackLogLevel(): string {
return process.env.LOG_LEVEL || process.env.LOGGING_LEVEL || 'info';
}

private isValidLogLevel(level: string): boolean {
return loggingLevelArr.includes(level);
}

private setLogLevel(level: string): void {
if ((this.logger as any).pinoLogger) {
(this.logger as any).pinoLogger.level = level;
} else if ((this.logger as any).logger) {
(this.logger as any).logger.level = level;
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class StandardQueueService extends QueueBaseService {
}

const schedulerMode = await this.featureFlagsService.getFlag<string>({
key: FeatureFlagsKeysEnum.CF_SCHEDULER_MODE,
key: FeatureFlagsKeysEnum.CF_SCHEDULER_MODE_STR,
defaultValue: CloudflareSchedulerMode.OFF,
organization: { _id: data.data._organizationId, apiServiceLevel: organization.apiServiceLevel },
environment: { _id: data.data._environmentId },
Expand Down
7 changes: 4 additions & 3 deletions packages/shared/src/types/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ export enum FeatureFlagsKeysEnum {
IS_BILLING_USAGE_CLICKHOUSE_ENABLED = 'IS_BILLING_USAGE_CLICKHOUSE_ENABLED',
IS_BILLING_USAGE_CLICKHOUSE_SHADOW_ENABLED = 'IS_BILLING_USAGE_CLICKHOUSE_SHADOW_ENABLED',
IS_BILLING_USAGE_DETAILED_DIAGNOSTICS_ENABLED = 'IS_BILLING_USAGE_DETAILED_DIAGNOSTICS_ENABLED',
IS_ANALYTICS_PAGE_ENABLED = 'IS_ANALYTICS_PAGE_ENABLED',
IS_LEGACY_SELECTOR_BUTTON_VISIBLE = 'IS_LEGACY_SELECTOR_BUTTON_VISIBLE',
Comment on lines +89 to +90
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n packages/shared/src/types/feature-flags.ts | head -120

Repository: novuhq/novu

Length of output: 7466


🏁 Script executed:

rg -n "testFlagEnumValidity" --type=ts -A5 -B5

Repository: novuhq/novu

Length of output: 3344


🏁 Script executed:

rg -n "BooleanFlagKey|NumericFlagKey" --type=ts -A2 -B2

Repository: novuhq/novu

Length of output: 841


🏁 Script executed:

rg -n "testFlagEnumValidity.*FeatureFlagsKeysEnum" --type=ts

Repository: novuhq/novu

Length of output: 37


🏁 Script executed:

rg -A5 "export enum FeatureFlagsKeysEnum" packages/shared/src/types/feature-flags.ts

Repository: novuhq/novu

Length of output: 412


IS_LEGACY_SELECTOR_BUTTON_VISIBLE violates the BooleanFlagKey naming convention.

The BooleanFlagKey type (line 5) requires flags to end with _ENABLED or _DISABLED, but IS_LEGACY_SELECTOR_BUTTON_VISIBLE ends with _VISIBLE. Rename to follow the pattern.

Suggested fix
-  IS_LEGACY_SELECTOR_BUTTON_VISIBLE = 'IS_LEGACY_SELECTOR_BUTTON_VISIBLE',
+  IS_LEGACY_SELECTOR_BUTTON_ENABLED = 'IS_LEGACY_SELECTOR_BUTTON_ENABLED',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
IS_ANALYTICS_PAGE_ENABLED = 'IS_ANALYTICS_PAGE_ENABLED',
IS_LEGACY_SELECTOR_BUTTON_VISIBLE = 'IS_LEGACY_SELECTOR_BUTTON_VISIBLE',
IS_ANALYTICS_PAGE_ENABLED = 'IS_ANALYTICS_PAGE_ENABLED',
IS_LEGACY_SELECTOR_BUTTON_ENABLED = 'IS_LEGACY_SELECTOR_BUTTON_ENABLED',
🤖 Prompt for AI Agents
In `@packages/shared/src/types/feature-flags.ts` around lines 89 - 90, The enum
value IS_LEGACY_SELECTOR_BUTTON_VISIBLE violates the BooleanFlagKey naming
convention; rename the enum member to follow the _ENABLED/_DISABLED pattern
(e.g., IS_LEGACY_SELECTOR_BUTTON_ENABLED) and update its string value to match,
then update all references/usages (imports, tests, feature-flag lookups, and any
persisted keys) to the new symbol; ensure BooleanFlagKey type usage and any
switch/conditionals that check IS_LEGACY_SELECTOR_BUTTON_VISIBLE are updated to
the new name so compilation and runtime behavior remain correct.


// String flags
CF_SCHEDULER_MODE = 'CF_SCHEDULER_MODE', // Values: "off" | "shadow" | "live" | "complete"
CF_SCHEDULER_MODE_STR = 'CF_SCHEDULER_MODE_STR', // Values: "off" | "shadow" | "live" | "complete"
Comment thread
djabarovgeorge marked this conversation as resolved.
LOG_LEVEL_STR = 'LOG_LEVEL_STR', // Values: "trace" | "debug" | "info" | "warn" | "error" | "fatal" | "none"

// Numeric flags
MAX_WORKFLOW_LIMIT_NUMBER = 'MAX_WORKFLOW_LIMIT_NUMBER',
Expand All @@ -98,8 +101,6 @@ export enum FeatureFlagsKeysEnum {
LOG_EXPIRATION_DAYS_NUMBER = 'LOG_EXPIRATION_DAYS_NUMBER',
MAX_DATE_ANALYTICS_ENABLED_NUMBER = 'MAX_DATE_ANALYTICS_ENABLED_NUMBER',
MAX_ENVIRONMENT_COUNT = 'MAX_ENVIRONMENT_COUNT',
IS_ANALYTICS_PAGE_ENABLED = 'IS_ANALYTICS_PAGE_ENABLED',
IS_LEGACY_SELECTOR_BUTTON_VISIBLE = 'IS_LEGACY_SELECTOR_BUTTON_VISIBLE',
}

export enum CloudflareSchedulerMode {
Expand Down
Loading