opentelemetry-python
opentelemetry-python copied to clipboard
New semconvgen prototype
First stab at https://github.com/open-telemetry/semantic-conventions/issues/551
Remaining discussions:
- [ ] How to generate metrics. See https://github.com/open-telemetry/opentelemetry-python/pull/3586#discussion_r1509494281 for options
- [ ] How to separate stable semconv from experimental. See https://github.com/open-telemetry/opentelemetry-python/pull/3586#issuecomment-1973978521 for options
Please delete options that are not relevant.
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [x] This change requires a documentation update
How Has This Been Tested?
Does This PR Require a Contrib Repo Change?
- [x] No.
Checklist:
- [ ] Followed the style guidelines of this project
- [ ] Changelogs have been updated
- [ ] Unit tests have been added
- [ ] Documentation has been updated
@open-telemetry/python-maintainers could you please take another look and tell if there you see any showstopping issues we need to resolve before moving forward with build-tools changes?
@lmolkova
Approach looks good to me for the new build-tools.
@ocelotl @aabmass
Could you PTAL and see if you are okay with these changes?
@lmolkova it looks like we decided to go ahead with the version namespace approach, is that right? I have no issue with it but wanted to understand what happens at the next semconv version bump, does v1_23_1 directory stay there or deleted?
Discussed offline with @lzchen wrt folder structure.
There was an idea from .NET SIG to generate namespace per-version. I.e. users import v1_23_1.http_attributes and will be able to use attributes from this namespace even when semconv library is updated and now contains v1_24_0, etc.
While it's good for stability, it's blows up artifact size.
As a middle ground, I changed folder structure to
opentelemetry-semantic-conventionssrc\opentelemetry\semconvexperimental(attributes, can also move them all toattributesfolder)db_attributes.py- individual experimental attribute files go here
http_attributes.py, etc - individual stable attribute files go heremetricshttp_metrics.py- stable metrics go hereexperimentaldb_metrics.py- experimental metrics go here
events(nothing here yet, but eventually will be)- stable events
experimental- experimental events
The folder structure is fully in Python SIG hands and does not need any build-tools work to be customized.
Documenting options to split stable/experimental
Option 1: ship 1 artifact containing stable and experimental semconv. Emphasize that some parts are experimental via namespace (import semconv.experimental.foo_attributes, perhaps we should call it _experimental )?
- Pros: easy to maintain
- Cons:
- violates semantic versioning
- when attribute stabilizes, it needs to be moved, i.e. libs/apps that depend on it need to change .See https://github.com/open-telemetry/semantic-conventions-java/pull/45#issuecomment-1973934974
Option 2: ship 1 artifact, but two different versions: stable and preview. Stable contains only stable semconv. Preview contains all of them.
- Pros:
- ideal in terms of semantic versioning
- when attribute stabilizes, it remains in the same namespace and all libs/apps that depend on it don't need to change
- Cons:
- version resolution would pick the highest version (1.1.0 stable vs 1.0.0-alpha) resulting in runtime issues.
Option 3: ship 2 artifacts (semconv and semconv-experimental) - Java does it this way and uses incubating term instead of experimental
- Pros:
- good in terms of semantic versioning
- easy to maintain
- Cons:
- when attribute stabilizes, all libs/apps that depend on it need to change.
The common mitigation to "when attribute stabilizes, all libs/apps that depend on it need to change" could be to preserve stabilized attributes in experimental, incubating namespaces (might need better name for it)
Update: reached consensus with Java SIG
Ship 2 artifacts: semconv (1.0.0 stable) and semconv-incubating (1.0.0-alpha):
- stable one contains all the stable things
- experimental one contains everything
- when attribute is deprecated (we won't remove them from semconv anymore):
- it stays in the artifact (
semconvit was stable,incubatingif it was experimental) - it's deprecated in the code
- it stays in the artifact (
- when attribute stabilizes
- it appears in the
semconvartifact - it stays in the
incubatingartifacts as deprecated pointing to the new location in the stable artifact
- it appears in the
The documentation will recommend:
- lib owners to never depend on incubating artifact, hardcode experimental attributes instead
Hi @open-telemetry/python-maintainers and @open-telemetry/python-approvers, I addressed your feedback.
And here's the summary of remaining decisions I'd like to get your input on
- [ ] How to generate metrics. See https://github.com/open-telemetry/opentelemetry-python/pull/3586#discussion_r1509494281 for options
- [ ] How to separate stable semconv from experimental. See https://github.com/open-telemetry/opentelemetry-python/pull/3586#issuecomment-1973978521 for options - Based on it we can also finalize the folder structure. Current structure is described here https://github.com/open-telemetry/opentelemetry-python/pull/3586#issuecomment-1973958604
Note: none of this discussions are related to build-tools/jinja templates, so I think it means we can move forward with build-tools changes independently from the results of these discussions. LMK if you have any objections.
Based on the SIG discussion 3/14, we have the following preliminary consensus on artifacts+stability outlined in https://github.com/open-telemetry/opentelemetry-python/pull/3586#issuecomment-1973978521 as Option 3:
- have 2 artifacts (
semantic-conventionsandsemantic-conventions-experimental|incubating). Different names and import path makes incubation/experimental status obvious - Attributes (experimental or stable) are never removed from the incubation artifact
- Stabilized attributes are deprecated in the incubating artifact an point to stable one
This makes Python consistent with Java approach.
Overall, we have proved that new codegeneration changes provide the flexibility to achieve it and generate idiomatic python code, so we can go ahead with them.
@xrmx
Is there a reason comments are added after and not before the variables?
it seems to be the pattern, e.g. here
https://github.com/open-telemetry/opentelemetry-python/blob/d285b7f6f51b5b2f28debc072d997726e67f3574/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py#L15-L21
Happy to change it otherwise.
@xrmx
Is there a reason comments are added after and not before the variables?
it seems to be the pattern, e.g. here
https://github.com/open-telemetry/opentelemetry-python/blob/d285b7f6f51b5b2f28debc072d997726e67f3574/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py#L15-L21
Happy to change it otherwise.
Don't mind, was just curious. I can see it making sense in sphinx documents but feels strange on python code.