search

grassrootsgrocery / admin-portal

GNU Affero General Public License v3.0
11 stars 6 forks source link

Server logging should include timestamps #45

Closed mattsahn closed 1 year ago

mattsahn commented 1 year ago

I was looking into an issue that Dan reported of not being able to trigger Make automations so I was looking at the log in Railway to see evidence of that, when it happened, etc. But, there are no timestamps in the logs currently.

image

I see it's using raw console.log() statements which we should upgrade to something better suited for a production app. Below articles have some suggestions on log frameworks to use.

https://blog.appsignal.com/2021/09/01/best-practices-for-logging-in-nodejs.html https://betterstack.com/community/guides/logging/nodejs-logging-best-practices/

zuechai commented 1 year ago

@mattsahn I started working on the logger. Can you take a look at my branch when you have time and let me know if what I've done so far aligns with what you were thinking for this? The file of note is /loggerUtils/logger.ts. The other changes I made are changing console.logs to logger.info.

mattsahn commented 1 year ago

Looks good so far. Winston looks like a good choice in terms of simplicity and popularity. I was expecting new PRs to get auto-deployed by Railway, but realize the current setup against production branch prevents that. I'm going to make a change to point the live app to main, then I might want you to resubmit the PR so we can test that functionality out.

zuechai commented 1 year ago

Resolved in PR #61