-
Notifications
You must be signed in to change notification settings - Fork 0
Fix browser logging issues #257
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
Fix browser logging issues #257
Conversation
|
||
[Home](./index.md) > [@eppo/js-client-sdk](./js-client-sdk.md) > [LogLevel](./js-client-sdk.loglevel.md) | ||
|
||
## LogLevel type |
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.
New docs auto-generated for the externally exposed method and type for setting the SDK's log level
{ | ||
"name": "@eppo/js-client-sdk", | ||
"version": "3.17.0", | ||
"version": "3.18.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.
Bumping minor version because of new public method and type
package.json
Outdated
}, | ||
"dependencies": { | ||
"@eppo/js-client-sdk-common": "4.15.1", | ||
"@eppo/js-client-sdk-common": "/tmp/packages/js-sdk-common", |
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.
This will update once js-sdk-common #296
is merged
}); | ||
td.verify( | ||
applicationLogger.error( | ||
td.matchers.contains({ err: td.matchers.anything() }), |
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.
Pino API (link) is to have the context object first.
yarn.lock
Outdated
version "4.15.1" | ||
resolved "https://registry.yarnpkg.com/@eppo/js-client-sdk-common/-/js-client-sdk-common-4.15.1.tgz#bf2d602b1462eb156224edd73f734a3c99267816" | ||
integrity sha512-ZVyczDCfAMAnuUlfE293b6h2wfDOcZnfxXHS52r0OU9GktdTYD9EuC3Arpzw4JeTgzV1vW6iTzoz5HDJHwZz6A== | ||
"@eppo/js-client-sdk-common@/tmp/packages/js-sdk-common": |
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.
Should update correctly once new version merged
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.
Don't forget to reference the correct @eppo/js-client-sdk-common version before merging!
} | ||
|
||
// @public | ||
export type LogLevel = 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'silent'; |
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're not supporting 'fatal'
in this list?
👯 Related PRs:
js-sdk-common #296
Eppo Only:
🎟️ Tickets:
Motivation and Context
This common repository is shared with the Node.js and Browser JavaScript SDKs. The latter won't have a global
process
, causing errors unless a dummy globalprocess
is defined.Also, currently, logging is hard-coded to be off (silent) for the Browser SDK. However, having logs can be helpful, especially for debugging.
Description
Use the latest common sdk with the updated application logger that now works with browsers.
Expose the
setLogLevel()
method for adjusting log levels.Also clean up existing logger statements to fit Pino API of having the context object first.
How has this been documented?
eppo-docs #698
- Documentation for setLogLevel()How has this been tested?
process
. Also I saw no logs by default, error if I provided a bad SDK key, and then all the info logs if I changed the log level on purpose.