Make severity required to allow filtering by severity as early as possible

[bogdandrutu]

Or alternative require that severity is provided as soon as possible in the https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/event-api.md#eventlogger, see for example an anti pattern of this in java where the severity can be provided after lots of other files in https://github.com/open-telemetry/opentelemetry-java/blob/main/api/incubator/src/main/java/io/opentelemetry/api/incubator/events/EventBuilder.java#L140 and that forces implementations to have to store all properties (attributes, etc.) until build time since severity can also be changed by calling setSeverity multiple times.

[jack-berg]

that forces implementations to have to store all properties (attributes, etc.) until build time since severity can also be changed by calling setSeverity multiple times.

Can you expand on this? If an implementation choses a builder pattern for constructing logs as java has done, how would that implementation avoid storing all properties?

[cijothomas]

Is the expectation that filtering is done at the OpenTelemetry level based on severity? (From my experience, such filtering is typically done inside the logging framework itself, so logs of undesired severity won't even reach OTel bridge layer)

[pellared]

Is the expectation that filtering is done at the OpenTelemetry level based on severity? (From my experience, such filtering is typically done inside the logging framework itself, so logs of undesired severity won't even reach OTel bridge layer)

Yes, we are expecting such use cases. E.g. when the minimum severity level could be set via OpenTelemetry Configuration File. Related PR: #4020.

[bogdandrutu]

@pellared that is different because Logger is not a real logger :) (I said this many times, but I will repeat). For EventLogger which is a real Logger, users will write events, and all the logging API I've seen they do offer a "isEnabledForLevel/Severity()" but they also allow filtering by level/severity. See a good answer from @jack-berg open-telemetry/opentelemetry-java#6603

The main idea is that an implementation of the EventLogger should be able to do minimal to zero work if filtering by severity is supported and this event will be filtered anyway. In order to do that with a "builder" pattern, what you can do is to pass the severity as an argument to the builder creation method, so that the implementation can return a no-op builder if that is the case and all calls will be no-ops. With the current API in Java at least this is impossible since the severity is not available when creating the builder, hence you need to return a real builder because you don't know if this event will or not be filtered.

[bogdandrutu]

Is the expectation that filtering is done at the OpenTelemetry level based on severity? (From my experience, such filtering is typically done inside the logging framework itself, so logs of undesired severity won't even reach OTel bridge layer)

For the EventLogger (which is a logging framework in my opinion or a thin wrapper over a logging framework) you need the level from the beginning, which is what every logging framework requires that first you provide the level and after everything else so that filtering can be done efficiently.

I expect that for the EventLogger implementation someone will use a logging framework to implement it, so hence you need the level immediately.

@cijothomas today based on the API in java (at least) https://github.com/open-telemetry/opentelemetry-java/blob/main/api/incubator/src/main/java/io/opentelemetry/api/incubator/events/EventLogger.java#L43 I need to create a real builder and accumulate all details for the event (create maps, etc.) because I don't know the level and cannot pass it to the logging framework, then only at the end when level is available I can do filtering, so a lot of unnecessary work before I can drop the data.

[mtwo]

Added to the Log / Event SIG project board

[pellared]

@bogdandrutu, what I described was just a clarification that allowing filtering by severity is needed also for logging library bridges (at least in Go). I think that nothing I wrote was against your proposal. Related issue: #4208