Autogenerate semantic convention constants#1759
Conversation
This reverts commit 68e6dc2.
…python into auto-semconv
|
This is cool. We will need another PR to replace all the constant usages correct? Maybe another issue is needed? |
Yes here is the PR that does it open-telemetry/opentelemetry-python-contrib#428. Another thing to point out here, there is manual list of resource related attributes here https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py#L74; Because removing them would be a breaking left as is and eventually it will be incomplete list. Going forward we can recommend people to use auto generated ones instead. |
codeboten
left a comment
There was a problem hiding this comment.
Nice, this is great. Any thoughts about putting this in a semconv module to make it easier to differentiate from the rest of the code?
Sounds good to me. Your suggestion is to move this from |
|
As discussed in the SIG meeting, let's publish a separate pypi package for all semantic conventions: |
Maybe we can make them derive from the autogenerated ones? |
|
Yes, that's what we decided on doing in the last SIG meeting. |
codeboten
left a comment
There was a problem hiding this comment.
Thanks for updating this PR, overall it looks good. Blocking on the version number as the spec for this is still not stable.
opentelemetry-semantic-conventions/src/opentelemetry/semconv/version.py
Outdated
Show resolved
Hide resolved
opentelemetry-sdk/setup.cfg
Outdated
| include_package_data = True | ||
| install_requires = | ||
| opentelemetry-api == 1.0.1.dev0 | ||
| opentelemetry-semantic-conventions == 1.0.1.dev0 |
There was a problem hiding this comment.
i wouldnt expect this package to be marked as stable as the spec is still experimental
There was a problem hiding this comment.
Then does it still make sense to use this unstable package in sdk to derive the resource attributes?
There was a problem hiding this comment.
I think that's fine, it just means that the SDK will need to be updated as the conventions package is updated. @lzchen any thoughts?
codeboten
left a comment
There was a problem hiding this comment.
Thanks for addressing my comments!
|
@lzchen Please review again. There have been some major changes since you last reviewed. |
Description
Fixes #1022
Type of change
Please delete options that are not relevant.
Does This PR Require a Contrib Repo Change?
Checklist: