feat(logs): improve android logging #5755
Conversation
|
nativescript-cli/lib/common/mobile/device-log-provider.ts Lines 88 to 90 in ba3293c I guess this can be removed now that the |
Probably yes, will have to see which one is better to keep. |
| lineText.match(AndroidLogFilter.API_LEVEL_23_LINE_REGEX) || | ||
| lineText.match(AndroidLogFilter.LINE_REGEX); |
There was a problem hiding this comment.
Note: I flipped this condition to speed up matching by optimizing for API23 first and then falling back to legacy regex.
| !line.includes(options.pid) | ||
| ) { | ||
| this.forceStop(deviceIdentifier); | ||
| options.onAppRestarted?.(); |
There was a problem hiding this comment.
Note: not break;-ing here, since it's possible (though unlikely) that two subsequent START lines come in the same lineBuffer. Though this doesn't really matter - this stream is not noisy at all because we're pre-filtering with the adb logcat regex.
| const acceptedTags = [ | ||
| "chromium", | ||
| '"Web Console"', | ||
| "JS", | ||
| "System.err", | ||
| "TNS.Native", | ||
| "TNS.Java", | ||
| ]; | ||
|
|
||
| if (pid && isLogcatPidSupported) { | ||
| logcatCommand.push(`--pid=${pid}`); | ||
|
|
||
| acceptedTags.forEach((tag) => { | ||
| // -s <tag> - shows only logs with the specified tag | ||
| logcatCommand.push("-s", tag); | ||
| }); |
There was a problem hiding this comment.
@ammarahm-ed this is a great idea to move here - technically if isLogcatPidSupported is false - we'd log out all tags even if not accepted - since the filtering has been removed from android-log-filter, however I don't think this would ever be a problem.
I've extracted the isLogcatPidSupported to a function since it's used by both methods here - and I marked is @deprecated because I don't think we need to worry about that soon anymore (if I understood correctly, that's for API24 or below - android 7.0)...
|
|
||
| // -b system - shows the system buffer/logs only | ||
| // -T 1 - shows only new logs after starting adb logcat | ||
| const logcatCommand = [`logcat`, `-b`, `system`, `-T`, `1`]; |
There was a problem hiding this comment.
Note: Added the -b system flag. Probably not a huge difference, but only showing the system buffer should theoretically be faster than showing the default (system, crash, main).
|
@rigor789 Is there a version to test these changes? |
|
The next tag should have it. |
PR Checklist
What is the current behavior?
Logging on android stops working if the app is restarted. This happens mainly due to logging directly linked with app
PIDwhen it was first launched. If the app restarts or crashes and is relauched, thePIDchanges which causes the logs to stop coming in. The only solution (very bad) is to kill the CLI and run the app again.What is the new behavior?
I wanted to completely ditch
PIDfor something else on android but I realized there are good reasons to keep usingPIDas it helps us filter logs precisely for the current app instance that is running.In the new approach we restart adb logcat with the new PID if the app restarts. This way we always keep the CLI in sync with the current PID of the running NativeScript app. This requires running a separate instance of
adb logcat -T 1 --regex=START.*com.myappidbasically to watch for app START log fromActivityManager/ActivityTaskManagerand then update the PID in the CLI and restart logcat. Yes we are running two instances of adb logcat but both are very lightweight and log precisely what we need by using native adb filters.This PR also makes logging much more performant on android by using native android filters of TAGs of concern instead of filtering on the host device.
JS:from logsCloses #5739