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 sample apps to work with OTel Python v1.3 #12

Merged
merged 4 commits into from
Jun 22, 2021

Conversation

NathanielRN
Copy link
Contributor

@NathanielRN NathanielRN commented Jun 21, 2021

Description

OTel Python has changed the names of many things as part of their RC 1. In this PR we fix the imports and other bugs necessary to make the sample apps work with at least OTel Python 1.3.0

Noteworthy points

  1. Auto instrumentation MUST have flask~=1.0 because even though OTel Python instrumentation works with flask~=2.0 they haven't updated their package to reflect that. Filed this PR to fix that.
  2. For some reason, the Flask "app auto reloader" available in debug mode will remove instrumentation made with auto instrumentation! I haven't looked into deeply and solved it with just debug=False for now. Production would be fine but I'm worried customers would find this bug difficult to resolve. Will investigate a little more and file an issue upstream.

@@ -18,7 +18,7 @@
# Setup AWS X-Ray Propagator

# Propagators can be set using environment variable: OTEL_PROPAGATORS = aws_xray
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious why not use the environment variable?

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 particular reason! It's just that the 2 main components you need to get OTel Python to work with X-Ray is that:

  1. The IdsGenerator
  2. The Propagator

So I wanted those to be obvious from the source code here in the manual instrumentation instead of hiding them in environment variables in the script.

You can do the ID Generator from an environment variables too as of recent I think!

@NathanielRN NathanielRN merged commit 2a732b0 into aws-observability:main Jun 22, 2021
@NathanielRN NathanielRN deleted the update-apps-for-rc1 branch June 22, 2021 17:32
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.

2 participants