Implement cached logging capability

[ptvrajsk]

Fixes #615

Summary

Caching of logs will now be done on localStorage which will be stored on the user's browser. We will be maintaining 4 sessions worth of logs in the cache but this number can be adjusted as per the app's requirement. A "Download Log" button has been added to the header should the user need to download the logs as a text file and submit it to us for inspection/troubleshooting.

Download Log Button

Log Sample

====== New CATcher Session Log ======
3/11/2021, 12:35:53 AM
AppConfig,[object Object]
Mode web

====== New CATcher Session Log ======
3/11/2021, 12:39:53 AM
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615394725773-6c82bc7e-8307-4546-95ee-c6820ba6618c

====== New CATcher Session Log ======
3/11/2021, 12:45:40 AM
Successfully authenticated with session: 1615394740443-ff8de1e6-413c-4da9-b503-8a5e1c8e03dc

====== New CATcher Session Log ======
3/12/2021, 1:39:21 AM
AppConfig,[object Object]
Mode web
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615484371414-7c741ad6-e2c5-4a70-90f8-053bf93a709b

Note

• 1st Log is a sample from running the application in Test mode (i.e. npm run ng:serve:test but not logging in
• 2nd Log is a sample where the app is run in Test mode and login is successful
• 3rd Log is a sample where the user logs out and logs back in without refreshing the page (i.e. App Services are not triggered from scratch).
• 4th Log is sample from Dev Version (i.e. npm run ng:serve:web)(with actual authentication). The repetition is caused by the logs from the OAuth Window
  [Note: The last log does not contain AppConfig ... as the the application was not restarted, the user simply logged out and back in]

Also Note

Since this is stored in localStorage the information can only be wiped programmatically or when the user clears their browser cache. There is a limit (typically 5-10MB) depending on the browser but I don't think we'd ever hit that especially since we are planning to limit the information based on some number of sessions (4 Sessions worth of Logs will currently be kept in the cache).

---

Proposed Commit Message:

Caching Logs in Browser

Log Information will now be cached on browser (through localStorage)
for retrieval later on when troubleshooting user issues. This Log can be
downloaded from the Application Header as a .txt file.

[codecov-io]

Codecov Report

Merging #616 (0862dd8) into master (22a652f) will decrease coverage by 0.10%.
The diff coverage is 61.81%.

@@            Coverage Diff             @@
##           master     #616      +/-   ##
==========================================
- Coverage   67.80%   67.70%   -0.11%     
==========================================
  Files          75       75              
  Lines        2196     2251      +55     
  Branches      202      207       +5     
==========================================
+ Hits         1489     1524      +35     
- Misses        664      683      +19     
- Partials       43       44       +1     

Impacted Files	Coverage Δ
src/app/core/services/logging.service.ts	61.42% <61.81%> (+1.42%)	⬆️
src/app/shared/issue-tables/issue-sorter.ts	48.00% <0.00%> (+4.00%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22a652f...0862dd8. Read the comment docs.

[dingyuchen]

LGTM! Just 1 discussion point

[dingyuchen]

I like the abstraction to a variable log count limit!
Have a few suggestions and also I think separate was spelled wrongly 😆
Looks good otherwise!

[anubh-v]

Sorry, I need some clarification about the intended scope of this PR.

In your PR summary:

Using regex, I've trimmed it such that each new session will wipe the existing logs and store data referring to the current session

But the log sample seems to show logs for several sessions (correct me if I'm wrong) -- are we keeping logs for multiple session or single session?

Saving multiple session entries will be done in another PR along with the ability to procure the logs as a file

But this PR also adds a "Download Log" button that creates a log file --> is the PR summary out-of-date ?

[anubh-v]

I tested the logging with a clean cache on Chrome.
Upon logging into CATcher, I downloaded the logs.
I see:

====== New CATcher Session Log ======
3/11/2021, 5:29:19 PM
AppConfig,[object Object]
Mode web

====== New CATcher Session Log ======
3/11/2021, 5:29:26 PM
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615454973810-5cdc7fdc-2801-49bc-8566-baf0fd9bf3a0

I understand the AppConfig and Mode web lines are logged twice as we load AppComponent a second time when GitHub redirects back to our app.

However, should all the statements be logged under a single New CATcher Session Log ?
I'm not very clear why we log New CATcher Session Log twice.
Is the current behaviour intended?

[ptvrajsk]

I understand the AppConfig and Mode web lines are logged twice as we load AppComponent a second time when GitHub redirects back to our app.

However, should all the statements be logged under a single New CATcher Session Log ?
I'm not very clear why we log New CATcher Session Log twice.
Is the current behaviour intended?

@anubh-v

Was a little stumped on the method to fix this but the issue is caused by the OAuth Pop-up window. On redirection from the GH Authentication URL, it goes back to another instance of our application which the 'new' LoggingService triggers a second start session. Was initially looking into ways to prevent this redirection and have the pop-up close on successful authentication in the Pop-up window but found an easier way and that is to only let the startSession be called by the primary window and not the pop-up (check the window type in startSession itself). But this would create a log that looks like

====== New CATcher Session Log ======
3/12/2021, 1:07:34 AM
AppConfig,[object Object]
Mode web
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615482474491-6b2f108c-ae2b-45dd-86f9-4bdb50cf39e1

Which is caused by the fact that any logs printed in the pop-up window is saved to the log. I think this is ok but if need be I can modify the logging functions to check where they are logged from and attach some sort of identifier to the string for us to easily differentiate between the windows. Shall I do that?

[anubh-v]

====== New CATcher Session Log ======
3/12/2021, 1:07:34 AM
AppConfig,[object Object]
Mode web
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615482474491-6b2f108c-ae2b-45dd-86f9-4bdb50cf39e1

This seems OK in my view.
The main goal would be to keep all logs for a single login under a single ====== New CATcher Session Log ====== header, which is done here.

The duplicate AppConfig and Mode web lines, from the redirect in the pop-up window, are not a big issue.

if need be I can modify the logging functions to check where they are logged from and attach some sort of identifier to the string for us to easily differentiate between the windows. Shall I do that?

No need, in my view, as the pop-up window's role is quite limited.
When we add logging for the code specific to the pop-up, the log statements themselves would make it clear that they are not from the main application window.

[ptvrajsk]

This seems OK in my view.
The main goal would be to keep all logs for a single login under a single ====== New CATcher Session Log ====== header, which is done here.

The duplicate AppConfig and Mode web lines, from the redirect in the pop-up window, are not a big issue.

No need, in my view, as the pop-up window's role is quite limited.
When we add logging for the code specific to the pop-up, the log statements themselves would make it clear that they are not from the main application window.

Okay, sounds good 👍

[anubh-v]

Thanks for the work on this @ptvrajsk
I noticed another minor issue for discussion:

If a user logs into the app, logs out and then logs in again, then any logs generated "in between" the login attempts, will be stored with the 1st login's logs.

By "in between", I mean "after the log-out", and "before the second log-in is completed".

The issue can be observed by following these steps:

• Load the application in the browser
• Log into the app
• Download the log

We will see:

====== New CATcher Session Log ======
3/12/2021, 10:43:43 PM
AppConfig,[object Object]
Mode web
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615560237424-444d670d-5eb4-4159-9502-0bfbb7aef604

• Next, logout of the app
• Log into the app again
• Download the log

We will see these logs:

====== New CATcher Session Log ======
3/12/2021, 10:43:43 PM
AppConfig,[object Object]
Mode web
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615560237424-444d670d-5eb4-4159-9502-0bfbb7aef604
AppConfig,[object Object]
Mode web

====== New CATcher Session Log ======
3/12/2021, 10:45:07 PM
Successfully authenticated with session: 1615560307475-609dba23-f988-407e-9958-d224b777cd24

Notice that the 3rd set of (AppConfig,[object Object], Mode web ) is added to the 1st session.
This log statement is from the pop-up window on the 2nd login.

In general, any logs generated "after a log-out" and "before the next log-in is complete" will be stored with the former login's logs.

Do you think there is any problem with this behaviour?
Though this is slightly counter-intuitive for debugging, we could go ahead with this behaviour.

Alternatively, we can choose to invoke startSession in AuthComponent's ngOnInit (instead of invoking startSession upon authentication in AuthService). This should ensure any logs generated after a log-out, are stored under a new session.

[ptvrajsk]

Do you think there is any problem with this behaviour?
Though this is slightly counter-intuitive for debugging, we could go ahead with this behaviour.

Alternatively, we can choose to invoke startSession in AuthComponent's ngOnInit (instead of invoking startSession upon authentication in AuthService). This should ensure any logs generated after a log-out, are stored under a new session.

Ah yes I think I missed this edge case since I was considering "sessions" as what happens after authentication. But yes I think I can fix this up fairly quick.

Its now correctly logged like this,

====== New CATcher Session Log ======
3/13/2021, 1:04:47 AM
AppConfig,[object Object]
Mode web
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615568713597-8e7d056f-ea60-4c69-85ed-b80785cf1603

====== New CATcher Session Log ======
3/13/2021, 1:05:15 AM
AppConfig,[object Object]
Mode web
Successfully authenticated with session: 1615568725700-b369e482-43dc-4203-bd98-ab204822bf20

[ptvrajsk]

Thanks for the work on this @ptvrajsk

@anubh-v
Haha no worries man thanks for reviewing this quite a number of things to iron out with this one 😆

[anubh-v]

Good work 👍
There's some follow-up work needed:

• For the web version, let's update the Reporting problems in using CATcher section in the usage docs, so users know how to download the logs. For the electron app, shall we stick to the logs generated by electron-log, as those include log statements from the main process as well?
  Let's update the usage notes in this PR?

• Improve the testing for LoggingService (as discussed here, can do in a separate PR)

• Briefly document how cached logging works in the Dev Notes (lower priority, can do separately)

[ptvrajsk]

Good work 👍
There's some follow-up work needed:

• For the web version, let's update the Reporting problems in using CATcher section in the usage docs, so users know how to download the logs. For the electron app, shall we stick to the logs generated by electron-log, as those include log statements from the main process as well?

Yeah I think we can stick to the logs generated by electron-log for that.

Let's update the usage notes in this PR?

Sure 👍

• Improve the testing for LoggingService (as discussed here, can do in a separate PR)

Yup I'm working on this in a local branch 👍 Will make a draft PR soon.

• Briefly document how cached logging works in the Dev Notes (lower priority, can do separately)

Yes I'll make an issue for this as a reminder as well 👍