-
Notifications
You must be signed in to change notification settings - Fork 183
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
Logs/Events guidance on what to put to attributes vs body #1651
Comments
I know there's been a lot of back and forth about this, but I think there's a correct answer here: Use the body to record what happened. Use attributes as a means of capturing supplementary contextual information. Semantic conventions should define fields for the body. Instrumentation authors should primarily record data to the body. The emit event API should prioritize the body, and provide great UX for adding to it. The emit event API should retain the ability to record attributes, but de-prioritize them and indicate that they are meant for contextualizing the event, rather than recording what happened. Reasoning:
A few additional reactions:
This is only the case because we haven't tackled the problem of how (attribute) limits apply to AnyValue body. This is a miss, and something we need to solve regardless of whether we put the data in body or attributes. I.e. putting the data in body doesn't magically erase backend limitations on size.
Since log record attributes support complex types today for bridging purposes, I think backends already need to figure out how query / index AnyValue. The difference between body and attributes is that body is type I think we need to tease apart the questions of "how should we instrument this" and "what do backends do". I don't think we should let current backend limitations encourage us to model instrumentation in a less optimal way. We should count on backends evolving over time to reflect the emerging standards being created. In the interim, we should create tools like Event to SpanEvent Bridge to help meet users / backends where they're at today - tools that translate from our idealized representations we use in instrumentation to representations currently supported. Sorry for the wall of text. Trying to be complete for this elusive question that continues to plague us. 🙂 |
I take your argument on consistency and that simple guidance is needed. I'm trying to find a solution that would support other requirements too:
A few more thoughts:
I prefer the following direction (we briefly chatted with @trask and @jsuereth about it in the past):
[Update] |
Generally, session, user and other correlation items are not (and should not be) part of the event as they are contextual in nature. With the exception of specific events related to user logging on / of, session start / stop and as part of these event definitions they would define the event where these
On this, if this situation occurred (because of the specific event), then they would know because the definition of the event would define this. But! by default backends should NOT make any assumptions about the "name" (or path) of any content of the As defined [in the events data-model [Update]
Agree, this is also why the event definition "supports" having no body, because there are "simple" events which will not require any specific fields as they would only represent contextual state changes. And for the existing "span" events there have been many "hacks" to squash the detail into a span event, due to the limitations of span events, so I don't believe that we should be "designing" the log based events around these historical "hacks", but we should acknowledge that there ARE simple events.
|
Just a quick point of process. It seems like there is actual guidance today on this topic, added in #1263. Is it safe to say we're revisiting and potentially clarifying that guidance?
You're right - to the extent that a field is used within an event with the same semantics as an attribute on a metric or span, I think the best case is to use the same attribute name as from the global registry. Let me lay out a few more details of my vision that (somehow?) I omitted before:
Events end up having two bags of attributes:
Query ergonomics shouldn't be any different because we guarantee that event payloads are
Agree attributes are simple and intuitive. So let's restrict the event API's body to be
I don't think so. Say that instrumentation always records what happened, and doesn't concern itself with any contextual information. Users can optionally enrich the records with additional contextual attributes in processors (or maybe with instrumentation customization callbacks). If its not recorded by the instrumentation, its contextual. Everything that we want recorded by the instrumentation is what happened.
Its certainly a direction we could go but I think its valuable to separate between what is recorded by instrumentation / conforms to semconv and what is extra bits of contextual information added by processors. As for span event's not having a body, I think we can nail down the mapping between events and span events later, but that there are reasonable options to do this.
This guidance meets the consistent criteria, which I think is the most important thing. But I think its rather difficult guidance to follow for a casual user. Events defined in semconv will likely be able to work out rules of thumb for what what are fields that might be aggregated / correlated with other signals, but casual users recording custom events may consistently struggle to do the right thing. Also, I just want to mention that while I'm stating an opinion here on how things might work, I don't plan on blocking or getting in the way of whatever the log SIG decides. This is just food for thought. For the purposes of this convo, think of me as a casual user and not wearing my TC hat. |
@jack-berg thanks for the context and sharing your thoughts - it's extremely valuable.
yes, I think we need to clarify this guidance. Let me clarify my understanding on your proposal (with some assumptions on how it solves cross-signal correlation).
Is my understanding correct? If so, I have a few additional thoughts:
I think we are going in the same direction though - attributes and body fields are the same and intend to transmit the same information. So I have a radical question on why do we need both (given that logs support Important Why don't we put everything into the attributes? You can only correlate across different signals using standard attributes and only logs/events attributes can have complex attribute value types. Backends may index based on attributes and may recognize known types by the attribute name. Queries/post-processing is the same. Everything is the same as today, but we open the door to Body is only used for unstructured things (log messages, request/response payloads/etc). Body is still This would be the most consistent solution, but a radical one. |
The proposal above applies nicely to existing semconvs
|
Yes. The instrumentations SHOULD not or CANNOT record contextual attributes is TBD in my opinion. I would say minimally its discouraged. But perhaps there is a case for instrumentation customization as discussed here.
That's already the case tho in other signals. E.g. spans contain attributes which are not present on corresponding metrics. All the fields are part of the global attribute registry, so its easy to go see what they mean. Albeit you may have to go look at a specific convention to see what the field means in the context of that convention.
One way or another the data processing pipeline is receiving a list of key value pairs, some of which constitute the event payload, some of which are extra (either added by user or instrumentation not conforming to conventions). By splitting payload into body, and additional contextual attributes into attributes, we just make this difference explicit for consuming pipelines. If they don't need to make use of this difference, they can just merge the two sources of attributes together and its no different than if they were merged to begin with.
The rule of thumb I would define is if its a field we want to define in semantic conventions for an event, then put it in the event payload (body). Attributes becomes purely extra stuff - I'm imaging app specific context things like baggage and the like. In the case of a connection termination event, presumably the
👍 I don't think its strictly necessary, but it is nice because:
This question is pretty aligned with my POV, just opting to put everything in attributes instead of body. The thing I like about this (and my vision for the same reason) is that it makes it super simple for users. If we went in this direction, I would say that events should always leave body blank. And the event API shouldn't provide a mechanism to record to it. When events do want to record unstructured things like request / response payloads, isn't it still better if we record those in an attribute key / value pair where the key is describes what the unstructured thing is? I.e. |
After the convo in the 12/10/24 spec SIG, I'm coming around to @lmolkova's proposal to dump everything in attributes. The highest value argument I saw for my suggestion of putting everything recorded by instrumentation in the body and all contextual information added later in attributes was to make it easy for backends to differentiate the payload for the purpose of validation. But consider this from the perspective a backend looking to perform such validation:
So if that argument goes away, the rest of my arguments seem weak compared to the arguments in favor of putting everything in attributes. Consider me in support of @lmolkova's proposal to put everything in attributes. |
Thanks @jack-berg, we had a similar discussion on validation in the Logs SIG call today. The validation needs schema (hardcoded or supplied) and given that the schema exists, the validation can be applied to the body and attributes together. And, while we would not encourage people to modify event payload, it still can be enriched, PII redacted, truncated, post-processed in other ways (e.g. here's the proposal for storing certain properties in an external storage and replacing them with ref URL), so we cannot provide any guarantees that the body received is exactly the same as the body produced. Other relevant discussions from the Logs SIG:
|
👍
In these cases, wouldn't it still be better to put it in attributes with a descriptive key defining "what" the opaque / external / verbose / binary data is? |
probably so, unless we want to have a dedicated place that's known to be opaque / external / verbose / binary (and backends can optimize for it). It can still be solved with a dedicated attribute, but given that we already have body and use it in log bridges for exactly the same purpose, it seems valid to keep using it. |
It's not clear what should go to event attribute vs body.
Some considerations:
As a result, each event author needs to decide where each property goes and take into account backend limitations.
We need some clear guidance in semantic conventions, but also this affects event API design.
The text was updated successfully, but these errors were encountered: