Skip to content

Make parse-server cloud code logging closer parse.com legacy #2550

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

Merged
merged 3 commits into from
Aug 19, 2016

Conversation

acinader
Copy link
Contributor

@acinader acinader commented Aug 19, 2016

Make parse-server cloud code logging closer to parse.com legacy. (fixes #2501)

  1. More closely mimic the wording. Include the user id.

  2. Truncate input and result at 1k char.

  3. Use more sensible metadata that would makes sense to index. The guideline I used was: if it makes sense to filter on, put it in metadata. If it makes sense to "free text" search on, then put it in the message.

    • file and console output, logging an object does not do what on might expect. For example, logging a function's "params":
    expected: 
    
      info: Ran cloud function aFunction for user qWHLVEsbEe with:
        Input: {"foo":"bar","bar":"baz"}
        Result: "it worked!" functionName=aFunction, params= { foo: "bar", "bar": baz }, user=qWHLVEsbEe
    
    what you actually get:
    
      info: Ran cloud function aFunction for user qWHLVEsbEe with:
        Input: {"foo":"bar","bar":"baz"}
        Result: "it worked!" functionName=aFunction, foo=bar, bar=baz, user=qWHLVEsbEe 
    
    • logging highly variable metadata is pretty useless for indexing when logs are sent to a logging repository like elastic search. In that use case, you want to index stuff you expect to filter on like user, hook type.
    • finally, putting the same input and result data in both the metadata and the message makes each message much larger with no additional value (that I know of anyway :).
  4. Change some of the naming of functions in trigger.js to make future work easier. I was confused about why there were three logging functions in trigger and it took me awhile to get that before hooks and after hooks are logged differently. I just changed the names to make it obvious at first glance.

  5. Add some try/catches to help any future futzers see syntax errors, etc instead of just hanging.

Some log examples from unit test output:

info: Ran cloud function loggerTest for user YUD2os1i5B with:
  Input: {}
  Result: {} functionName=loggerTest, user=YUD2os1i5B

info: beforeSave triggered for MyObject for user nssehQ3wtz:
  Input: {}
  Result: {} className=MyObject, triggerType=beforeSave, user=nssehQ3wtz

info: afterSave triggered for MyObject for user XdznQgTD0p:
  Input: {"createdAt":"2016-08-19T01:11:31.249Z","updatedAt":"2016-08-19T01:11:31.249Z","objectId":"POoOOLL89U"} className=MyObject, triggerType=afterSave, user=XdznQgTD0p

error: beforeSave failed for MyObject for user 7JHqCZgnhf:
  Input: {}
  Error: {"code":141,"message":"uh oh!"} className=MyObject, triggerType=beforeSave, code=141, message=uh oh!, user=7JHqCZgnhf

info: Ran cloud function aFunction for user YR3nOoT3r9 with:
  Input: {"foo":"bar"}
  Result: "it worked!" functionName=aFunction, user=YR3nOoT3r9

error: Failed running cloud function aFunction for user Xm6NpOyuMC with:
  Input: {"foo":"bar"}
  Error: {"code":141,"message":"it failed!"} functionName=aFunction, code=141, message=it failed!, user=Xm6NpOyuMC

info: Ran cloud function aFunction for user CK1lvkmaLg with:
  Input: {"longString":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus lobortis semper diam, ac euismod diam pharetra sed. Etiam eget efficitur neque. Proin nec diam mi. Sed ut purus dolor. Nulla nulla nibh, ornare vitae ornare et, scelerisque rutrum eros. Mauris venenatis tincidunt turpis a mollis. Donec gravida eget enim in luctus.\n\nSed porttitor commodo orci, ut pretium eros convallis eget. Curabitur pretium velit in odio dictum luctus. Vivamus ac tristique arcu, a semper tellus. Morbi euismod purus dapibus vestibulum sagittis. Nunc dapibus vehicula leo at scelerisque. Donec porta mauris quis nulla imperdiet consectetur. Curabitur sagittis eleifend arcu eget elementum. Aenean interdum tincidunt ornare. Pellentesque sit amet interdum tortor. Pellentesque blandit nisl eget euismod consequat. Etiam feugiat felis sit amet porta pulvinar. Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nNulla faucibus sem ipsum, at rhoncus diam pulvinar at. Vivamus consectetur, diam... (truncated)
  Result: {"longString":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus lobortis semper diam, ac euismod diam pharetra sed. Etiam eget efficitur neque. Proin nec diam mi. Sed ut purus dolor. Nulla nulla nibh, ornare vitae ornare et, scelerisque rutrum eros. Mauris venenatis tincidunt turpis a mollis. Donec gravida eget enim in luctus.\n\nSed porttitor commodo orci, ut pretium eros convallis eget. Curabitur pretium velit in odio dictum luctus. Vivamus ac tristique arcu, a semper tellus. Morbi euismod purus dapibus vestibulum sagittis. Nunc dapibus vehicula leo at scelerisque. Donec porta mauris quis nulla imperdiet consectetur. Curabitur sagittis eleifend arcu eget elementum. Aenean interdum tincidunt ornare. Pellentesque sit amet interdum tortor. Pellentesque blandit nisl eget euismod consequat. Etiam feugiat felis sit amet porta pulvinar. Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nNulla faucibus sem ipsum, at rhoncus diam pulvinar at. Vivamus consectetur, diam... (truncated) functionName=aFunction, user=CK1lvkmaLg

…arse-community#2501)

1. More closely mimic the wording.  Include the user id.

2. Truncate input and result at 1k char.

3. Use more sensible metadata that would makes sense to index.  The guideline I used was: if it makes sense to filter on, put it in metadata.  If it makes sense to "free text" search on, then put it in the message.

  - file and console output, logging an object does not do what on might expect.  For example, logging a function's "params":

  ```
  expected:

    info: Ran cloud function aFunction for user qWHLVEsbEe with:
      Input: {"foo":"bar","bar":"baz"}
      Result: "it worked!" functionName=aFunction, params= { foo: "bar", "bar": baz }, user=qWHLVEsbEe

  what you actually get:

    info: Ran cloud function aFunction for user qWHLVEsbEe with:
      Input: {"foo":"bar","bar":"baz"}
      Result: "it worked!" functionName=aFunction, foo=bar, bar=baz, user=qWHLVEsbEe
  ```
  - logging highly variable metadata is pretty useless for indexing when logs are sent to a logging repository like elastic search.  In that use case, you want to index stuff you expect to filter on like user, hook type.

  - finally, putting the same input and result data in both the metadata and the message makes each message much larger with no additional value (that I know of anyway :).

4. Change some of the naming of functions in trigger.js to make future work easier.  I was confused about why there were three logging functions in trigger and it took me awhile to get that before hooks and after hooks are logged differently.  I just changed the names to make it obvious at first glance.

5. Add some try/catches to help any future futzers  see syntax errors, etc instead of just hanging.

Some log examples from unit test output:

```
info: Ran cloud function loggerTest for user YUD2os1i5B with:
  Input: {}
  Result: {} functionName=loggerTest, user=YUD2os1i5B

info: beforeSave triggered for MyObject for user nssehQ3wtz:
  Input: {}
  Result: {} className=MyObject, triggerType=beforeSave, user=nssehQ3wtz

info: afterSave triggered for MyObject for user XdznQgTD0p:
  Input: {"createdAt":"2016-08-19T01:11:31.249Z","updatedAt":"2016-08-19T01:11:31.249Z","objectId":"POoOOLL89U"} className=MyObject, triggerType=afterSave, user=XdznQgTD0p

error: beforeSave failed for MyObject for user 7JHqCZgnhf:
  Input: {}
  Error: {"code":141,"message":"uh oh!"} className=MyObject, triggerType=beforeSave, code=141, message=uh oh!, user=7JHqCZgnhf

info: Ran cloud function aFunction for user YR3nOoT3r9 with:
  Input: {"foo":"bar"}
  Result: "it worked!" functionName=aFunction, user=YR3nOoT3r9

error: Failed running cloud function aFunction for user Xm6NpOyuMC with:
  Input: {"foo":"bar"}
  Error: {"code":141,"message":"it failed!"} functionName=aFunction, code=141, message=it failed!, user=Xm6NpOyuMC

info: Ran cloud function aFunction for user CK1lvkmaLg with:
  Input: {"longString":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus lobortis semper diam, ac euismod diam pharetra sed. Etiam eget efficitur neque. Proin nec diam mi. Sed ut purus dolor. Nulla nulla nibh, ornare vitae ornare et, scelerisque rutrum eros. Mauris venenatis tincidunt turpis a mollis. Donec gravida eget enim in luctus.\n\nSed porttitor commodo orci, ut pretium eros convallis eget. Curabitur pretium velit in odio dictum luctus. Vivamus ac tristique arcu, a semper tellus. Morbi euismod purus dapibus vestibulum sagittis. Nunc dapibus vehicula leo at scelerisque. Donec porta mauris quis nulla imperdiet consectetur. Curabitur sagittis eleifend arcu eget elementum. Aenean interdum tincidunt ornare. Pellentesque sit amet interdum tortor. Pellentesque blandit nisl eget euismod consequat. Etiam feugiat felis sit amet porta pulvinar. Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nNulla faucibus sem ipsum, at rhoncus diam pulvinar at. Vivamus consectetur, diam... (truncated)
  Result: {"longString":"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus lobortis semper diam, ac euismod diam pharetra sed. Etiam eget efficitur neque. Proin nec diam mi. Sed ut purus dolor. Nulla nulla nibh, ornare vitae ornare et, scelerisque rutrum eros. Mauris venenatis tincidunt turpis a mollis. Donec gravida eget enim in luctus.\n\nSed porttitor commodo orci, ut pretium eros convallis eget. Curabitur pretium velit in odio dictum luctus. Vivamus ac tristique arcu, a semper tellus. Morbi euismod purus dapibus vestibulum sagittis. Nunc dapibus vehicula leo at scelerisque. Donec porta mauris quis nulla imperdiet consectetur. Curabitur sagittis eleifend arcu eget elementum. Aenean interdum tincidunt ornare. Pellentesque sit amet interdum tortor. Pellentesque blandit nisl eget euismod consequat. Etiam feugiat felis sit amet porta pulvinar. Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n\nNulla faucibus sem ipsum, at rhoncus diam pulvinar at. Vivamus consectetur, diam... (truncated) functionName=aFunction, user=CK1lvkmaLg
```
@@ -4,6 +4,8 @@ import AdaptableController from './AdaptableController';
import { LoggerAdapter } from '../Adapters/Logger/LoggerAdapter';

const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000;
const logStringTruncateLength = 1000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i know, i know, it should be adjustable. but this is the limit in parse.com and its a good start. If someone needs to adjust they can do a pr??

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope not adjustable :) I know we're not supposed to use upper case strings as var names but that's more readable. Can you make it LOG_STRING_TRUNCATE_LENGTH?

@codecov-io
Copy link

codecov-io commented Aug 19, 2016

Current coverage is 92.17% (diff: 97.72%)

Merging #2550 into master will decrease coverage by 0.04%

@@             master      #2550   diff @@
==========================================
  Files            97         97          
  Lines         11814      11833    +19   
  Methods        1454       1456     +2   
  Messages          0          0          
  Branches       1906       1908     +2   
==========================================
+ Hits          10895      10907    +12   
- Misses          919        926     +7   
  Partials          0          0          

Powered by Codecov. Last update 9ecb9a3...2e6f00b

const cleanResult = logger.truncateLogMessage(JSON.stringify(result.response.result));
logger.info(`Ran cloud function ${req.params.functionName} for user ${userString} `
+ `with:\n Input: ${cleanInput }\n Result: ${cleanResult }`, {
functionName: req.params.functionName,
Copy link
Contributor

Choose a reason for hiding this comment

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

as a user of the JSON logs, I'd like to keep the params there :)

- add back params to metadata and add back to the test
- use screaming snake case for conts
@ghost
Copy link

ghost commented Aug 19, 2016

@acinader updated the pull request - view changes

@flovilmart flovilmart mentioned this pull request Aug 19, 2016
@acinader acinader changed the title Make parse-server cloud code logging much to parse.com legacy. (fixes… Make parse-server cloud code logging closer parse.com legacy Aug 19, 2016
@@ -4,6 +4,8 @@ import AdaptableController from './AdaptableController';
import { LoggerAdapter } from '../Adapters/Logger/LoggerAdapter';

const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000;
const LOG_STRING_TRUCATE_LENGTH = 1000;
Copy link
Contributor

@steven-supersolid steven-supersolid Aug 19, 2016

Choose a reason for hiding this comment

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

Can we make this configurable somehow? It's been useful so far in dev not to have this limit
Minor: typo in TRUCATE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I say no? ;). Seems like getting back to the way logs were on parse is a good starting point? You wanna do a commit for it? Otherwise, I'll get to it, but would rather not hold this up for that, not a lot of work, I assume, but I'll have to learn how to do, document and unit test it.

Thanks for catching the typo! doh.

Copy link
Contributor

Choose a reason for hiding this comment

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

let's stick with that, move fast!

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine although now it's not possible to write a custom LoggerAdapter without truncation. We could add a clean() function to the adapter and call from the controller to enable this, instead of doing the cleaning in controller.

@ghost
Copy link

ghost commented Aug 19, 2016

@acinader updated the pull request - view changes

@flovilmart
Copy link
Contributor

That looks good to me, it's a work in progress, and is better than it was :)

@flovilmart flovilmart merged commit 5f67cae into parse-community:master Aug 19, 2016
caoer added a commit to caoer/parse-server that referenced this pull request Aug 29, 2016
* ParsePlatform/master: (100 commits)
  Only allow basic auth credentials with a known appId (parse-community#2574)
  vk.com provider registered (parse-community#2579)
  chore(package): update parse-server-push-adapter to version 1.1.0 (parse-community#2588)
  vk.com auth data manager implemented (parse-community#2578)
  Fix a typo (parse-community#2563)
  Makes sure routes don't overlap and yield a header set error (parse-community#2559)
  Postgres: $all, $and CLP and more (parse-community#2551)
  Changelog 2.2.18 (parse-community#2558)
  chore(package): update winston-daily-rotate-file to version 1.3.0 (parse-community#2547)
  chore(package): update parse-server-s3-adapter to version 1.0.5 (parse-community#2536)
  Adds bcrypt native binding for better login performance (parse-community#2549)
  chore(package): update mongodb to version 2.2.7 (parse-community#2554)
  Make parse-server cloud code logging closer parse.com legacy (parse-community#2550)
  chore(package): update pg-promise to version 5.3.1 (parse-community#2519)
  Postgres: Operations, Hooks, OAuth login, Files support (parse-community#2528)
  Syncing afterSave/afterDelete trigger calls (Issue parse-community#2489) (parse-community#2499)
  Updated README.md (parse-community#2538)
  Fix capitalization, typo, and grammar mistake (parse-community#2533)
  Update ISSUE_TEMPLATE.md
  fix typo (parse-community#2525)
  ...
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.

Make Logging More Useful
4 participants