feat(api-service): update logger level runtime#9902
feat(api-service): update logger level runtime#9902djabarovgeorge wants to merge 10 commits intonextfrom
Conversation
github-actions
bot
added
@novu/webhook
@novu/ws
@novu/shared
@novu/worker
@novu/api-service
labels
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
had a problem deploying
to
Linting
GitHub Actions
Error
djabarovgeorge
had a problem deploying
to
Linting
GitHub Actions
Error
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
WalkthroughAdds a new LogLevelService that reads the log level from the feature flag LOG_LEVEL_STR with fallbacks to LOG_LEVEL / LOGGING_LEVEL, validates against exported loggingLevelArr/loggingLevelSet, and polls for changes at runtime. Exposes the service via multiple SharedModules (api, webhook, worker, ws). Publishes logging constants and a log-level service barrel in application-generic. Updates feature flag types (adds LOG_LEVEL_STR, renames CF_SCHEDULER_MODE to CF_SCHEDULER_MODE_STR, moves two flags), adjusts env validators to validate boolean, numeric, and specific string flags, and switches scheduler usage to CF_SCHEDULER_MODE_STR. 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
…hq/novu into update-logger-level-runtime
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
…ndling - Updated env.validators.ts to categorize feature flags into Boolean, Numeric, and String types. - Refactored log-level.service.ts to use new feature flag keys for LOG_LEVEL and added a default polling interval. - Adjusted standard-queue.service.ts to reference the updated CF_SCHEDULER_MODE_STR flag. - Added new feature flag keys IS_ANALYTICS_PAGE_ENABLED and IS_LEGACY_SELECTOR_BUTTON_VISIBLE in feature-flags.ts.
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
djabarovgeorge
temporarily deployed
to
Linting
GitHub Actions
Inactive
LaunchDarkly flag references🔍 4 flags added or modified
❌ 1 flag removed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/types/feature-flags.ts (1)
96-104:MAX_ENVIRONMENT_COUNTdoesn't follow theNumericFlagKeynaming convention.The
NumericFlagKeytype (line 6) expects numeric flags to end with_NUMBER, butMAX_ENVIRONMENT_COUNTdoes not. This inconsistency means it won't be picked up by the numeric validator filter inenv.validators.tsand may fail type checking withtestFlagEnumValidity.💡 Suggested fix to follow the naming convention
- MAX_ENVIRONMENT_COUNT = 'MAX_ENVIRONMENT_COUNT', + MAX_ENVIRONMENT_COUNT_NUMBER = 'MAX_ENVIRONMENT_COUNT_NUMBER',
🤖 Fix all issues with AI agents
In `@apps/api/src/config/env.validators.ts`:
- Around line 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.
In `@libs/application-generic/src/services/log-level/log-level.service.ts`:
- Around line 95-101: Replace the fragile any-casts in setLogLevel by using the
public NestJS PinoLogger root API: instead of touching (this.logger as
any).pinoLogger or .logger, set PinoLogger.root.level directly (import or
reference PinoLogger and assign its static/root level) to update the log level;
update the setLogLevel method to read the incoming level and set
PinoLogger.root.level = level, removing the any casts and internal property
checks on this.logger.
In `@packages/shared/src/types/feature-flags.ts`:
- Around line 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.
🧹 Nitpick comments (2)
libs/application-generic/src/services/log-level/log-level.service.ts (2)
67-85: Clarify the string'undefined'check.Line 75 checks both truthiness and explicitly compares against the string
'undefined'. If the feature flag service can return a literal string'undefined', consider adding a comment to document this behavior. Otherwise, the truthy check alone should suffice.💡 Suggested simplification (if string 'undefined' is not expected)
- if (flagValue && flagValue !== 'undefined') { + if (flagValue) { return flagValue; }
60-64: Consider handlingsetLogLevelfailures.If
setLogLevelthrows an exception (e.g., due to unexpected Pino internals), thecurrentLogLevelfield won't be updated, but the exception will propagate and potentially stop the polling. Consider wrappingsetLogLevelin a try-catch to maintain resilience.💡 Suggested defensive handling
if (newLogLevel !== this.currentLogLevel) { + try { this.setLogLevel(newLogLevel); Logger.log(`Log level changed from "${this.currentLogLevel}" to "${newLogLevel}"`, LOG_CONTEXT); this.currentLogLevel = newLogLevel; + } catch (error) { + Logger.warn(`Failed to set log level to "${newLogLevel}": ${(error as Error).message}`, LOG_CONTEXT); + } }
| // 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; | ||
| }, {}), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the MAX_ENVIRONMENT_COUNT definition and any usages
rg -n "MAX_ENVIRONMENT_COUNT" --type=ts -C2Repository: 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.tsRepository: 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.
| IS_ANALYTICS_PAGE_ENABLED = 'IS_ANALYTICS_PAGE_ENABLED', | ||
| IS_LEGACY_SELECTOR_BUTTON_VISIBLE = 'IS_LEGACY_SELECTOR_BUTTON_VISIBLE', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/shared/src/types/feature-flags.ts | head -120Repository: novuhq/novu
Length of output: 7466
🏁 Script executed:
rg -n "testFlagEnumValidity" --type=ts -A5 -B5Repository: novuhq/novu
Length of output: 3344
🏁 Script executed:
rg -n "BooleanFlagKey|NumericFlagKey" --type=ts -A2 -B2Repository: novuhq/novu
Length of output: 841
🏁 Script executed:
rg -n "testFlagEnumValidity.*FeatureFlagsKeysEnum" --type=tsRepository: novuhq/novu
Length of output: 37
🏁 Script executed:
rg -A5 "export enum FeatureFlagsKeysEnum" packages/shared/src/types/feature-flags.tsRepository: 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.
| 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.
1 participant
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer