-
Notifications
You must be signed in to change notification settings - Fork 1
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
NH-98561 Remove support of SW_APM_EXPORT_LOGS_ENABLED #518
NH-98561 Remove support of SW_APM_EXPORT_LOGS_ENABLED #518
Conversation
@@ -173,7 +172,9 @@ def _configure_otel_components( | |||
apm_config, | |||
oboe_api, | |||
) | |||
self._configure_logs_exporter(apm_config) | |||
|
|||
# Always setup logs exporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will always be called for now. Currently _configure_logs_exporter
includes a check of OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED
and if False does not set up log handling nor exporter. This will change in another upcoming PR.
mock_config_logs_exp.assert_called_once_with( | ||
mock_apmconfig_enabled_legacy | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment.
if self._instrumentor_logs_enabled is False: | ||
kwargs["logger_provider"] = NoOpLoggerProvider() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No upstream instrumentor supports this kwarg at the moment, so this does nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, appreciate the helpful comments :)
Note: this will merge into feature branch
NH-79205-otlp-by-default
.This removes support of
SW_APM_EXPORT_LOGS_ENABLED
/exportLogsEnabled
. The value is no longer stored internally. If configured by user, a warning is logged.This is a little aggressive but will be part of a major release that breaks larger default signal export behaviours.