[bug] BaseProcessor<LogRecord>.OnStart not being called

[Leonardo-Ferreira]

Package

OpenTelemetry

Package Version

Package Name	Version
OpenTelemetry.Instrumentation.AspNetCore	1.10.1

Runtime Version

net9.0

Description

When I register a processor such as

public class MyProcessor : BaseProcessor<LogRecord>
{
    public override void OnStart(LogRecord data)
    {
        throw new Exception("BOOM");
        base.OnStart(data);
    }

    public override void OnEnd(LogRecord data)
    {
        base.OnEnd(data);
    }
}

it doesn't go boom! OnEnd gets called all the time though...

I want to create a filter processor that will inspect and possibly redact logs before they are sent outwards.

Steps to Reproduce

builder.Services.AddOpenTelemetry().WithTracing(b =>
{            
    b.AddAspNetCoreInstrumentation().AddConsoleExporter();
}).WithLogging(b =>
{
    b.AddConsoleExporter();
    b.AddProcessor(new MyProcessor());
})

Expected Result

that the log OnStart method would be called so I can prevent the log from moving forward "as-is" in certain conditions

Actual Result

Only OnEnd gets called

Additional Context

No response

[cijothomas]

Logs has no notion of start and end. Only OnEnd is called for Logs. (For Span(Activity), both start and end are invoked.)
https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/logs/extending-the-sdk#processor

[Leonardo-Ferreira]

How would a Processor filter out logs then? On the OnEnd method, I commented the base.OnEnd but the log still went out

[cijothomas]

For basic filter, ILogger has native support. https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/logs#log-filtering

But for anything more advanced, only option currently is to wrap the exporting processor in your custom processor, and have the custom processor call/not-call the exporting-processor depending on whether LogRecord should be filtered out or not.

This is not documented, but the below issue has some hints on how to do it.
#5924

[Leonardo-Ferreira]

@cijothomas Shouldn't we make BaseProcessor<LogRecord> more relevant? In my opinion, the Exporter should handle exporter specific tasks and configs, like, "In order to export to Datadog, I should do it like this and to Prometheus, like that", while the Processor should deal with the log itself

[cijothomas]

Not sure I understand the comment. Could you elaborate?

[Leonardo-Ferreira]

If it was up to me, the extensibility model provided by "BaseProcessor" would be more robust. Like, OnStart would be implemented, signifying that a log is being composed (to be written). Through it, implementers would be able to actually alter/edit/enrich/modify/filter any logs, regardless of which exporters are being used. Implementers would also be able to ABORT the log pipeline, effectively filtering out the log. OnEnd would remain signifying that a log was composed and have been dispatched to the exporter chain.

With these changes, Log Processors would have complete independence to handle log specific logic/rules.

Don't get me wrong, Exporters should continue to have such capacities, we would just encourage these to be moved to the processors for separation of concerns sake.

[cijothomas]

I still don't follow what is the exact proposal...

Through it, implementers would be able to actually alter/edit/enrich/modify/filter any logs, regardless of which exporters are being used.

This is already the case. OTel SDK never talks to exporter directly, it only hands over telemetry (LogRecord) to the Processor, and it is unto the processor to drop it or enrich it or pass it another process or pass it to exporter or do the export inside the processor or anything else.

Like, OnStart would be implemented, signifying that a log is being composed (to be written).

OTel Spec for Logs don't support it, and I don't see a reason why it should be supported.

Log signal is different from other signals, in the sense OTel integrates with existing logging libraries rather than build full fledged logging library - we expect most capabilities like filtering is already natively supported by the existing Logging library (ILogger, in this case), and OTel can compensate for anything missing there or offer OTel specific functionalities.

Exporters should continue to have such capacities, we would just encourage these to be moved to the processors for separation of concerns sake.

Again, I am not sure if I follow this. Exporters only has one responsibility - serialize the data in a format understood by its target backend and send the data there (handling endpoint specific retry etc/ as needed), and nothing more.