Specify how Logs SDK implements Enabled

[pellared]

Context: #4203 (comment)

Blocked by:

• Define Enabled parameters for Logger #4203

[pellared]

Proposal A - Extend Processor

Below is a proposal mostly based on how Go Logs SDK currently implements Logger.Enabled.
Spike: open-telemetry/opentelemetry-go#5816

The idea is to add an OnEnabled operation to the LogRecordProcessor.

The parameters of OnEnabled would be defined in the same way as Logger.Enabled. However, it is important to notice that in future the parameters may independently change (for instance, the LogRecordProcessor.OnEnabled may start accepting an instrumentation scope to allow dynamic filtering of loggers by name). Still, I propose to start with the minimal set of parameters.

The return value of OnEnabled would be defined in the same way as Logger.Enabled.

Implementation of OnEnabled operation is optional so that the specification and existing processors implementations remain backwards compatible. If a LogRecordProcessor does not implement OnEnabled then it defaults to returning true.

When the user calls Logger.Enabled, the SDK would call OnEnabled in the same order as processors were registered until any of the processors' returns true. Logger.Enabled returns false if none of registered processors returns true. Pseudocode:

func (l *logger) Enabled(ctx context.Context, param log.EnabledParameters) bool {
	newParam := l.newEnabledParameters(ctx, param)
	for _, p := range l.provider.processors {
		if enabled := p.Enabled(ctx, newParam); enabled {
			return true
		}
	}
	return false
}

In order the implement filtering, the processor would need to wrap (decorate) an existing processor. Example pseudocode of a processor filtering log records by setting a minimum severity level:

// LogProcessor is an [log.Processor] implementation that wraps another
// [log.Processor]. It will pass-through calls to OnEmit and Enabled for
// records with severity greater than or equal to a minimum. All other method
// calls are passed to the wrapped [log.Processor].
type LogProcessor struct {
	log.Processor
	Minimum api.Severity
}

// OnEmit passes ctx and r to the [log.Processor] that p wraps if the severity
// of record is greater than or equal to p.Minimum. Otherwise, record is
// dropped.
func (p *LogProcessor) OnEmit(ctx context.Context, record *log.Record) error {
	if record.Severity() >= p.Minimum {
		return p.Processor.OnEmit(ctx, record)
	}
	return nil
}

// Enabled returns if the [log.Processor] that p wraps is enabled if the
// severity of record is greater than or equal to p.Minimum. Otherwise false is
// returned.
func (p *LogProcessor) OnEnabled(ctx context.Context, param log.EnabledParameter) bool {
	lvl, ok := param.Severity()
	if !ok {
		return true
	}
	return lvl >= p.Minimum && p.Processor.OnEnabled(ctx, param)
}

I also want to call out that the idea of composing/wrapping/decorating the processors is already used by the isolating processor. Composing of processors is necessary to achieve complex processing pipelines in the SDK.

Personal remarks:
I am going to work on another proposal(s) which be more inspired on tracing sampling. I want to coin a simpler filtering mechanism - a new abstraction called Filterer that would be used by SDK's Logger in both Emit and Enabled implementations.

[pellared]

Proposal B - Add filtering via Filterer

Below is a proposal inspired by sampling design in tracing SDK.
Spike: open-telemetry/opentelemetry-go#5825

The idea is to add a new abstraction named Filterer with a Filter method.

The parameters of Filter would be defined in the same way as Logger.Enabled. However, it is important to notice that in future the parameters may independently change (for instance, the Filterer.Filter may start accepting an instrumentation scope to allow dynamic filtering of loggers by name). Still, I propose to start with the minimal set of parameters.

The return value of Filter would be defined in the same way as Logger.Enabled.

Pseudocode:

type Filterer interface {
	Filter(ctx context.Context, param FilterParameters) bool
}

When the user calls Logger.Enabled, the SDK would call Filter in the same order as filterers were registered until any processor returns false (then it breaks and returns false). Logger.Enabled returns true if none of registered filterers returned false. Pseudocode:

// Enabled returns false if at least one Filterer held by the LoggerProvider
// that created the logger will return false for the provided context and param.
func (l *logger) Enabled(ctx context.Context, param log.EnabledParameters) bool {
	newParam := l.newEnabledParameters(param)
	for _, flt := range l.provider.filterers {
		if !flt.Filter(ctx, newParam) {
			return false
		}
	}
	return true
}

When the user calls Logger.Emit, the SDK would first check if the record should be filtered out. Pseudocode:

func (l *logger) Emit(ctx context.Context, r log.Record) {
	param:= l.toEnabledParameters(r)
	if !l.Enabled(ctx, param) {
		return
	}

	newRecord := l.newRecord(ctx, r)
	for _, p := range l.provider.processors {
		if err := p.OnEmit(ctx, &newRecord); err != nil {
			otel.Handle(err)
		}
	}
}

Pseudocode of a custom minimum severity level filterer :

type MinSevFilterer struct {
	Minimum api.Severity
}

func (f *MinSevFilterer) Filter(ctx context.Context, param log.FilterParameters) bool {
	lvl, ok := param.Severity()
	if !ok {
		return true
	}
	return lvl >= p.Minimum
}

Filtering is coupled to emitting log records. Therefore, a custom processor filtering implementation does not need to implement filtering on both OnEmit and OnEnabled as in Proposal A. It makes also makes defining filters easier for users.

This design should be easier to be added to existing SDKs as it adds a new abstraction instead of adding more responsibilities to Processor.

IMPORTANT:
This proposal assumes that each logger provider is used to define a separate processing pipeline (e.g. open-telemetry/opentelemetry-go#5830). It is NOT inline with #4010 (and #4062) which added the possibility to have multiple processing pipelines to the SDK's processing. It may decrease the performance as it will introduce more overhead and other unwanted side-effects (e.g. more debug logging). The benefit is that the solution is not coupled to the SDK (people may also use this approach e.g. to use few implementation of API). Basically it assumes that for complex pipelines the user is composing providers (and not processors).

[MSNev]

I'm a little concerned that by "defining" the way that "enabled" should be implemented (especially adding layers of filtering (as part of the required implementation)) would (potentially) worse from a perf perspective than just constructing and emitting the Log that eventually gets dropped. ie. the whole point of the "Is enabled" is for avoid perf impact...

[MSNev]

I think it's fine to define "what" options should be used (ie the parameters) to "check" whether something is enabled or not and this should be limited.
If an end-user application wants to do something more complicated then the "better" option would be to have "them" (not OTel) implement their own "Filtered" logger thingy.

[pellared]

I'm a little concerned that by "defining" the way that "enabled" should be implemented (especially adding layers of filtering (as part of the required implementation)) would (potentially) worse from a perf perspective than just constructing and emitting the Log that eventually gets dropped. ie. the whole point of the "Is enabled" is for avoid perf impact...

I am not sure if I follow.

Logger.Enabled gives the possibility to for the user to check if it is even necessary to construct and emit a big and expensive log record.

Which part of the implementation proposal (A or B or both) are you concerned about?