Deprecate error.message in favor of domain-specific attributes#3308
Deprecate error.message in favor of domain-specific attributes#3308lmolkova wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
ab03348 to
c09cfc1
Compare
|
This PR contains changes to area(s) that do not have an active SIG/project and will be auto-closed:
Such changes may be rejected or put on hold until a new SIG/project is established. Please refer to the Semantic Convention Areas |
askpt
left a comment
There was a problem hiding this comment.
Added a couple of suggestions and a minor question. Thanks!
|
After reviewing the linked issue, it's still not clear to me why |
I think the tooling here is a bit aggressive. It's expected that sigs wind down after doing their work. There isn't any reason for the feature flag sig to continue to meet when regular changes are not expected.
I agree with @beeme1mr it seems like this is a documentation issue. I found an attempt to document this at #2296 which was closed as stale, but I don't see any unresolvable issues with it. To me the biggest issue with using a generic attribute is what happens if multiple semconv areas need to use it on the same span. So far this has been avoided with |
|
@dyladan @beeme1mr thanks for the feedback. We've chatted about it in the SemConv call, documenting additional context here: The
So the practical benefit of having one attribute across conventions is low, while the ambiguity it introduces is high. This is the only attribute that is in development state in feature flags and, at least in my head, was the main blocker preventing FF from going stable. While I understand it introduces churn, it also opens up a path forward for FF to stabilize at any time. |
What about in the case of client errors, where span status is left unset, but where there is an error message (the client error)? |
|
One other concern about domain-specific error attributes: visualization. Won't this require visualization UIs to now look for "the error message" in more places? Wouldn't it be better if UIs could easily find a single attribute for "the error message"? |
@michaelsafyan do you have a suggestion of the specific user experience that would need it? We've been trying to come up with generic logs dashboard and came up with something like
So what would be the case when UX needs to visualize something around error message and would it make sense across all conventions? |
it sounds like it wasn't an error if status was unset and it'd be a log record with appropriate severity. |
|
In the case of a Such an operation could have multiple logs correlated with the span of an appropriate severity, but there is no way to know if the log represents the message of the overall operation vs just a warning that happened within it. One could imagine a view of the span which elevates error information to bring it front and center. While perhaps there are cases where domain-specific errors are appropriate (errors that are "fail open" and do not fail the operation), deprecating |
@michaelsafyan I see your point and I think this scenario makes sense. Note that |
|
Would it be possible to simply expand the meaning/scope of Note that OTel SemConv is not comprehensive or complete. There are uses of OTel SemConv in vendor-specific conventions that may not necessarily be documented in the OTel SemConv repo. That is, it is incorrect to conclude that lack of references within OTel SemConv necessarily means that the attribute is unused for the given meaning/purpose. |
As you can see, we tried to define this attribute and failed. Please create an issue and it'd also be a good change to rename this attribute to match the use-case. We don't recommend external semconv registries to take dependency on experimental parts of OTel semconv. |
|
@open-telemetry/semconv-feature-flag-approvers Could you please take another look? |
Fixes #3307