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

Update patch of upstream OTel Python otel-instrument script #158

Conversation

NathanielRN
Copy link
Contributor

Description:

With the upstream open-telemetry/opentelemetry-lambda#164 merged, we want to update our patched version to be very similar.

It only has the difference that it adds the xray propagator.

Link to tracking Issue:

N/A

Testing:

N/A

Documentation:

N/A

@NathanielRN NathanielRN force-pushed the update-python-otel-instrument-patch branch from 70ac179 to dcb0613 Compare October 28, 2021 19:15
Comment on lines +125 to +129

if [ -z ${OTEL_PROPAGATORS} ]; then
export OTEL_PROPAGATORS="tracecontext,baggage,xray";
fi
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 line is the only line that is different with the otel-instrument script on opentelemetry-lambda upstream

Upstream we say:

# - Uses the default `OTEL_PROPAGATORS` which is set to `tracecontext,baggage`

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just have this line and then call the upstream otel-instrument script?

It's not the cleanest way, but removes the need to maintain duplicate scripts so we use that approach in the other exec wrappers

https://github.com/aws-observability/aws-otel-lambda/blob/main/java/build.sh#L27
https://github.com/aws-observability/aws-otel-lambda/blob/main/java/scripts/otel-handler#L13

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this! I made #163 to try and implement what you suggested! 🙂

@wangzlei wangzlei merged commit cd6d8ba into aws-observability:main Oct 28, 2021
@NathanielRN NathanielRN deleted the update-python-otel-instrument-patch branch October 29, 2021 00:36
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