matrix-org / matrix-org/matrix-js-sdk
Modules using a variable binding to `logger.log` do not respect `logger.setLevel`
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.2k
- Forks
- 704
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 40
Description
### Issue
I'm getting noisy logs from this library despite setting `loglevel.getLogger("matrix").disableAll()`.
### Cause
https://github.com/matrix-org/matrix-js-sdk/blob/acbcb4658a5d5903dfd557e3e115241d0a6f38bb/src/models/event-timeline-set.ts#L30-L38
https://github.com/matrix-org/matrix-js-sdk/blob/acbcb4658a5d5903dfd557e3e115241d0a6f38bb/src/timeline-window.ts#L34
`loglevel` applies `setLevel` by replacing the relevant methods on the logger object: https://github.com/pimterry/loglevel/blob/f0187213feb6495630545a34a3b91633db47a1ee/lib/loglevel.js#L86-L97
If `debuglog` binds `logger.log` at the time of module load, `debuglog` will not be updated to respect `setLevel`. This causes noisy logs in my application even when running `loglevel.getLogger("matrix").disableAll()`.
### Possible fixes
1. Use `logger.log` directly instead of `debuglog`. I don't understand why the library is disabling `debuglog` based on the local `DEBUG` constant, but I bet this could be achieved with `loglevel`'s `setLevel`.
2. Forfeit line numbers and use a reference to `logger.log` (e.g. `const debuglog = (...args) => logger.log(...args)`)
3. Some galaxy-brain `Proxy` implementation that I wasn't able to achieve (feel like there's a solution here that would keep the current `DEBUG` behavior *and* get line numbers with logs)
I've implemented option 1 in my branch: https://github.com/davidisaaclee/matrix-js-sdk/commit/9849818efa1d9101e68126af60fba180d7a5f756
Please let me know if there's a better way to get these changes moving than filing Github issues – it seems like you have a lot on your plate, and I'd like to help in an effective way.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the logger bindings in src/models/event-timeline-set.ts and src/timeline-window.ts, then read loglevel's method replacement behavior at the linked implementation. Compare the available fixes against the current DEBUG behavior and line-number requirements. Done means logging from these modules respects getLogger("matrix").disableAll() or setLevel without regressing the stated behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, typescript
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100