jack-berg
commented
1 month ago We have experimental support for checking if a Logger or Tracer is enabled, and if a particular instrument is enabled (all instruments have APIs like this).
This seems like an extension of that request - support for the logger.isDebugEnabled() style of API, but for events.
The suggestion to accomplish by accepting a severity argument to the build is interesting, since it means you can accomplish this goal of avoiding unnecessary compute without requiring anything extra from the user. I.e. you can avoid the isDebugEnabled() call altogether.
It seems like it may be premature though, since there is nothing in the spec yet that suggests SDK capabilities around filtering based on log record severity. Some say you should configure this in the log framework when you're configuring it to bridge logs to opentelemetry - i.e. only configure logs with severity info or higher to be bridged. But this falls apart when you consider the event API since we want users to call it directly. Seems like the event SIG should consider whether severity should be a required arg as a prerequisite to stability, and push to evolve the log SDK to have the ability to filter based on severity.
The main idea is that now, even if users sets a "filter" on events with severity lower than "error" the implementation of the EventLogger.builder() cannot return a "No-op" builder and has to record all the data until "build()" is called because the severity is not known.
It would be amazing to require severity to be provided as early as possible (with the event name) so that we can return a "no-op" builder when we are confident the event will be dropped, and not have to store attributes, etc.