Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filter out logs based on ActivityTraceFlags #3918

Closed
cataggar opened this issue Nov 17, 2022 · 12 comments
Closed

filter out logs based on ActivityTraceFlags #3918

cataggar opened this issue Nov 17, 2022 · 12 comments
Labels
enhancement New feature or request logs Logging signal related

Comments

@cataggar
Copy link

I want to prevent logs from being sent to Azure.Monitor. Based on the filtering processor example, I created:

open System.Diagnostics
open OpenTelemetry
open OpenTelemetry.Logs

/// Remove Heartbeat from Host.Results
type HeartbeatLogFilter() =
    inherit BaseProcessor<LogRecord>()
    override _.OnEnd log =
        if log.CategoryName = "Host.Results" then
            let fullName =
                log.StateValues |> Seq.tryPick(fun kv ->
                    if kv.Key = "FullName" then
                        Some kv.Value
                    else
                        None
                )
            if fullName = Some "Heartbeat" then
                log.TraceFlags <- log.TraceFlags &&& ~~~ ActivityTraceFlags.Recorded

It removes ActivityTraceFlags.Recorded from the LogRecord.TraceFlags.

Expected behavior

I was expecting the log records with ActivityTraceFlags.Recorded removed to not be exported to Azure Monitor.

Actual behavior

The log records still end up in Azure Monitor.

@cijothomas
Copy link
Member

TraceFlags are part of LogRecord, but they are not used to make any decision about export vs not export for logs. (They are used for traces)

As of today, there is not easy mechanism to filter logs based on the log content itself. You can write own FilteringProcessor wrapping the exporting processor, but that'll only work if the Exporter being target has public ctor.

https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/logs/extending-the-sdk

@cijothomas
Copy link
Member

Tagging @CodeBlanch to see if we can offer a doc/feature to filter logs easily.

@cataggar
Copy link
Author

cataggar commented Dec 16, 2022

As of today, there is not easy mechanism to filter logs based on the log content itself. You can write own FilteringProcessor wrapping the exporting processor, but that'll only work if the Exporter being target has public ctor.

@cijothomas, I see a public constructor, but the class is internal.

    internal class AzureMonitorLogExporter : BaseExporter<LogRecord>
    {
        private readonly ITransmitter _transmitter;
        private readonly string _instrumentationKey;
        private readonly ResourceParser _resourceParser;
        private readonly AzureMonitorPersistentStorage _persistentStorage;

        public AzureMonitorLogExporter(AzureMonitorExporterOptions options, TokenCredential credential = null) : this(new AzureMonitorTransmitter(options, credential))
        {
        }

https://github.com/Azure/azure-sdk-for-net/blob/c3fc45c5d59afcadca84e49e291d33ff439751a0/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/AzureMonitorLogExporter.cs#L15-L24

I reopened Azure/azure-sdk-for-net#32276 (comment).

@cataggar
Copy link
Author

@cijothomas, the possibility of making AzureMonitorLogExporter public was rejected. There is still no way to filter logs easily that I'm aware of. Our logs of full of "Heartbeat"s that we do not need to be logging and I'd like to filter out. Azure/azure-functions-host#4742 (comment)

@cijothomas
Copy link
Member

Are you not able to filter the logs using https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/logs/customizing-the-sdk#log-filtering ?

@cataggar
Copy link
Author

cataggar commented Apr 14, 2023

@cijothomas, unfortunately no. That is limited to filtering on log.CategoryName. The logic is shown in the issue description. For the category Host.Results, I want to filter out any logs that have a FullName of Heartbeat.

@cijothomas
Copy link
Member

@cijothomas, unfortunately no. That is limited to filtering on log.CategoryName. The logic is shown in the issue description. For the category Host.Results, I want to filter out any logs that have a FullName of Heartbeat.

Could you ask the log producer to use a different category for Heartbeats?

@cataggar
Copy link
Author

That is the category that Azure Functions to log all requests. We need the other requests to be logged, just not certain ones like Heartbeat. I'm pursuing that angle here, but no traction on it yet. Back to this angle, why can't we filter LogRecord in an an exporter? Why limit it to its CategoryName?

@cijothomas
Copy link
Member

why can't we filter LogRecord in an an exporter

No reason. Its upto individual exporters to offer such capabilities. From OTEL, there should be some solution to filter logs - this just do not exist today, unless the Exporter itself has public ctors.

@cijothomas
Copy link
Member

dotnet/runtime#82465 - @cataggar Please check this too. ILogger is considering more filtering capabilities, which might solve the problem for all, not just OTel.

@CodeBlanch
Copy link
Member

I have thought about how the SDK might provide some filtering mechanism(s) which would work across the board, but at this time I don't have anything super great 🤣 I feel like this should really be an OTel-spec thing. What I was noodling was something along the lines of...

  1. Add a predicate for filtering records before they are handed to the exporter/batch on BaseExportProcessor<T> (would work for tracing & logs):
public abstract class BaseExportProcessor<T>
{
        public Predicate<T>? InclusionPredicate { get; set; }

        public override void OnEnd(T data)
        {
            if (this.InclusionPredicate?.Invoke(data) ?? true)
            {
                this.OnExport(data);
            }
        }
}

That part is pretty simple. The hard part is most of the existing extensions don't expose the processor so there isn't a chance to set the predicate. Exporters could be updated to expose something on the extension or their options or whatever, but we could do something like this to allow it to be set externally...

        public static TracerProvider SetExportProcessorInclusionPredicate<TExporter>(
            this TracerProvider tracerProvider,
            Predicate<Activity> predicate)
            where TExporter : BaseExporter<Activity>
        {
            if (tracerProvider is TracerProviderSdk tracerProviderSdk
                && tracerProviderSdk.Processor != null)
            {
                SetExportProcessorInclusionPredicate(tracerProviderSdk.Processor, predicate, typeof(TExporter));
            }

            return tracerProvider;
        }

        public static OpenTelemetryLoggerProvider SetExportProcessorInclusionPredicate<TExporter>(
            this OpenTelemetryLoggerProvider loggerProvider,
            Predicate<LogRecord> predicate)
            where TExporter : BaseExporter<LogRecord>
        {
            if (loggerProvider.Processor != null)
            {
                SetExportProcessorInclusionPredicate(loggerProvider.Processor, predicate, typeof(TExporter));
            }

            return loggerProvider;
        }

        private static void SetExportProcessorInclusionPredicate<T>(BaseProcessor<T> processor, Predicate<T> predicate, Type exporterType)
             where T : class
        {
            if (processor is CompositeProcessor<T> compositeProcessor)
            {
                var current = compositeProcessor.Head;
                while (current != null)
                {
                    if (current.Value is BaseExportProcessor<T> exportProcessor
                        && exportProcessor.Exporter.GetType() == exporterType)
                    {
                        exportProcessor.InclusionPredicate += predicate;
                    }

                    current = current.Next;
                }
            }
            else if (processor is BaseExportProcessor<T> exportProcessor
                && exportProcessor.Exporter.GetType() == exporterType)
            {
                exportProcessor.InclusionPredicate += predicate;
            }
        }

Kind of nasty which is why I haven't pursued it. Also it doesn't solve for users with multiple exporters of the same type sending records to different places. I think that is important for logs. Could maybe change up that API so it returns all the found BaseExportProcessor<T>s registered and then we let the user sort them out? 🤷

/cc @alanwest

@cataggar
Copy link
Author

I'm closing this and will subscribe to dotnet/runtime#82465.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request logs Logging signal related
Projects
None yet
Development

No branches or pull requests

3 participants