Support per extension log level - #2791
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (42.85%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files
... and 4 files with indirect coverage changes
@@ Coverage Diff @@
## improvement/BB-686 #2791 +/- ##
======================================================
- Coverage 75.67% 75.42% -0.26%
======================================================
Files 201 201
Lines 13929 13932 +3
======================================================
- Hits 10541 10508 -33
- Misses 3378 3414 +36
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const log = new werelogs.Logger('Backbeat:IngestionPopulator'); | ||
| werelogs.configure({ level: config.log.logLevel, | ||
| dump: config.log.dumpLevel }); | ||
| const ingestionLogConfig = ingestionExtConfigs.log ?? config.log; |
There was a problem hiding this comment.
Partial extension log config drops the global fallback for omitted fields. If a user sets log: { logLevel: 'debug' } without dumpLevel, the ?? sees a non-nullish object and uses it as-is, so dumpLevel becomes undefined instead of falling back to config.log.dumpLevel. Merge at field level instead:
| const ingestionLogConfig = ingestionExtConfigs.log ?? config.log; | |
| const ingestionLogConfig = { | |
| logLevel: ingestionExtConfigs.log?.logLevel ?? config.log.logLevel, | |
| dumpLevel: ingestionExtConfigs.log?.dumpLevel ?? config.log.dumpLevel, | |
| }; |
There was a problem hiding this comment.
mhh fair enough but im gonna go with the option of making both fields required when specified (If you provide log level you must provide dump level and other way around).
This is how the operator works anyway
ba30395 to
152ea1b
Compare
| const logJoiOptional = | ||
| joi.object({ | ||
| logLevel: joi.alternatives() | ||
| .try('error', 'warn', 'info', 'debug', 'trace').required(), |
There was a problem hiding this comment.
Should it be a const array to avoid duplication ?
| }); | ||
|
|
||
| // logJoi with no default : | ||
| // Callers fall back to the global log config when this one is not configured |
There was a problem hiding this comment.
This is pretty standard behaviour ? Why a comment ?
7e9cc25 to
b350a05
Compare
Hello sylvainsenechal,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
ConflictThere is a conflict between your branch Please resolve the conflict on the feature branch ( git fetch && \
git checkout origin/improvement/BB-807 && \
git merge origin/development/9.5Resolve merge conflicts and commit git push origin HEAD:improvement/BB-807 |
4 participants
Issue: BB-807
Related Zenko Operator : https://github.com/scality/zenko-operator/pull/625