add support for custom http request key, trace #54
Conversation
src/stackdriver.js
Outdated
| const defaultKeys = { | ||
| httpRequest: 'httpRequest', | ||
| trace: 'trace' | ||
| } |
There was a problem hiding this comment.
Can you make the props of defaultKeys optional like:
const defaultKeys = { httpRequest: 'httpRequest' }
and then when adding them to entry.meta... loop through keys to see if any exist like Object.keys
This way we can keep ik backwards compatible and not add anything extra if not specifically asked for.
There was a problem hiding this comment.
This should be resolved in 2352d3d , I've removed trace from the defaults
| trace: _getKey(log, data, 'trace', keys), | ||
| httpRequest: _getKey(log, data, 'httpRequest', keys) |
There was a problem hiding this comment.
add these dynamically by looking at specified keys
There was a problem hiding this comment.
This is currently just set to undefined if key is not present, which should achieve the same behaviour - I've improved this and added additional tests in 2352d3d as well
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
| // main cli logic | ||
| function main () { | ||
| program | ||
| .version(pkg.version) | ||
| .option('-c, --credentials <credentials>', 'The file path of the JSON file that contains your service account key') | ||
| .option('-p, --project <project>', 'Your Google Cloud Platform project ID') | ||
| .action(({ credentials, project }) => { | ||
| .option('-k, --key <key:customKey>', 'Customize additional data to include in log metadata', collect, []) |
There was a problem hiding this comment.
Could you add a test for the new option so we'll have 100% test coverage again?
There was a problem hiding this comment.
Shouldn't it be covered by this test? https://github.com/ovhemert/pino-stackdriver/pull/54/files#diff-da8fe3f2391da380ddef00afcc0f93f0R54
There was a problem hiding this comment.
Ah I see - I've added an additional error case in 776b6e2
This PR adds a
keysoption to provide custom keys programatically, and as part of the change allows users to settraceChecklist
npm run test