Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log objects rather than JSON stringified objects #1922

Merged
merged 1 commit into from
Jun 2, 2016

Conversation

spenthil
Copy link
Contributor

@spenthil spenthil commented May 26, 2016

To easily configure CloudWatch (AWS), each log entry should be on a single line. This makes VERBOSE logging do just that.

In our case - when connecting the parse-server app, we also configure the logger to stringify JSON:

logger.configure({
  level: 'silly',
  transports: [
    new (winston.transports.Console)({
      json: true,
      stringify: true
    })
  ]
});

We can then pipe straight to CloudWatch which ingests the JSON without issue.

@ghost
Copy link

ghost commented May 26, 2016

@spenthil updated the pull request.

@codecov-io
Copy link

codecov-io commented May 26, 2016

Current coverage is 91.87%

Merging #1922 into master will decrease coverage by <.01%

  1. 2 files (not in diff) in src were modified. more
    • Misses +2
    • Hits -2
@@             master      #1922   diff @@
==========================================
  Files            91         91          
  Lines          6399       6400     +1   
  Methods        1094       1094          
  Messages          0          0          
  Branches       1342       1342          
==========================================
- Hits           5881       5880     -1   
- Misses          518        520     +2   
  Partials          0          0          

Powered by Codecov. Last updated by 103839c...eb9677b

@facebook-github-bot
Copy link

@spenthil updated the pull request.

@facebook-github-bot
Copy link

@spenthil updated the pull request.

@ghost
Copy link

ghost commented May 26, 2016

@spenthil updated the pull request.

@ghost
Copy link

ghost commented May 27, 2016

@spenthil updated the pull request.

@ghost
Copy link

ghost commented May 27, 2016

@spenthil updated the pull request.

@ghost
Copy link

ghost commented May 27, 2016

@spenthil updated the pull request.

@@ -155,14 +155,18 @@ export default class PromiseRouter {
function makeExpressHandler(promiseHandler) {
return function(req, res, next) {
try {
log.verbose(req.method, maskSensitiveUrl(req), req.headers,
JSON.stringify(maskSensitiveBody(req), null, 2));
log.verbose('Got request', {
Copy link
Contributor

@flovilmart flovilmart May 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the 'Got request'? or remplace by REQUEST or something more easily greppable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@flovilmart
Copy link
Contributor

@spenthil there as some reconfigure that can be happening (upon log rotation for example). I like your idea of exposing the global logger like that :)

See:

I'm all willing to simplify all that. Maybe even drop the logsFolder thing, the log rotation stuff for basic logging.

What do you think?

@ghost
Copy link

ghost commented May 27, 2016

@spenthil updated the pull request.

@ghost
Copy link

ghost commented May 31, 2016

@spenthil updated the pull request.

@ghost
Copy link

ghost commented Jun 1, 2016

@spenthil updated the pull request.

@spenthil
Copy link
Contributor Author

spenthil commented Jun 1, 2016

The dashboard only pulls out the message field from the logged object - since this logs objects as meta (winston concept), they don't appear. This is only an issue with VERBOSE log messages. I've updated the PR so some information is included in parse dashboard (http://files.spenthil.com/wZTQ). The rest of the fields are present in the files.

@drew-gross is this a reasonable compromise? This results in the information being minimally logged twice - some stuff unstructured (in message for parse-dashboard) and everything as structured (JSON field).

http://files.spenthil.com/wZTQ
http://files.spenthil.com/154ph

@drew-gross
Copy link
Contributor

Yes I'd say thats fairly reasonable. Maybe you could include an update to the docs as well to make it clear what the default is and how to get more info? @flovilmart I'll let you do the final go-ahead/merge on this one.

@ghost
Copy link

ghost commented Jun 1, 2016

@spenthil updated the pull request.

@spenthil
Copy link
Contributor Author

spenthil commented Jun 1, 2016

@drew-gross: updated docs

@ghost
Copy link

ghost commented Jun 1, 2016

@spenthil updated the pull request.

@TylerBrock TylerBrock merged commit bea655e into parse-community:master Jun 2, 2016
@drew-gross
Copy link
Contributor

Not having the full request and response body is interfering with my debugging flow a lot more than I thought :( I may have to revert this.

drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 6, 2016
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 6, 2016
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 6, 2016
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 8, 2016
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 10, 2016
drew-gross added a commit that referenced this pull request Jun 11, 2016
* Add unique indexing

* Add unique indexing for username/email

* WIP

* Finish unique indexes

* Notes on how to upgrade to 2.3.0 safely

* index on unique-indexes: c454180 Revert "Log objects rather than JSON stringified objects (#1922)"

* reconfigure username/email tests

* Start dealing with test shittyness

* Remove tests for files that we are removing

* most tests passing

* fix failing test

* Make specific server config for tests async

* Fix more tests

* fix more tests

* Fix another test

* fix more tests

* Fix email validation

* move some stuff around

* Destroy server to ensure all connections are gone

* Fix broken cloud code

* Save callback to variable

* no need to delete non existant cloud

* undo

* Fix all tests where connections are left open after server closes.

* Fix issues caused by missing gridstore adapter

* Update guide for 2.3.0 and fix final tests

* use strict

* don't use features that won't work in node 4

* Fix syntax error

* Fix typos

* Add duplicate finding command

* Update 2.3.0.md
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 11, 2016
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 12, 2016
drew-gross added a commit that referenced this pull request Jun 12, 2016
* Remove adaptiveCollection

* Remove an adaptiveCollection use

* Remove an adaptiveCollection

* make adaptiveCollection private

* Remove collection from mongoadapter

* Move schema collection usage into mongo adapter

* stop relying on mongo format for removing join tables

* reduce usage of schemaCollection

* remove uses of _collection

* Move CLP setting into mongo adapter

* remove all uses of schemaCollection

* make schemaCollection private

* remove transform from schemaCollection

* rename some stuff

* Tweak paramaters and stuff

* reorder some params

* reorder find() arguments

* finishsh touching up argument order

* Accept a database adapter as a parameter

* First passing test with postgres!

* Actually use the provided className

* index on unique-indexes: c454180 Revert "Log objects rather than JSON stringified objects (#1922)"

* Start dealing with test shittyness

* Make specific server config for tests async

* Fix email validation

* Fix broken cloud code

* Save callback to variable

* undo

* Fix tests

* Setup travis

* fix travis maybe

* try removing db user

* indentation?

* remove postgres version setting

* sudo maybe?

* use postgres username

* fix check for _PushStatus

* excludes

* remove db=mongo

* allow postgres to fail

* Fix allow failure

* postgres 9.4

* Remove mongo implementations and fix test

* Fix test leaving behind connections
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 13, 2016
… stringified objects (parse-community#1922)"

reconfigure username/email tests

Fix broken cloud code

Save callback to variable

undo

Fix all tests where connections are left open after server closes.

Fix issues caused by missing gridstore adapter

remove uses of _collection

reorder find() arguments

Accept a database adapter as a parameter

sudo maybe?

use postgres username

reorder find() arguments

Build objects with default fields correctly

Don't tell adapter about ACL

WIP
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 13, 2016
WIP

Notes on how to upgrade to 2.3.0 safely

index on unique-indexes: c454180 Revert "Log objects rather than JSON stringified objects (parse-community#1922)"

reconfigure username/email tests

Start dealing with test shittyness

most tests passing

Make specific server config for tests async

Fix more tests

Save callback to variable

undo

remove uses of _collection

reorder some params

reorder find() arguments

finishsh touching up argument order

Accept a database adapter as a parameter

First passing test with postgres!

Fix tests

Setup travis

sudo maybe?

use postgres username

reorder find() arguments

Build objects with default fields correctly

Don't tell adapter about ACL

WIP

Passing postgres test with user

Fix up createdAt, updatedAt, nad _hashed_password handling
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 13, 2016
… stringified objects (parse-community#1922)"

reconfigure username/email tests

Fix broken cloud code

Save callback to variable

undo

Fix all tests where connections are left open after server closes.

Fix issues caused by missing gridstore adapter

remove uses of _collection

reorder find() arguments

Accept a database adapter as a parameter

sudo maybe?

use postgres username

reorder find() arguments

Build objects with default fields correctly

Don't tell adapter about ACL

WIP
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 13, 2016
WIP

Notes on how to upgrade to 2.3.0 safely

index on unique-indexes: c454180 Revert "Log objects rather than JSON stringified objects (parse-community#1922)"

reconfigure username/email tests

Start dealing with test shittyness

most tests passing

Make specific server config for tests async

Fix more tests

Save callback to variable

undo

remove uses of _collection

reorder some params

reorder find() arguments

finishsh touching up argument order

Accept a database adapter as a parameter

First passing test with postgres!

Fix tests

Setup travis

sudo maybe?

use postgres username

reorder find() arguments

Build objects with default fields correctly

Don't tell adapter about ACL

WIP

Passing postgres test with user

Fix up createdAt, updatedAt, nad _hashed_password handling
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 15, 2016
… stringified objects (parse-community#1922)"

reconfigure username/email tests

Fix broken cloud code

Save callback to variable

undo

Fix all tests where connections are left open after server closes.

Fix issues caused by missing gridstore adapter

remove uses of _collection

reorder find() arguments

Accept a database adapter as a parameter

sudo maybe?

use postgres username

reorder find() arguments

Build objects with default fields correctly

Don't tell adapter about ACL

WIP
drew-gross added a commit to drew-gross/parse-server that referenced this pull request Jun 15, 2016
WIP

Notes on how to upgrade to 2.3.0 safely

index on unique-indexes: c454180 Revert "Log objects rather than JSON stringified objects (parse-community#1922)"

reconfigure username/email tests

Start dealing with test shittyness

most tests passing

Make specific server config for tests async

Fix more tests

Save callback to variable

undo

remove uses of _collection

reorder some params

reorder find() arguments

finishsh touching up argument order

Accept a database adapter as a parameter

First passing test with postgres!

Fix tests

Setup travis

sudo maybe?

use postgres username

reorder find() arguments

Build objects with default fields correctly

Don't tell adapter about ACL

WIP

Passing postgres test with user

Fix up createdAt, updatedAt, nad _hashed_password handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants