-
Notifications
You must be signed in to change notification settings - Fork 19
Add log-level option to runtime configuration #51 #54
Add log-level option to runtime configuration #51 #54
Conversation
throw new Error('config.logLevel must be a number or decimal ' + | ||
'representation of a number in string form'); | ||
} | ||
} else if (has(process.env, 'GCLOUD_ERRORS_LOGLEVEL')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the environment provided log level over the config provided log level for consistency with the other agents: https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/master/index.js#L66
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! ack
00fb99a
to
e8d8513
Compare
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
@@ -65,6 +65,7 @@ var errors = require('@google/cloud-errors').start({ | |||
projectId: 'my-project-id', | |||
key: 'my-api-key', | |||
reportUncaughtExceptions: false, // defaults to true. | |||
logLevel: 0, // defaults to logging warnings (2). Available levels: 0-5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this result in a default logging level of 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthewloring Yeah, I'm just showing you can provide a different value and reporting the default value in the comment like the preceding line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh! Thought this was the set of defaults and not the README.
LGTM. |
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
2 similar comments
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Changes Unknown when pulling e8d8513 on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Add the logLevel property to the runtime configuration and modify
the logger creation flow to parse this option second (if applicable)
after the environmental GCLOUD_ERRORS_LOGLEVEL level variable
and to use a singular instance of the logger class upon library
init. Add corresponding tests and update documentation accordingly.
Add a script to `package.json` for easy coverage without coveralls:
`npm run-script coverage`.
**Original config object:**
```JS
var errors = require('@google/cloud-errors').start({
projectId: 'my-project-id',
key: 'my-api-key',
reportUncaughtExceptions: false, // defaults to true.
serviceContext: {
service: 'my-service',
version: 'my-service-version'
}
});
```
**Updated configuration object:**
*Notice the `logLevel` property*
```JS
var errors = require('@google/cloud-errors').start({
projectId: 'my-project-id',
key: 'my-api-key',
reportUncaughtExceptions: false, // defaults to true.
logLevel: 0, // defaults to logging warnings
serviceContext: {
service: 'my-service',
version: 'my-service-version'
}
});
```
Related issues:
Fixes #51
e8d8513
to
39afdca
Compare
Changes Unknown when pulling 39afdca on cristiancavalli:issue/51/new-log-configuration-flow into * on GoogleCloudPlatform:master*. |
Add the
logLevel
property to the runtime configuration and modifythe logger creation flow to parse this option second (if applicable)
after the environmental
GCLOUD_ERRORS_LOGLEVEL
level variableand to use a singular instance of the logger class upon library
init. Add corresponding tests and update documentation accordingly.
Add a script to
package.json
for easy coverage without coveralls:npm run-script coverage
.Original config object:
Updated configuration object:
Notice the
logLevel
propertyRelated issues:
Fixes #51