tools/helpers/scrub_logs deserves some maintainance

tiol11 commented 1 year ago

Hi and thank you for the great job on this modem FW!

Looks to me that tools/helpers/scrub_logs deserves some maintainance:

modemNum is still 015550199999 but modem phone number is now +22 33 44 55 66 77 (maybe this filtering is no more needed as openqti is no more logging plain phone numbers?)
scrub_logs looks for log files without timestamp at the end of filename, but collect_logs save logs with timestamp at the filename
as Github does not accept uploading filenames like something.log.1, maybe change it to something.1.log?

CodePhase commented 1 year ago

Good looking out @tiol11.

For the modem number, the scrubber uses that logic for a few different log files, not just the openqti log. I'll update it so that it will work with both the old and new numbers; in case someone is using an old version of the firmware.
the older version of collect_logs didn't use timestamps and that's what I wrote scrub_logs to work against originally. Time for an update
changing the log filename might be a call that should be approved by @Biktorgj. I like the suggestion though.

I've been meaning to update scrub_logs so I'm glad you brought this up. I also wanted to ask if it would be worth capturing and censoring the mmsd-tng log. I use a modified version of scrub_logs to do just that when I need to capture logs for mmsd. Maybe that's out of scope though since that's an entirely different project.

CodePhase commented 1 year ago

also, looking at the collect_logs, why is the '.log.1' suffix even necessary? Since the timestamp is now included, can't these files be just '.log'?

CodePhase commented 1 year ago

ok I see that these are for the previous boot log files. Is this really necessary?

tiol11 commented 1 year ago

AFAIK you collect logs after restarting a stuck modem, then you are more interested in the logs from previous boot than in the actual one's.

Biktorgj commented 1 year ago

Good looking out @tiol11.
* For the modem number, the scrubber uses that logic for a few different log files, not just the openqti log.  I'll update it so that it will work with both the old and new numbers; in case someone is using an old version of the firmware.

* the older version of collect_logs didn't use timestamps and that's what I wrote scrub_logs to work against originally. Time for an update

* changing the log filename might be a call that should be approved by @Biktorgj. I like the suggestion though.
I've been meaning to update scrub_logs so I'm glad you brought this up. I also wanted to ask if it would be worth capturing and censoring the mmsd-tng log. I use a modified version of scrub_logs to do just that when I need to capture logs for mmsd. Maybe that's out of scope though since that's an entirely different project.

The only reason to have the file names named as openqti.log.Y is because the script to rotate them does it that way :) https://github.com/the-modem-distro/meta-qcom/blob/kirkstone/recipes-scripts/shared-scripts/files/log_rename.sh

I don't really care about the naming scheme as long as we ensure we rename all the existing files prior to changing the extension, so we don't leave 30 stale files in /persist in everyone's modem for no reason ;)

CodePhase commented 1 year ago

Fair enough. I'll come up with a PR soon and will welcome your input.

CodePhase commented 1 year ago

I made a couple PR's for this. One is to address the log_rename.sh script. I had to rewrite it to contain backwards compatibility with the older naming convention of log files as @Biktorgj required. That said, not only does that need to be merged and installed, it has to run at least once before the updates to collect_logs and scrub_logs will work with the PR I just made

the-modem-distro / pinephone_modem_sdk

tools/helpers/scrub_logs deserves some maintainance #148