serilog-contrib / serilog-sinks-applicationinsights

A Serilog sink that writes events to Microsoft Azure Application Insights
Apache License 2.0
220 stars 72 forks source link

Filter not working? #180

Closed niclasrothman closed 2 years ago

niclasrothman commented 2 years ago

HI there, I'm trying to add a Serilog Filter to prevent Kubernetes health requests from showing up in application insights but with no luck at all. I have created the most simple asp.net core .NET 6 API that defines a filter that should prevent all requests from being sent to Application Insights, however, all request are written to AI. Any idea what I am doing wrong?

using Microsoft.ApplicationInsights.Extensibility;
using Serilog;
using Serilog.Events;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddApplicationInsightsTelemetry();

builder.Host.UseSerilog((context, sp, loggerConfiguration) =>
{
    loggerConfiguration
        .Filter.ByExcluding(evt =>
        {
            // Just return true to test if request is filtered.
            return true;
            // var isHealthRequest = evt.Properties.TryGetValue("RequestPath", out var name) && (name as ScalarValue)?.Value as string == "/health";
            // return isHealthRequest!;
        })
        .WriteTo.ApplicationInsights(sp.GetRequiredService<TelemetryConfiguration>(), TelemetryConverter.Traces);
});

var app = builder.Build();
app.MapGet("/", () => "Hello World!");
app.MapGet("/health", () => "Healty");

app.Run();

In the output of Visual Studio and in AI Azure i see the request despite my filter:

category: Application Insights Telemetry: {"name":"AppRequests","time":"2022-02-25T17:51:18.0142358Z","iKey":"XXX","tags":{"ai.application.ver":"1.0.0.0","ai.cloud.roleInstance":"DESKTOP-MK5SQRL","ai.user.id":"crM+j","ai.operation.id":"77e6c8e9d162f4e0cb2ae33516152f79","ai.operation.name":"GET /health","ai.location.ip":"::1","ai.internal.sdkVersion":"aspnet5c:2.20.0+51c3ed8aa3f32209edf01168f9136a3ac8486c5d","ai.internal.nodeName":"DESKTOP-MK5SQRL"},"data":{"baseType":"RequestData","baseData":{"ver":2,"id":"d512738ee5c7eda5","name":"GET /health","duration":"00:00:00.0006621","success":true,"responseCode":"200","url":"https://localhost:7109/health","properties":{"_MS.ProcessedByMetricExtractors":"(Name:'Requests', Ver:'1.1')","AspNetCoreEnvironment":"Development","DeveloperMode":"true"}}}}

Any help much appreciated! Nic

UPDATE From what I can see filtering stops working when adding the line'services.AddApplicationInsightsTelemetry()' as outlined here: https://github.com/serilog/serilog-sinks-applicationinsights#telemetryconfigurationactive-is-deprecated-in-the-app-insights-sdk-for-net-core-what-do-i-do

If I remove this line and use the old way with TelemetryConfiguration.Active filtering works!

nblumhardt commented 2 years ago

Hi Nic - those requests aren't being logged by Serilog, they're being recorded by AI directly as far as I can tell, hence the Serilog settings will have no effect on them. HTH!

niclasrothman commented 2 years ago

Ok and thanks for your reply! But how should we then initialize ApplicationInsights, I've just followed the documentation that says that we should pass in the TelemetryConfiguration that was added in ConfigureServices;

image
nblumhardt commented 2 years ago

Hi! Initializing it in ConfigureServices via the telemetry configuration should allow this. I'm not familiar enough with AI to be able to answer, unfortunately.

We're currently in the process of switching maintenance teams here and don't currently have the bandwidth to help with usage questions. Posting your question on Stack Overflow, tagged serilog, should get the most eyes on it and has a good chance of attracting a correct/helpful answer. Good luck with it!

timstokman commented 2 years ago

Had the same issue. Don't really agree with closing the issue like this. The documentation this repo has suggests a setup that can't work, and will emit logging events to AI twice. So this is either a documentation issue, or an issue with the code.

For reference, I solved it like this:

        /// <summary>
        /// Add application insights without immediate logging integration
        /// </summary>
        /// <param name="services">Service collection</param>
        /// <param name="options">Application insights options</param>
        /// <returns>The service collection</returns>
        public static IServiceCollection AddApplicationInsightsTelemetryWorkerServiceWithoutLogging(this IServiceCollection services, ApplicationInsightsServiceOptions options)
        {
            services.AddApplicationInsightsTelemetryWorkerService(options);

            // Application insights immediately integrates with the logging. We want integration through serilog, so remove that integration.
            Type[] loggingTypesToRemove = new[] { typeof(ILogger<>), typeof(ILoggerProvider), typeof(ILoggerFactory) };
            var descriptorsToRemove = services.Where(descriptor => loggingTypesToRemove.Contains(descriptor.ServiceType)).ToList();

            foreach (var descriptor in descriptorsToRemove)
            {
                services.Remove(descriptor);
            }

            return services;
        }

Remove the application insights logging integration afterwards, and it shouldn't be a problem. Bit of a hack, but there's no easy way otherwise.

I think the root issue is that you can't add AI without the logging integration, and the methods that'd allow customization of which services are added are all private. I think I'll add an issue at the AI repository for this.