semantic-conventions icon indicating copy to clipboard operation
semantic-conventions copied to clipboard

Events should recommend having an outcome attribute

Open thompson-tomo opened this issue 1 year ago • 13 comments

Area(s)

area:event

Is your change request related to a problem? Please describe.

I want easily visualise my events based on success/failure

Describe the solution you'd like

I would like the events api/payload to include a convention of being able to specify the status/result of the event which is a strongly typed enumeration similar to as what is in Elastic Common Schema & also supported by the span status field: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status

Tooling would be able to aggregate the statuses to produce additional metrics about the events. A key point is just because an event doesn't mean that a retry didn't occur which lead to the span being successful.

Describe alternatives you've considered

Use my own standard but compatibility would be lost across systems

Additional context

https://www.elastic.co/guide/en/ecs/current/ecs-allowed-values-event-outcome.html

thompson-tomo avatar May 29 '24 03:05 thompson-tomo

Does this belong on the "core" Event abstraction, or as a semantic conventions?

cc @jack-berg @MSNev

jsuereth avatar May 29 '24 16:05 jsuereth

hi @thompson-tomo, does the event severity field (e.g. DEBUG, INFO, WARN, ERROR) address your needs?

trask avatar May 29 '24 18:05 trask

I would 2nd what @trask is suggesting, this looks like it would map to the severity which is already available as an optional field (as not all events want / need this).

Failing that (if it doesn't really match the severity), then I would recommend that this would just be a general semantic convention attribute (in the event.* attribute space) so that if can be optionally added as an attribute to any event in the same manner as being proposed for #1074 (ie. it's an optional piece of data that does not explicitly relate to the event detail) again explicitly optional as not all events will want or need to include this.

Now there is also a 3rd option, which seems (maybe) "more" relevant to the description of my events, in that you can always define your own specific events and those events could be (or describe) a mapping of ECS events -> OTel events (by defining your own event.name(s) as required, then this field would then just be documented as part of those specific events (whether they are attributes or body fields)

MSNev avatar May 29 '24 19:05 MSNev

+1 for severity.

Also, nothing stops someone from using the error.type attribute - there is no need to define a new one specifically for events.

lmolkova avatar May 29 '24 20:05 lmolkova

So it would also work to have it as an optional attribute which has been suggested. I would not like to use severity for this purpose as that has other uses.

Let me talk through an example of how I would be using the OTEL concepts keep in mind I am coming from the dotnet world.

An api request comes in to a Web api which creates an activity/transaction/trace. Within the method log records are written to capture info. An event is added for each call to the db with the name indicating the event type similar to what ecs has as part of their categorisation fields obviously in this case we would be using the dot notation. The outcome field indicates if the method was successful. At the same time a file could be written to the file system containing data from the payload hence a second event would be generated but with a different name.

In the above use case the severity of events could be used as follows:

  • a select event could be debug/trace
  • insert would be info
  • delete/update would be warning as data is being changed.

As such there is no way to indicate if the select failed & the failure of a select should be handled differently to an insert hence different severity.

thompson-tomo avatar May 30 '24 04:05 thompson-tomo

After thinking about this more, i have identified an additional benefit/use case which would be able to be achieved by having an outcome strongly typed property. Scenario is that a success is indicated by a 1 & a failure as 0, with it possible to be aggregated and published also as a metric likely a gauge so that you could track success percentage &/or a counter of success/failure events. Hence this gives us even greater insight into the health of our system etc.

thompson-tomo avatar Jun 04 '24 10:06 thompson-tomo

@thompson-tomo Not all events will represent "success percentage &/or a counter of success/failure events" scenarios.

So as I called out above, there is nothing stopping these specific events from providing their own body field identifying this value.

As with the event.summary discussion however, I'm not opposed to introducing an optional field in the event namespace that any event of this type could choose to use within their own defined events, but I would be 100% opposed to making this a mandatory attribute.

MSNev avatar Jun 24 '24 15:06 MSNev

@thompson-tomo we record success or failure of operations on Spans using the Status field. Is that sufficient? Is there a need to double up on this concept and have operation status recorded in two different places?

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status

tedsuo avatar Jul 12 '24 17:07 tedsuo

Hi @tedsuo & @MSNev as mentioned above, if you want to make the outcome attribute as optional, that would also work for me.

The reason i feel that placing the outcome/status on the event is the best option would be to cater for the scenario where 1 span is triggering multiple events which could have different outcome/statuses. But yes i do agree it probs makes sense to leverage the same terminology & enumeration for events as Spans. Hence will update the description.

thompson-tomo avatar Jul 14 '24 23:07 thompson-tomo

Please consider reusing existing error.type attribute on events, I don't see why it can't work in the scenario you shared in https://github.com/open-telemetry/semantic-conventions/issues/1089#issuecomment-2138636832

lmolkova avatar Jul 14 '24 23:07 lmolkova

Perhaps we can use error.type to provide additional context if the status is error.

thompson-tomo avatar Jul 15 '24 03:07 thompson-tomo

Perhaps we can use error.type to provide additional context if the status is error.

could we close this issue or there is something else that needs to be addressed?

lmolkova avatar Jul 15 '24 20:07 lmolkova

Nope, I am still wanting to add a convention of including a status property on events, the error.type is to complement the status. Also is it in the semantic convention to include error.type on events?

thompson-tomo avatar Jul 16 '24 07:07 thompson-tomo