-
Notifications
You must be signed in to change notification settings - Fork 897
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
Align Embedded Logs data model with Standalone Logs data model #622
Comments
This is part of the alignment effort defined here open-telemetry/opentelemetry-specification#622 The comment added clarifies the semantics, but I believe this is compatible with our previous understanding and doe not change that understanding.
This is part of the alignment effort defined here open-telemetry/opentelemetry-specification#622 The comment added clarifies the semantics, but I believe this is compatible with our previous understanding and doe not change that understanding. This does not break the protocol since renaming the fields does not change the wire format.
|
I would like to know what others think. This seems reasonable to me. |
Yes, it can. The question is: do we need a Body for Span Events? Our API does not have anything like that and I do not think we need to add it to the API (but perhaps there is a good use case that I am not aware of). |
I agree, it can. However, I think there is a value in consistency and uniformness. Making it a field in Log Data model and a semantic convention in Span Event is somewhat inconsistent. |
This came up several times in the past in OpenTelemetry, not just for logs, but for Spans too. I think it is time to make it possible.
We use strong typing for attribute values. There is no way to know that a string attribute is a JSON value that needs to be decoded to be understood. |
Contributes to open-telemetry/opentelemetry-specification#622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Contributes to open-telemetry/opentelemetry-specification#622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Hi folks! Is this something you are still planning on doing? Tks. |
Contributes to open-telemetry#622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Contributes to open-telemetry/opentelemetry-specification#622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Contributes to open-telemetry/opentelemetry-specification#622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Contributes to open-telemetry/opentelemetry-specification#622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Contributes to #622 This aligns the naming to what is already used for Embedded Logs (Span.Event). There is no change of semantics.
Now that we have an official standalone logs data model it will be beneficial to see if we can align logs embedded in the Spans (Span events) with this data model.
Embedded spans should use the same or similar model except the following, which are already recorded in the containing Span:
The differences currently are:
only field names are different.We need to decide which of these differences are acceptable/necessary. My preliminary thoughts are the following:
RenameRenameName
toShortName
.ShortName
toName
. There is no semantics change. DONE.Body
to Span Events. For standalone logs theBody
is necessary to record what cannot be represented in the attributes or other fields for log formats that have unusual/unknown requirements. There are no such requirements for Span Events because we know they are generated (using OpenTelemetry API).Attributes
to allow the same nested values. This was requested/discussed in the past too.Severity
. We should use it if we believe the Error Semantics it introduces are applicable to Span Events.The text was updated successfully, but these errors were encountered: