opentelemetry-js
opentelemetry-js copied to clipboard
feat(sdk-logs): added colors option to ConsoleLogRecordExporter
Which problem is this PR solving?
I'm running this exporter in a worker thread (using a pino logger), and since in that context process.stdout is not a TTY, the logs are not colorized. I'd like the option to force the colorization.
Short description of the changes
Added an optional config for the ConsoleLogRecordExporter
that has a single option colors
which can override the automatic color support detection to turn colorization on or off.
Type of change
Please delete options that are not relevant.
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [x] This change requires a documentation update
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
- Added new unit tests that cover the change
- Installed into another project and validated manually
Checklist:
- [x] Followed the style guidelines of this project
- [x] Unit tests have been added
- [x] Documentation has been updated
👋 @open-telemetry/javascript-approvers
Codecov Report
Merging #4524 (885cc15) into main (f6a075b) will increase coverage by
0.00%
. The diff coverage is100.00%
.
Additional details and impacted files
@@ Coverage Diff @@
## main #4524 +/- ##
=======================================
Coverage 92.84% 92.85%
=======================================
Files 328 328
Lines 9494 9498 +4
Branches 2040 2042 +2
=======================================
+ Hits 8815 8819 +4
Misses 679 679
Files | Coverage Δ | |
---|---|---|
...es/sdk-logs/src/export/ConsoleLogRecordExporter.ts | 90.00% <100.00%> (+2.50%) |
:arrow_up: |
I only use the ConsoleLogRecordExporter
in NODE_ENV=development
I'm using pino for logging with pino-opentelemetry-transport. Since its running in a worker thread there is never colorization. Currently, I'm conditionally setting process.env.FORCE_COLORS="1"
when I create the exporter to provide this. What I'd like to do is pass my options as { colors: process.stdout.isTTY }
from the main thread.
@pichlermarc what do you think?
@10xLaCroixDrinker would it help to have colorized output be the default behavior (also for metric and trace console exporters)? :thinking:
I'm thinking something like: if process
is defined (may not be defined in the browser) and output is TTY, then automatically set the option. This way we would not add to the API surface of the ConsoleExporter :slightly_smiling_face:
@pichlermarc that is already the default behavior when process.stdout is a TTY. I'm running into this because the exporter is in a worker thread where it is not a TTY. When I say that I'd set the config to { colors: process.stdout.isTTY }
, that would just be when I'm setting the config in the main thread. When the exporter is created in the worker thread, it will just receive { colors: true }
Sorry for leaving you waiting for so long.
@pichlermarc that is already the default behavior when process.stdout is a TTY. I'm running into this because the exporter is in a worker thread where it is not a TTY. When I say that I'd set the config to
{ colors: process.stdout.isTTY }
, that would just be when I'm setting the config in the main thread. When the exporter is created in the worker thread, it will just receive{ colors: true }
I see.
I'd like to avoid having to increase the API surface for the console exporters. A reason why we have not added any features/config options to any of the other console exporters in the past was that the exporter interface is reasonably easy to implement on one's own. We therefore usually recommend users to implement a custom exporter for use-cases like this.
While I see that the feature can be useful we'd still want to apply the same principle for this exporter.