feat(ui): logs panel #94
Conversation
| { name: 'Fatal', value: LogPriority.FATAL }, | ||
| ] | ||
|
|
||
| export const Filter = observer(({ column }: { column: Column<LogcatEntryMessage, unknown> }) => { |
|
|
||
| const FILE_EXTENSION_OPTIONS: SelectOption<LogsFileExtension>[] = [ | ||
| { name: 'JSON', value: 'json' }, | ||
| { name: 'LOG', value: 'log' }, |
There was a problem hiding this comment.
Is it translatable?
Maybe more human string can be used here like JSON file or Raw lines (.log)
| /* NOTE: Do not mutate this object to ensure stable references for React Table. | ||
| For more details, see: https://tanstack.com/table/latest/docs/guide/data#give-data-a-stable-reference | ||
| */ |
There was a problem hiding this comment.
Judging by the constructor() this comment block should relate only to batchedLogs member
There was a problem hiding this comment.
And if an object should not mutate, maybe use readonly or const?
There was a problem hiding this comment.
I can't do this because of that
| }) | ||
|
|
||
| this.onLogcatEntry = this.onLogcatEntry.bind(this) | ||
| this.debouncedFlushLogs = debounce(this.flushLogs.bind(this), 100) |
There was a problem hiding this comment.
This should be throttle instead of debounce
There was a problem hiding this comment.
That's a very good point, I agree
| this.debouncedFlushLogs(data.serial) | ||
| } | ||
|
|
||
| private debouncedFlushLogs: (serial: string) => void |
There was a problem hiding this comment.
Why not implement the debouncedFlushLogs here?
| this.logsByDeviceSerial = { | ||
| ...this.logsByDeviceSerial, | ||
| [serial]: { | ||
| ...this.logsByDeviceSerial[serial], | ||
| logs: [...this.logsByDeviceSerial[serial].logs, ...this.batchedLogs], | ||
| }, | ||
| } |
There was a problem hiding this comment.
This will very quickly drain the memory for prolonged logcat sessions.
Android devices generate A LOG of logs.
We need to find a way to store them without the copy overhead.
Maybe even implement filters when receiving the log entries so we don't even store the unnecessary logs
There was a problem hiding this comment.
There is a limit of 3000 logs per device
I've also implemented filters when receiving the logs
| this.setLogcatStarted(serial, false) | ||
| } | ||
|
|
||
| clearDeviceLogs(serial: string): void { |
There was a problem hiding this comment.
This should also be called when we release the device or the device panel is no longer shown/used
| UNKNOWN, | ||
| DEFAULT, | ||
| VERBOSE, | ||
| DEBUG, | ||
| INFO, | ||
| WARN, | ||
| ERROR, | ||
| FATAL, | ||
| SILENT, |
There was a problem hiding this comment.
I think that is the third time I see this enumeration.
It's mostly OK but what if something can be done about it?
| clearTimeout(lastFn) | ||
|
|
||
| lastFn = setTimeout( |
There was a problem hiding this comment.
Every call to throttle while it's inTrottle will clear the last timeout.
ui/src/lib/utils/throttle.util.ts
Outdated
| if (!inThrottle) { | ||
| fn.apply(this, args) | ||
| lastTime = Date.now() | ||
| inThrottle = true |
There was a problem hiding this comment.
inThrottle is never set to false after it's not in throttle mode
| let inThrottle: boolean | ||
| let lastFn: ReturnType<typeof setTimeout> | ||
|
|
||
| return function func(this: any, ...args: any[]): void { |
There was a problem hiding this comment.
I think a better implementation would be as such:
- if we don't have a pending timeout - call the function immediately and start a timeout
- if we have a timeout - set a flag that the timeout should call the function
- after the timeout is reached - call the function if the flag is set
oshcherbakovv
force-pushed
the
o.shcherbakov/logs-panel/QA-16029
branch
from
51d3066 to
8c0fccf
Compare
No description provided.