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

NH-98561 Remove support of SW_APM_EXPORT_LOGS_ENABLED #518

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Feb 12, 2025

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.

@@ -173,7 +172,9 @@ def _configure_otel_components(
apm_config,
oboe_api,
)
self._configure_logs_exporter(apm_config)

# Always setup logs exporter
Copy link
Contributor Author

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.

Comment on lines +149 to +151
mock_config_logs_exp.assert_called_once_with(
mock_apmconfig_enabled_legacy
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines -308 to -309
if self._instrumentor_logs_enabled is False:
kwargs["logger_provider"] = NoOpLoggerProvider()
Copy link
Contributor Author

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.

@tammy-baylis-swi tammy-baylis-swi marked this pull request as ready for review February 13, 2025 00:36
@tammy-baylis-swi tammy-baylis-swi requested a review from a team as a code owner February 13, 2025 00:36
Copy link
Contributor

@cheempz cheempz left a 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 :)

@tammy-baylis-swi tammy-baylis-swi merged commit 334fd8e into NH-79205-otlp-by-default Feb 14, 2025
15 checks passed
@tammy-baylis-swi tammy-baylis-swi deleted the NH-98561-decommission-sw-logs-enabled branch February 14, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants