feat(opentelemetry-configuration): parse logger provider from config file #5995
Conversation
…o parse-more-parameters
…o parse-more-parameters
…o parse-more-parameters
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5995 +/- ##
=======================================
Coverage 94.99% 94.99%
=======================================
Files 313 314 +1
Lines 8775 8837 +62
Branches 1884 1909 +25
=======================================
+ Hits 8336 8395 +59
- Misses 439 442 +3
🚀 New features to boost your workflow:
|
|
cc @JamieDanielson just merged with main, after the tracer parser PR was merged, so this PR is ready for review and should be straight forward now 😄 |
There was a problem hiding this comment.
Thanks @maryliag ! I've left some questions and notes for your review.
| element['exporter'], | ||
| ProviderType.LOGGER | ||
| ); | ||
| const simpleConfig: SpanProcessor = { |
There was a problem hiding this comment.
Should this be a log processor?
| }, | ||
| loggers: [ | ||
| { | ||
| name: 'io.opentelemetry.contrib.*', |
There was a problem hiding this comment.
I guess this doesn't matter but I think this is the prefix for java instrumentations not js right?
| const loggers = []; | ||
| for ( | ||
| let i = 0; | ||
| i < loggerProvider['logger_configurator/development'].loggers.length; | ||
| i++ |
There was a problem hiding this comment.
nit... could we replace with something like
const container = loggerProvider['logger_configurator/development'];
for (const logger of container.loggers) {
...
}| element['exporter'], | ||
| ProviderType.LOGGER | ||
| ); | ||
| const batchConfig: SpanProcessor = { |
There was a problem hiding this comment.
Should this be a log processor?
| function parseConfigExporter( | ||
| exporter: SpanExporter, | ||
| providerType: ProviderType | ||
| ): SpanExporter { |
There was a problem hiding this comment.
Since we are now using this for multiple signals, do we need to return different exporters for each signal (e.g. LogExporter vs SpanExporter)?
| } | ||
| const attrList = getStringFromConfigFile( | ||
| parsedContent['resource']?.['attributes_list'] | ||
| parsedContent['resource']['attributes_list'] |
There was a problem hiding this comment.
Reviewing utils, I think resource could still be undefined here right? Is there a reason to drop the optional chaining?
2 participants
Parse Logger Provider