-
Notifications
You must be signed in to change notification settings - Fork 650
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
ReadTheDocs and pkg_resources fix #1145
Conversation
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 approach seems fine to me. Thanks for the fix!
@@ -95,9 +95,6 @@ | |||
TELEMETRY_SDK_NAME = "telemetry.sdk.name" | |||
TELEMETRY_SDK_VERSION = "telemetry.sdk.version" | |||
|
|||
OPENTELEMETRY_SDK_VERSION = pkg_resources.get_distribution( |
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.
Would opencensus-exporter have the same issue?
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.
I believe it would if the autodoc directive included that util.py
file, but it just includes it doesn't: https://github.com/open-telemetry/opentelemetry-python/blob/master/docs/exporter/opencensus/opencensus.rst
So all you see in the docs here is the docstring here:
Lines 15 to 18 in b8a8016
""" | |
The **OpenCensus Exporter** allows to export traces and metrics using | |
OpenCensus. | |
""" |
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.
I could fix that one up to do this as well though in case it's added in the future. Another option is to update the docs-requirements.txt
file to actually install the opentelemetry-sdk/opentelemetry-api in the virtualenv so that pkg_resources.get_distrubution()
works correctly.
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.
Gonna go with this second approach instead 👍
b95def6
to
83c5d15
Compare
* chore: creating new metric kind * chore: adding todo to remove observer kind later
Description
ReadTheDocs builds do not currently install the OTel packages in a virtualenv, but just update
sys.path
to include the modules. Autodoc imports the modules, which runs this code, but since the package isn't installed,pkg_resources.get_distribution()
fails.This change has
opentelemetry-api
andopentelemetry-sdk
installed in the virtualenv to fix this.Fixes #1144
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Ran the commands from https://readthedocs.org/projects/opentelemetry-python/builds/11937984/ and the issue is resolved.
Checklist: