-
Notifications
You must be signed in to change notification settings - Fork 626
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
Move opentelemetry-instrumentation from core #465
Conversation
Programming Language :: Python :: 3.6 | ||
Programming Language :: Python :: 3.7 | ||
Programming Language :: Python :: 3.8 | ||
Programming Language :: Python :: 3.9 |
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.
py39 is not part of ci. would you mind adding it to env list https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/.github/workflows/test.yml#L13?
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.
Added this in a separate PR #466
help=""" | ||
The service name that should be passed to a trace exporter. | ||
""", | ||
) |
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.
Looks like this --service-name
doesn't do anything right now, Is this deprecated or not fully developed? Either way this can be addressed in another PR but we should decide on removing or implementing the missing part.
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.
@owais you might know better about this.
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.
Looks like this was removed here: open-telemetry/opentelemetry-python@c61a712#diff-8e926c8a585e70ab1e5df5d63ed7709fb982976f01bbdee56e238ad74153ef57L86
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.
Oh, it was useful. We should probably add it back.
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.
@owais
Probably won't be adding it back due to that PR? ^
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.
Thanks for moving this
help=""" | ||
The service name that should be passed to a trace exporter. | ||
""", | ||
) |
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.
Looks like this was removed here: open-telemetry/opentelemetry-python@c61a712#diff-8e926c8a585e70ab1e5df5d63ed7709fb982976f01bbdee56e238ad74153ef57L86
Part of open-telemetry/opentelemetry-python#1797