Initial Commit of Logging SDK#5
Conversation
Including the loggin API initial PR
Codecov Report
@@ Coverage Diff @@
## master #5 +/- ##
==========================================
- Coverage 94.98% 94.76% -0.23%
==========================================
Files 162 169 +7
Lines 6996 7082 +86
==========================================
+ Hits 6645 6711 +66
- Misses 351 371 +20
|
ranton256
left a comment
ranton256
left a comment
There was a problem hiding this comment.
I'm not sure what is signified by the sdk/logs/TBD and sdk/src/logs/TBD empty files.
| */ | ||
| std::shared_ptr<LogProcessor> GetProcessor() noexcept; | ||
|
|
||
| // Sets the common processor that all the Logger instances will use |
There was a problem hiding this comment.
Why the // comment outside the /** comment? Are you excluding this intentionally?
| */ | ||
| opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger> GetLogger( | ||
| opentelemetry::nostd::string_view name, | ||
| opentelemetry::nostd::string_view options = "") noexcept override; |
There was a problem hiding this comment.
Are there docs in API or spec or elsewhere for what options argument should support?
| opentelemetry::sdk::AtomicSharedPtr<LogProcessor> processor_; | ||
|
|
||
| // A vector of pointers to all the loggers that have been created | ||
| std::vector<std::shared_ptr<Logger>> loggers_; |
There was a problem hiding this comment.
Storing the loggers in a vector will require linear search in O(n) time anytime someone asks for a logger by name. I am not sure if this is okay. It depends on the expected worst case for how many loggers anyone is going to have.
| opentelemetry::nostd::string_view name, | ||
| opentelemetry::nostd::string_view options) noexcept | ||
| { | ||
| // Search if a logger with the given name already exists. |
There was a problem hiding this comment.
since lookup is probably a more frequent use case than scanning all loggers, and uniqueness by name is important, you should probably use std::unordered_set or maybe map instead of vector for storing the loggers. It's possible if you only ever have very few elements in the vector(like 3 or 4) that. you might win that way, but the performance will be bad for someone with lots. It would also save you having to manage the uniqueness constraint yourself.
…ogger limit in LoggerProvider
4 participants
This PR contains an initial implementation of the Logging SDK, which follows from the initial API commit and the logging prototype issue.
This PR adds:
The design doc for the Logging SDK will be released in a future PR.
cc @xukaren @alolita