Add EventName parameter to Logger.Enabled

[pellared]

There is a an open question whether EventName (or EventID) should be added as an optional parameter to Logger.Enabled.

Regd. EventName being used for EnabledCheck:

1. It is very important (actually the EventId, the numerical, machine friendly version of EventName that is most important to do ultra fast checks!) for scenarios we are working on in OTel Rust, C++. I don't know if it is something every language/implementation cares about. Given spec does not prohibit an implementation from allowing more parameters, I am totally okay if spec does not mention it, as OTel C++/Rust can offer this as extras.
2. From the Event Oteps, there were mention of scenarios where Loggers can filter based on EventName, route things to different places based on EventName etc. It'd be good to wait to see the progress on Events before we can really say if EventName is a important parameter for the Enabled check.

Originally posted by @cijothomas in https://github.com/open-telemetry/opentelemetry-specification/pull/4203#discussion_r1768896732

[pellared]

Notes from 2023-09-20 Events/Logs SIG meeting:

@lmolkova, pointed out that if we would like to filter by event name then we would need to store a map (set) of disabled (or enabled) event names which can be inefficient (memory overhead).

@MSNev, had a remark that it is better to filter out events by severity rather than by name.

@pellared, said that on top of that each Logger can have its own severity threshold so each instrumentation library can have a distinct severity level.

@cijothomas, agreed that probably it is indeed better to keep the parameters small and just accept context and severity.

I am planning to discuss it during the next Specification SIG meeting. If there will be no objections then I plan to close this issue. We can reopen it in future if needed.

[pellared]

Spec SIG meeting notes: The event name may be added before stabilizing the API.

[pellared]

I am against this proposal as I do not think that handling event.name for Enabled is more important than for other attributes. Passing all attributes "destroys" the idea of Enabled which is supposed to be used by the user to check whether it makes sense to build and emit a log records (performance tuning). The parameters accepted by Logger.Enabled should be "cheap" to construct.

The SDK's Enabled implementation would get the context, severity level, instrumentation scope that should be enough.

Side note: I think that instrumentation libraries emitting events could add a event.namespace instrumentation scope attribute instead of putting the namespace to each event.name value as a prefix. EDIT: I created https://github.com/open-telemetry/opentelemetry-specification/issues/4239

[trask]

I am against this proposal as I do not think that handling event.name for Enabled is more important than for other attributes.

I'm not sure I agree, but even if we decide we want event name, I think it would be fine (maybe even better?) to introduce a new method EventEnabled (possibly taking both event name and severity), which I think would remove Go's worry about introducing a new parameter to the Enabled method(?)

[pellared]

As for Go SIG we are not worried about adding a parameter in future. We decided to favor future-compatibilty instead of slightly better ergonomics.

I am not sure if this issue should be a blocker for stabilizing Logger.Enabled.

[cijothomas]

I am against this proposal as I do not think that handling event.name for Enabled is more important than for other attributes. Passing all attributes "destroys" the idea of Enabled which is supposed to be used by the user to check whether it makes sense to build and emit a log records (performance tuning). The parameters accepted by Logger.Enabled should be "cheap" to construct.

The SDK's Enabled implementation would get the context, severity level, instrumentation scope that should be enough.

Side note: I think that instrumentation libraries emitting events could add a event.namespace instrumentation scope attribute instead of putting the namespace to each event.name value as a prefix. EDIT: I created #4239

If "event.name" is an attribute, then I totally agree! (Which is why OTel .NET, C++, Rust all decided to put EventName as a top-level field in LogRecord, and not inside Attributes)

[pellared]

It is possible that it will be a parameter for a dedicated enabled method for events (and not for log records):

• https://github.com/open-telemetry/opentelemetry-specification/issues/4263

[pellared]

Closing per https://github.com/open-telemetry/opentelemetry-specification/issues/4220#issuecomment-2389082794 and https://github.com/open-telemetry/opentelemetry-specification/issues/4220#issuecomment-2415980647

[pellared]

Reopening as OTel Go SIG strongly prefers https://github.com/open-telemetry/opentelemetry-go/pull/6018

[pellared]

Per https://github.com/open-telemetry/opentelemetry-specification/issues/4220#issuecomment-2364270664, I think it is not required for the stability of Events API. We can always add a parameter later.

[pellared]

2025-04-23 SIG meeting notes: There is a proposal to add this parameter to the specification. However, it would be good to have a use case for it.

[pellared]

@cijothomas, is adding EventName parameter to Enabled still important for C++ and Rust (given EventName is a string)? We could add this for Go. Yet, we have not heard of any good use-case so far (nobody asked for it).

[cijothomas]

@cijothomas, is adding EventName parameter to Enabled still important for C++ and Rust (given EventName is a string)? We could add this for Go. Yet, we have not heard of any good use-case so far (nobody asked for it).

Yes. Otel Rust already has more parameters for the Enabled() check.

We expect to use that in Etw/user_events exporters scenarios, where we ask the OSKernel about whether a given event is interesting or not. Its currently leveraging severity only, but we expect to leverage EventName(and scope too) in the future.

[pellared]

I created a PR:

• https://github.com/open-telemetry/opentelemetry-specification/pull/4489