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-100646 Default OTLP auth header always in lambda #494

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Jan 29, 2025

Simplifies APM Python custom distro by removing one unnecessary is_lambda check.

Change is that now, in lambda environment, default values for OTEL_EXPORTER_OTLP_(TRACES|METRICS|LOGS)_HEADERS always get set to f"authorization=Bearer%20{header_token}" even if header_token is empty, which should normally be the case. This does not affect export to otel-collector layer on localhost -- see ticket comment for more test info. This is again for default values, so user can always override with their own HEADERS set in environment.

Works the same as before outside of lambda.

Also adds more unit tests and clarifies existing ones.

@@ -52,6 +52,9 @@ def before_and_after_each(self):
if old_otel_ev_lh:
del os.environ["OTEL_EXPORTER_OTLP_LOGS_HEADERS"]
old_key = os.environ.get("SW_APM_SERVICE_KEY", None)
old_otel_ev_proto = os.environ.get("OTEL_EXPORTER_OTLP_PROTOCOL", None)
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 os.environ stash and reset was missing and causing inconsistencies, noticed only after updating tests in this PR.

@@ -347,6 +352,18 @@ def test__configure_traces_export_env_defaults_valid_protocol_grpc(self, mocker)
assert os.environ.get(OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) == "https://otel.collector.na-01.cloud.solarwinds.com:443/v1/traces"
assert os.environ.get(OTEL_EXPORTER_OTLP_TRACES_HEADERS) == "authorization=Bearer%20foo-token"

def test_configure_set_otlp_header_defaults_not_lambda_no_header_token(self, mocker):
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 test coverage was missing before

@tammy-baylis-swi tammy-baylis-swi changed the title NH-100646 Default OTLP auth header always NH-100646 Default OTLP auth header always in lambda Jan 29, 2025
@tammy-baylis-swi tammy-baylis-swi marked this pull request as ready for review January 29, 2025 18:21
@tammy-baylis-swi tammy-baylis-swi requested a review from a team as a code owner January 29, 2025 18:21
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, thanks for the writeup and tests @tammy-baylis-swi!

@tammy-baylis-swi tammy-baylis-swi merged commit cd6293a into main Jan 29, 2025
17 checks passed
@tammy-baylis-swi tammy-baylis-swi deleted the NH-100646-otlp-header branch January 29, 2025 23:22
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