[exporter/debug] add ability to set output_paths #14164
Conversation
andrzej-stencel
changed the title
andrzej-stencel
left a comment
andrzej-stencel
left a comment
There was a problem hiding this comment.
Thanks for the contribution @ishaish103! I added a couple suggestions for the docs and the tests.
exporter/debugexporter/README.md
Outdated
| Refer to [Zap docs](https://godoc.org/go.uber.org/zap/zapcore#NewSampler) for more details | ||
| on how sampling parameters impact number of messages. | ||
| - `use_internal_logger` (default = `true`): uses the collector's internal logger for output. See [below](#using-the-collectors-internal-logger) for description. | ||
| - `output_paths` (default = `["stdout"]`): a list of URLs or file paths to write logging output to. This option is only used when `use_internal_logger` is `false`. The URLs could only be with "file" schema or without schema. The URLs with "file" schema must be an absolute path. The URLs without schema are treated as local file paths. "stdout" and "stderr" are interpreted as os.Stdout and os.Stderr. |
There was a problem hiding this comment.
Let's not mention "URLs", it may be confusing to users. I propose to officially support only bare file paths, regardless of what is internally supported by Zap.
There was a problem hiding this comment.
ok, ill also add validation for the paths as well to validate that its not one of the following:
file:///path/to/file (contains ://)
http://example.com (contains ://)
file:path/to/file (matches scheme: pattern)
| return cfg | ||
| }(), | ||
| }, | ||
| { |
There was a problem hiding this comment.
Please add a test case for when user sets output_paths but does not set use_internal_logger to false.
What should happen then? Shouldn't such config be invalid?
There was a problem hiding this comment.
basically the same thing:
throw an error if out_paths is null and set_intenral_logger is false
| return cfg | ||
| }(), | ||
| }, | ||
| { |
There was a problem hiding this comment.
Please add a test case for when user sets output_paths to an empty array.
What should happen then? Shouldn't such config be invalid?
There was a problem hiding this comment.
good catch, ill validate it and throw an error if out_paths is null and set_intenral_logger is false
Co-authored-by: Andrzej Stencel <[email protected]>
|
@andrzej-stencel Hey :) |
2 participants
Link to tracking issue
Fixes #10472