Prepare stable release #130
Conversation
| +++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. | ||
| +++ NEW SUPERCLASS: java.lang.Object | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) io.opentelemetry.api.common.AttributeKey<java.lang.String> EXCEPTION_TYPE | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) io.opentelemetry.api.common.AttributeKey<java.lang.Boolean> EXCEPTION_ESCAPED |
There was a problem hiding this comment.
should we proactively remove this unused (and likely to be deprecated) constant? open-telemetry/semantic-conventions#1516
cc @lmolkova
There was a problem hiding this comment.
should we proactively remove this unused (and likely to be deprecated) constant?
Would you imagine doing this in semantic-conventions or in the code generation tooling or in the code generation configuration local to semantic-convention-java?
There was a problem hiding this comment.
I think we would need to hack it somehow to remove it in this repo, and wait to see what happens to the attribute in semconv
not sure it's worth it, there will be other deprecated "stable" attributes in the future anyways...
There was a problem hiding this comment.
It should be really easy to exclude (a few) specific attributes by extending this list -
It would felt great if we already deprecated it in semconv.
There was a problem hiding this comment.
thanks, I'll send a separate PR to try it out
There was a problem hiding this comment.
Just so we're on the same page, the principle behind removing this attribute is that we want to start with a clean slate and not stabilize anything deprecated unnecessarily. By this logic, this is the only instance where we would do this, since any attribute which is deprecated in the future will have to be maintained for backwards compatibility.
Is this your understanding?
There was a problem hiding this comment.
Makes sense to me. If for some strange reason we need this attribute, we can revert the exclusion to restore it.
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_24_0 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_23_1 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_22_0 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_29_0 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_28_0 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_27_0 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_26_0 | ||
| +++ NEW FIELD: PUBLIC(+) STATIC(+) FINAL(+) java.lang.String V1_25_0 |
There was a problem hiding this comment.
any thoughts on this list (i.e. how far to go back)?
cc @lmolkova
There was a problem hiding this comment.
Notable versions:
- 1.21.0 was first release from
semantic-conventionsrepo - 1.23.0 stable http conventions
- 1.24.0 stable JVM conventions, significant restructuring of
semantic-convention-javato switch to{Domain}Attributesnaming convention and breakoutopentelemetry-semconvandopentelemetry-semconv-incubatingartifacts. - 1.28.0 switch code gen for
semantic-convention-javato weaver
We could start conservative with the schema URLs we include and go further back later if needed?
There was a problem hiding this comment.
yeah, I don't really have any specific concerns or objection with the existing list
1.21.0 was first release from
semantic-conventionsrepo
this seems as good as any reason, but agree we can add later, only thing that would affect this PR / release would be if we wanted to restrict the list
1.23.0 stable http conventions
btw, I noticed 1.23.0 is missing in the list (but 1.22.0 and 1.23.1 are included)
There was a problem hiding this comment.
btw, I noticed 1.23.0 is missing in the list (but 1.22.0 and 1.23.1 are included)
I think 1.23.0 was a botched semantic convention release so we skipped right to 1.23.1.
There was a problem hiding this comment.
I did a quick search for "SchemaUrls.V1_" NOT is:fork across GitHub and it seems there are just a few places that use it.
I only see usages of V1_24_0 and V1_25_0 - which makes sense - we ask to stay at 1.24.0 in DB and messaging semconv.
I'd remove anything below 1.24.0 - it does not seem to be used and is really old.
There was a problem hiding this comment.
Quick thought - would we regret having exact patch in the version?
Should we do V1_25 (V1_25_X) instead similarly to release branches?
I.e. if semconv is patched, we'd update patch in the value, but would not add a new version.
Happy to send a PR if you folks think it makes sense.
There was a problem hiding this comment.
I kind of agree, but if we're saying there's no difference between patched versions, shouldn't the schema url itself be https://opentelemetry.io/schemas/1.29 or https://opentelemetry.io/schemas/1.29.x?
There was a problem hiding this comment.
That's a can of worms though because these are resolvable URLs, and ../1.29 and ../1.29.x do not resolve. I think our schema URLs should reflect whatever the publishing convention is on opentelemetry.io. We could open an issue about this and block until resolved, but I don't think we should block stability. If opentelemetry.io later updates to reflect one of the conventions you propose, we can update our schema URL values from that point onward.
There was a problem hiding this comment.
ok, I created open-telemetry/opentelemetry-specification#4355 - we can change the format of the constant later if it comes through.
Resolves #90.
opentelemetry-semconvartifact