Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/debug] Add option to not use collector's internal logger for output #10226

Closed
andrzej-stencel opened this issue May 27, 2024 · 4 comments · Fixed by #10227
Closed
Assignees
Labels
exporter/debug Issues related to the Debug exporter

Comments

@andrzej-stencel
Copy link
Member

Is your feature request related to a problem? Please describe.

Currently (v0.101.0), the Debug exporter uses the collector's internal logger for output.
This comes with several consequences:

  1. The exporter's output is prefixed and suffixed by the internal logger's data.
    • The prefix is the current timestamp and the info level marker, similar to 2023-11-10T22:49:03.510-0600 info
    • The suffix is the exporter's metadata in JSON format, similar to {"kind": "exporter", "data_type": "traces", "name": "debug"}
  2. The exporter's output is sent to the same destination as the collector's logs (stderr by default). It is not possible to send the Debug exporter's output to a different stream than the collector's internal logs.
  3. The output from the Debug exporter is sent to the internal logger with the log level INFO. This means that if user configures the service.telemetry.logs.level to WARN or ERROR, they will not see the output from the Debug exporter. As a result, it is not possible to configure the collector to see the Debug exporter's output and mute collector's INFO logs at the same time.
  4. The exporter's output is subject to the internal logger's sampling, which is separate from the sampling defined in the exporter's configuration. See [exporter/debug] Debug exporter has sampling enabled by default #9921 and the note in the "Workaround" section of the issue.

Describe the solution you'd like

Add configuration option use_internal_logger that would make it possible to opt out of using the internal logger and instead use a logger configured separately from the collector's internal logger.

Describe alternatives you've considered

The only alternative I see is not use the Debug exporter and instead build a separate exporter for this, or use existing exporter like the File exporter.

@mx-psi
Copy link
Member

mx-psi commented May 29, 2024

Apart from adding this option, I think we should vote on whether we want to change the default behavior. On /issues/5652 I proposed this change, but since:

I still feel like the debug exporter should use the internal logger (but without sampling), but (i) I know there are other people who feel differently here, and (ii) I don't think either side has many arguments other than "this feels like a more intuitive UX". So I feel like we should vote on what we want the default behavior should be (internal logger vs stderr logger).

What do you think?

@andrzej-stencel
Copy link
Member Author

Preventing a breaking change is perhaps one strong argument to keep the current behavior (which uses internal logger). I'm saying this even though I prefer the other behavior. 🙃

@mx-psi
Copy link
Member

mx-psi commented May 29, 2024

I don't have data on this but I assume most users are not explicitly setting the output path for the Collector logger, so the blast radius of a change like this would likely be rather small

@andrzej-stencel
Copy link
Member Author

Proposed a pull request to expose the option use_internal_logger:

@andrzej-stencel andrzej-stencel self-assigned this May 31, 2024
mx-psi pushed a commit that referenced this issue Jun 27, 2024
#### Description

Adds a new configuration option `use_internal_logger` to the Debug
exporter, that makes it possible to prevent unwanted side effects of the
exporter using the collector's internal logger.

#### Link to tracking issue

Fixes
#10226

#### Testing

Updated unit tests to cover the new config option.

#### Documentation

Added documentation for the feature.

---------

Co-authored-by: Yang Song <songy23@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/debug Issues related to the Debug exporter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants