feat(api-logs, sdk-logs)!: add Logger#enabled method#6371
feat(api-logs, sdk-logs)!: add Logger#enabled method#6371david-luna wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6371 +/- ##
=======================================
Coverage 95.48% 95.48%
=======================================
Files 363 363
Lines 11563 11569 +6
Branches 2669 2669
=======================================
+ Hits 11041 11047 +6
Misses 522 522
🚀 New features to boost your workflow:
|
| /** | ||
| * Tells if the logger is enabled for the given context, severity number and event | ||
| * name if provided. The context details to the active one | ||
| * TODO: check in the spec what means implicit/explicit context support |
There was a problem hiding this comment.
note for reviewer: this TODO is also to discuss about the params of this method. The current implementations of Logger (NopLogger and Logger form the logs SDK) do not seem to need the params but I think with these params the spec allows to have loggers that my filter for a specific event, log level or context
| severityNumber?: SeverityNumber; | ||
| eventName?: string; | ||
| }): boolean { | ||
| return true; |
There was a problem hiding this comment.
note: we do actually have this property _loggerConfig (experimental feature) that allows users to disable certain loggers based on severity number, name of the logger and trace sampling decision - I think we'd need to read this here.
Which problem is this PR solving?
Closes #6351
Added as a breaking change since anyone implementing this interface now must to implement the new method.
Short description of the changes
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: