Logger: Support for Instrumentation library #1128
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1128 +/- ##
=======================================
Coverage 93.39% 93.39%
=======================================
Files 165 165
Lines 6229 6229
=======================================
Hits 5817 5817
Misses 412 412 |
esigo
changed the title
|
|
||
| virtual nostd::shared_ptr<Logger> GetLogger(nostd::string_view logger_name, | ||
| nostd::span<nostd::string_view> args) = 0; | ||
| virtual nostd::shared_ptr<Logger> GetLogger(nostd::string_view library_name, |
There was a problem hiding this comment.
Sorry for not clarifying this in the issue, but I think logger_name and library_name are two separate entities and would be good to support both of them. While LoggerName is still not in the log data model ( in specs ) the PR is open - open-telemetry/opentelemetry-specification#1236 - and hopefully it would get added soon.
Also, do you think we can keep options and args even though they are not used as of now?
There was a problem hiding this comment.
So logger_name, library_name, options and args. library_name goes to the instrumentation , and options and args will stay unused correct?
| @@ -44,19 +46,13 @@ nostd::shared_ptr<logs_api::Logger> LoggerProvider::GetLogger(nostd::string_view | |||
|
|
|||
There was a problem hiding this comment.
The search logic also needs changes. Probably storing in the vector instead of map, and comparing for instrumentation_library ( as done in tracer ) along with the logger_name to get the right logger.
|
|
@lalitb Sorry, seems my Windows machine's git is still connected to my old email. I'd prefer to close this one and raise a new PR. |
Fixes #1087 (issue)
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes