Log objects rather than JSON strings and option for single line logs #2028
Conversation
ghost
added
the
|
EDIT: Went a different direction - see #2028 (comment) for updated description.
|
| * to the console | ||
| * daily rotating files as new line delimited JSON | ||
|
|
||
| Logs are also be viewable in Parse Dashboard but it only displays the `messages` field of each log entry. For example, with VERBOSE set this will exclude `origin` on each request. |
There was a problem hiding this comment.
This paragraph is probably unnecessary. Could be shortened to "Logs are also viewable in Parse Dashboard."
|
bump on this one, would be nice to have the logs working on cloudwatch if you have some free cycles @spenthil |
|
Also on GCP, one line JSON's would be nice to read :) |
|
https://cloud.google.com/appengine/articles/logging#filtering_and_finding_logs The recommendations for JSON logging on app engine |
|
@spenthil updated the pull request. |
| @@ -184,6 +184,10 @@ export default { | |||
| env: "VERBOSE", | |||
| help: "Set the logging to verbose" | |||
| }, | |||
| "jsonLogs": { | |||
| env: "JSON_LOGS", | |||
There was a problem hiding this comment.
rename to PARSE_SERVER_JSON_LOGS
|
@spenthil updated the pull request. |
|
Instead of the
|
| @@ -75,5 +90,5 @@ export function addGroup(groupName) { | |||
| return winston.loggers.get(groupName); | |||
| } | |||
|
|
|||
| export { logger }; | |||
| export {logger}; | |||
|
Got a few comments, I'll try it out later to see how it looks like |
This reverts commit fcd914b.
|
@spenthil updated the pull request. |
|
@drew-gross I think it's all good for that PR on my side, should we merge and move forward with it? |
| @@ -59,7 +59,7 @@ describe('verbose logs', () => { | |||
| level: 'verbose' | |||
| }); | |||
| }).then((results) => { | |||
| expect(results[1].message.includes('"password": "********"')).toEqual(true); | |||
There was a problem hiding this comment.
This is no longer testing the thing it should be testing, which is that passwords don't get output to the logs. The original version of this PR caused passwords to start getting output to logs, so I'd like to see these tests either not change, or be a lot more high-level/E2E and test the actual string that is logged rather than some intermediate object.
There was a problem hiding this comment.
I would agree with that. That's majorly blocking
|
I'm a bit concerned about those tests. But as long as the dashboard still shows logs without any effort, I'll leave it up to you. |
Better password stripping tests
|
@spenthil updated the pull request. |
|
@flovilmart added spenthil@bc09f88 - looks like we are good to go now. |
|
Awesome, that looks great. |
|
Good! Let's go with that after Travis |
|
Great Job @spenthil ! |
|
no thank YOU @flovilmart & @drew-gross |
…arse-community#2028) * Log objects rather than JSON strings and option for single line logs This reverts commit fcd914b. * Better password stripping tests
…arse-community#2028) * Log objects rather than JSON strings and option for single line logs This reverts commit fcd914b. * Better password stripping tests
No description provided.