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

Add logger_provider exporter and log_record_processors #20

Merged
merged 2 commits into from
Dec 6, 2022

Conversation

jack-berg
Copy link
Collaborator

Related to #5.

@jack-berg jack-berg mentioned this pull request Dec 5, 2022
config.yaml Outdated Show resolved Hide resolved
# Environment variable: OTEL_EXPORTER_OTLP_TIMEOUT, OTEL_EXPORTER_OTLP_LOGS_TIMEOUT
timeout: 10000
# List of log record processors. Each log record processor has a name and args used to configure it.
log_record_processors:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the "appender"/handler is language-specific, but I think it is essential to the logging SDK configuration. Should this minimal config define some mechanism for it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.

The wiring up of appenders to the SDK seems more like an instrumentation concern than an SDK concern. I suggest we include that under the instrumentation section. I.e. something like:

sdk:
  ...
instrumentation:
  log_appenders:
    - name: log4j
    - name: logback

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a separate issue for configuring handlers, feel free to resolve that issue in this PR or separately.

Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
Copy link
Collaborator

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, there's only the question of the appenders/handlers. This can be resolved separately.

@jack-berg jack-berg merged commit a84c90b into main Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants