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

TelemetryConfiguration.Active is deprecated #121

Closed
mikegoatly opened this issue Sep 25, 2019 · 72 comments
Closed

TelemetryConfiguration.Active is deprecated #121

mikegoatly opened this issue Sep 25, 2019 · 72 comments

Comments

@mikegoatly
Copy link

Context: I've been working to upgrade some applications to .NET Core 3.0 - this has led me to update a number of packages as well, including App Insights.

Previously the recommendation was to configure AppInsights and Serilog together using the IHostBuilder in Program.cs, e.g.:

return builder
    .UseApplicationInsights()
    .UseSerilog((context, configuration) =>
{
    var telemetry = TelemetryConfiguration.Active;

    configuration
        .ReadFrom.Configuration(context.Configuration)
        .MinimumLevel.Override("Microsoft.AspNetCore.Authentication", LogEventLevel.Information)
        .Enrich.FromLogContext()
        .WriteTo.Console(outputTemplate: "[{Timestamp:HH:mm:ss} {Level}] {SourceContext}{NewLine}{Message:lj}{NewLine}{Exception}{NewLine}", theme: AnsiConsoleTheme.Literate)
        .WriteTo.ApplicationInsights(telemetry, new TraceTelemetryConverter());
},
writeToProviders: true);

But after upgrading to the latest AppInsights, I'm starting to get the warnings (as errors):

'ApplicationInsightsWebHostBuilderExtensions.UseApplicationInsights(IWebHostBuilder)' is obsolete: 'This method is deprecated in favor of AddApplicationInsightsTelemetry() extension method on IServiceCollection.'

and

'TelemetryConfiguration.Active' is obsolete: 'We do not recommend using TelemetryConfiguration.Active on .NET Core. See https://github.com/microsoft/ApplicationInsights-dotnet/issues/1152 for more details'

The first can be overcome by configuring App Insights during ConfigureServices, but I'm left a bit stumped as to how to configure Serilog without TelemetryConfiguration.Active - is there a recommended way to do this?

Apologies if I've missed something in the documentation - I couldn't spot anything relevant to this.

@cmeeren
Copy link

cmeeren commented Sep 30, 2019

Yes, this is puzzling. Logging is set up in UseSerilog, but the logging setup depends on a telemetry configuration that isn't set up before ConfigureServices (in the call to AddApplicationInsightsTelemetry).

Any insights are most welcome.

Related: serilog/serilog-aspnetcore#84

@cmeeren
Copy link

cmeeren commented Oct 1, 2019

Cross-posting potential workaround from serilog/serilog-aspnetcore#84 (comment):

After checking library sources and experimenting for many hours today, here is a workaround I have arrived at which seems to work. Let me know if I'm shooting myself in the foot.

In doing this I have sacrificed logging early startup errors (in main or Startup.ConfigureServices up to and including where I configure the logger), but I am successfully logging "late" startup errors (e.g. in the Startup.Configure method) to Application Insights, and everything else seems to work as normal after (very) cursory testing.

Program.fs

[<EntryPoint>]
let main args =
  WebHost
    .CreateDefaultBuilder(args)
    .CaptureStartupErrors(true)
    .UseSerilog(dispose = true)
    .UseStartup<Startup>()
    .Build()
    .Run()
  0

Startup.fs

type Startup() =

  member __.ConfigureServices(services: IServiceCollection) : unit =
    services.AddApplicationInsightsTelemetry() |> ignore
    let sp = services.BuildServiceProvider()
    let telemetryConfig = sp.GetRequiredService<TelemetryConfiguration>()

    Log.Logger <-
      LoggerConfiguration()
        .WriteTo.ApplicationInsights(telemetryConfig, TelemetryConverter.Traces)
        .CreateLogger()

    services.AddSingleton<ILogger>(Log.Logger) |> ignore

@Lybecker
Copy link

Lybecker commented Oct 8, 2019

@cmatskas it works for me 👍

@crgolden
Copy link

I changed it like this and it seems to be working for me:

var log = new LoggerConfiguration()
    .WriteTo
	.ApplicationInsights(TelemetryConfiguration.CreateDefault(), TelemetryConverter.Events)
    .CreateLogger();

@cmeeren
Copy link

cmeeren commented Oct 15, 2019

@crgolden AFAIK that will only work if you don't otherwise configure the telemetry configuration through DI (in ConfigureServices). Once you start adding telemetry processors and initializers, or changing other telemetry settings, those changes will not apply to the configuration you create directly in WriteTo.ApplicationInsights (or configurations you create directly any other places for that matter).

@ghost
Copy link

ghost commented Oct 15, 2019

In my AspNetCore 3.0 web app, I got this working with the recommended way of setting up logging in Program.cs with the following code:

private static IHostBuilder CreateHostBuilder(string[] args) =>
	Host.CreateDefaultBuilder(args)
		.ConfigureWebHostDefaults(webBuilder =>
		{
			webBuilder
				.ConfigureLogging((hostingContext, logging) => logging.ClearProviders())
				.UseSerilog((hostingContext, loggerConfiguration) =>
				{
					var telemetryConfiguration = TelemetryConfiguration.CreateDefault();
					telemetryConfiguration.InstrumentationKey = hostingContext.Configuration["ApplicationInsights:InstrumentationKey"];

					loggerConfiguration
						.MinimumLevel.Information()
						.MinimumLevel.Override("Microsoft", LogEventLevel.Warning)
						.MinimumLevel.Override("System", LogEventLevel.Warning)
#if DEBUG
						.MinimumLevel.Verbose()
						.WriteTo.Console()
						.WriteTo.Debug()
#endif
						.Enrich.FromLogContext()
						.Enrich.WithMachineName()
						.WriteTo.ApplicationInsights(telemetryConfiguration, TelemetryConverter.Traces);
				})
				.UseStartup<Startup>();
		});

In addition to the code above, I also have the normal services.AddApplicationInsightsTelemetry() in Startup/ConfigureServices and the "default" appsetting:

  "ApplicationInsights": {
    "InstrumentationKey": "mykey"
  },

However, I don't have a ApplicationInsights.config at all. I suspect that file won't be read with this code.

@cmeeren
Copy link

cmeeren commented Oct 15, 2019

@MathiasRonnblom-Cint I think my comment above applies to your solution, as well. Since you instantiate a new TelemetryConfiguration and pass it to WriteTo.ApplicationInsights, you won't get telemetry initializers/processors (configured in ConfigureServices) added to the one passed to WriteTo.ApplicationInsights.

@JasonTheProgrammer
Copy link

@cmeeren

Startup.fs

type Startup() =

  member __.ConfigureServices(services: IServiceCollection) : unit =
    services.AddApplicationInsightsTelemetry() |> ignore
    let sp = services.BuildServiceProvider()
    let telemetryConfig = sp.GetRequiredService<TelemetryConfiguration>()

    Log.Logger <-
      LoggerConfiguration()
        .WriteTo.ApplicationInsights(telemetryConfig, TelemetryConverter.Traces)
        .CreateLogger()

    services.AddSingleton<ILogger>(Log.Logger) |> ignore

I'd be a little wary of using BuildServiceProvider(). When the Startup class is constructed by the framework, there will already be a service provider built. If you built another one, it will not have the same lifetime scope, which could be problematic for singletons (which I think you'll find, is how TelemetryConfiguration is registered).

Instead, as I posted over in issue 84, I'd recommend using the constructor of Startup to request an instance of TelemetryConfiguration. serilog/serilog-aspnetcore#84 (comment)

Also, I agree with your feedback above about people creating new instances of TelemetryConfiguration. It will "work" at a superficial level, but you won't be sharing around the same instance, which would cause unexpected outcomes if you do any further configuration on the instance.

@cwoolum
Copy link

cwoolum commented Oct 16, 2019

I got this working in AspNet Core 3.0 and I believe it avoids the pitfalls of the previous solutions. I had to replace UseSerilog with my own registration to access the built IServiceProvider.

return Host.CreateDefaultBuilder(args)
                .ConfigureAppConfiguration((context, config) =>
                {
                    // Add Key Vault or whatever
                })
                .ConfigureLogging((context, logging) =>
                {
                    logging.ClearProviders();
                })
                .ConfigureServices((HostBuilderContext context, IServiceCollection services) =>
                {
                    // Explicitly use the configuration previously built above
                    services.AddApplicationInsightsTelemetry(context.Configuration);
                    // Add enrichers and processors
                    services.AddApplicationInsightsKubernetesEnricher();
                    services.AddApplicationInsightsTelemetryProcessor<HealthCheckTelemetryFilter>();
                    
                    services.AddSingleton<ILoggerFactory>(provider =>
                    {
                        TelemetryConfiguration telemetryConfiguration = provider.GetRequiredService<TelemetryConfiguration>();
                        var loggerConfiguration = new LoggerConfiguration();

                        loggerConfiguration.ReadFrom.Configuration(context.Configuration)
                           .Enrich.WithExceptionDetails()
                           .Enrich.FromLogContext()
                           .WriteTo.Console()
                           .WriteTo.ApplicationInsights(telemetryConfiguration, TelemetryConverter.Traces);

                        Serilog.Core.Logger logger = loggerConfiguration.CreateLogger();
                       
                        Log.Logger = logger;

                        var factory = new SerilogLoggerFactory(logger, true);

                        return factory;
                    });
                })
                .ConfigureWebHostDefaults(webBuilder =>
                {
                    webBuilder.UseStartup<T>();
                });

@cmeeren
Copy link

cmeeren commented Oct 17, 2019

By removing UseSerilog you miss out on additional work done by that method (directly or through internal helper functions). In your case, that seems to be limited to registering the IDiagnosticsContext:

https://github.com/serilog/serilog-aspnetcore/blob/71165692d5f66c811c3b251047b12c259ac2fe23/src/Serilog.AspNetCore/SerilogWebHostBuilderExtensions.cs#L149-L156

If additional arguments were supplied to the initial UseSerilog call, even more functionality must be duplicated, starting to destroy the purpose of having a library in the first place.

@cwoolum
Copy link

cwoolum commented Oct 17, 2019

Yeah, I do register those manually for my own app but left them out of the example code above.

@MattMinke
Copy link

So far the conversation has focused on asp.net core applications but this also needs to be solved for non-http context applications and console applications. for console applications I think something like this should work but have yet to figure out why it is not. Any feed back on how to make this work or improve this is appreciated

using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.ApplicationInsights.WorkerService;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.ApplicationInsights;
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.Extensions.DependencyInjection;
using PowerArgs;
using Serilog;
using System;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.ApplicationInsights.WorkerService;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
using Serilog.Events;
using Serilog.Exceptions;
using Serilog.Extensions.Logging;
using System.Collections.Generic;
using System.Text;

namespace YourApplication
{

    class Program
    {

        static int Main(string[] args)
        {
            using (var source = new CancellationTokenSource())
            {
                Console.CancelKeyPress += (sender, e) =>
                {
                    source.Cancel();
                };


                return MainAsync( source.Token)
                    .GetAwaiter()
                    .GetResult();
            }
        }

        private static async Task<int> MainAsync(CancellationToken cancellationToken)
        {
            ServiceProvider serviceProvider = null;
            TelemetryClient telemetryClient = null;
            try
            {
                serviceProvider = Bootstrapper.ConfigureServices()
                        .BuildServiceProvider();

                telemetryClient = serviceProvider.GetRequiredService<TelemetryClient>();


                var httpClient = new HttpClient();
                var logger = serviceProvider.GetService<ILogger>();
                var i = 0;
                while (i < 10) // This app runs indefinitely. replace with actual application termination logic.
                {
                    logger.Information("Worker running at: {time}", DateTimeOffset.Now);

                    // Replace with a name which makes sense for this operation.
                    using (telemetryClient.StartOperation<RequestTelemetry>("operation"))
                    {
                        logger.Warning("A sample warning message. By default, logs with severity Warning or higher is captured by Application Insights");
                        logger.Information("Calling bing.com");
                        var res = await httpClient.GetAsync("https://bing.com");
                        logger.Information("Calling bing completed with status:" + res.StatusCode);
                        logger.Fatal("Fatal action");
                        telemetryClient.TrackEvent("Bing call event completed");
                    }

                    await Task.Delay(1000);
                    i++;
                }
                return 0;
            }
            catch (Exception e)
            {
                ILogger logger = null;
                try
                {
                    if (serviceProvider != null)
                    {
                        logger = serviceProvider.GetService<ILogger>();
                    }
                }
                catch { }

                if (logger != null)
                {
                    logger.Error(e, "An error has occurred");
                }
                else
                {
                    Console.Error.WriteLine("An error has occurred" + e.ToString());
                }
                return -500;
            }
            finally
            {
                Log.CloseAndFlush();


                // This is to ensure that even if application terminates, telemetry is sent to the back-end.
                if (telemetryClient != null)
                {
                    telemetryClient.Flush();
                    await Task.Delay(5000);
                }

                if (serviceProvider != null)
                {
                    serviceProvider.Dispose();
                }
            }
        }
    }


    public static class Bootstrapper
    {
        private const string InstrumentationKeyFromConfig = "ApplicationInsights:InstrumentationKey";

        public static ServiceCollection ConfigureServices()
        {
            var config = BuildConfiguration();
            var services = new ServiceCollection();
            services.AddSingleton(provider => config);
            // https://docs.microsoft.com/en-us/azure/azure-monitor/app/worker-service
            services.AddApplicationInsightsTelemetryWorkerService(config[InstrumentationKeyFromConfig]);
            services.ConfigureLogging();
            return services;
        }

        private static IConfiguration BuildConfiguration() => new ConfigurationBuilder()
                .AddJsonFile("config/ai.json", false, true)
                .Build();

        private static void ConfigureLogging(this IServiceCollection services)
        {
            services.AddSingleton<Serilog.ILogger>(provider =>
            {
                var configuration = provider.GetRequiredService<IConfiguration>();
                var telemetryConfiguration = provider.GetRequiredService<TelemetryConfiguration>();

                var logger = new LoggerConfiguration()
                    .ReadFrom.Configuration(configuration)
                    .MinimumLevel.Override("Microsoft", LogEventLevel.Warning)
                    .MinimumLevel.Override("System", LogEventLevel.Warning)

                    .Enrich.FromLogContext()
                    .Enrich.WithMachineName()
                    .Enrich.WithExceptionDetails()
                    .WriteTo.Console()
                    .WriteTo.ApplicationInsights(telemetryConfiguration, TelemetryConverter.Traces)
                    .CreateLogger();

                Log.Logger = logger;
                return Log.Logger;
            });

            services.AddSingleton<ILoggerFactory>(provider =>
            {
                var logger = provider.GetRequiredService<Serilog.ILogger>();
                return new SerilogLoggerFactory(logger, true);
            });
        }
    }
}

@naishx
Copy link

naishx commented Nov 14, 2019

I have the the same problem i can't figure out, how set this up properly (for asp net core and console).

@xantari
Copy link

xantari commented Nov 27, 2019

Also wondering what the recommendation is here, doesn't seem like there are any good solutions to this issue yet.

@GeeWee
Copy link

GeeWee commented Dec 2, 2019

I'm wondering if there's some potential issues with configuring the Logger inside the "Configure" block or a hosted service.
I'm thinking that there's a possibility that:

  • Instances might be created before the static Logger is updated
  • And if they have an ILogger injected, I assume they're going to end up with the wrong one.

@batizar
Copy link

batizar commented Jan 16, 2020

Any updates on this? What's the recommendation to use this in Console apps?

@jbrodsky-code
Copy link

TelemetryConfiguration.CreateDefault() has been working for me:

builder.Services.AddLogging(builder => builder .AddSerilog( new LoggerConfiguration() .MinimumLevel.Debug() .WriteTo.Console() .WriteTo.ApplicationInsights(TelemetryConfiguration.CreateDefault(), TelemetryConverter.Traces) .CreateLogger(), true) );

Any updates on this? What's the recommendation to use this in Console apps?

@rodolphocastro
Copy link

Just created PR #128 in order to update the README so new users can go straight into TelemetryConfiguration.CreateDefault() instead of the deprecated one.

@triforcely
Copy link

Is there any real solution to this problem? Creating new telemetry configuration is no-go for me, because then traces are not corellated as well as when they used to with TelemetryConfiguration.Active.

@mslot
Copy link

mslot commented Feb 9, 2020

Is there any real solution to this problem? Creating new telemetry configuration is no-go for me, because then traces are not corellated as well as when they used to with TelemetryConfiguration.Active.

I also need a way to get the default processors, like adaptive sampling etc, when retrieving the TelemetryConfiguration. For now I need to use TelemetryConfiguration.Active until either a default TelemetryConfiguration is added to the IoC, or Microsoft describe more clearly how I should construct a good confiugration.

@henriksen
Copy link

Has a recommended way of doing this come out on top? I'm having a hard time weighing all these suggestions up against each other.

@jernejk
Copy link

jernejk commented Feb 26, 2020

Likewise.

I'm currently doing this:

.UseSerilog((hostingContext, loggerConfiguration) => {
    // Logging setup needs to be part of Azure Service Fabric setup atm.
    // If you work with hostingContext outside the ASF config class library, it will fail due hostingContext not being available. (even though you are literally passing it as a parameter)
    loggerConfiguration
        .ReadFrom.Configuration(hostingContext.Configuration)
        .Enrich.FromLogContext()
        .Enrich.WithProperty("ApplicationName", typeof(T).Assembly.GetName().Name)
        .Enrich.WithProperty("Environment", hostingContext.HostingEnvironment)
        // TODO: Wait for a solution to have full AppInsights support:
        // https://github.com/serilog/serilog-sinks-applicationinsights/issues/121
        .WriteTo.Async(x => x.ApplicationInsights(TelemetryConfiguration.CreateDefault(), TelemetryConverter.Traces));

#if DEBUG
    // Used to filter out potentially bad data due debugging.
    // Very useful when doing Seq dashboards and want to remove logs under debugging session.
    loggerConfiguration.Enrich.WithProperty("DebuggerAttached", System.Diagnostics.Debugger.IsAttached);
#endif

It works on most apps but when running it on Azure Service Fabric, it doesn't work.
I suspect that ASF creates its own instance of ApplicationInsights before the application is bootstrapped since it can kill and restart the ASP .NET core web app without killing the app. (the entry point is ASF rather than ASP .NET)

UPDATE: Using TelemetryConfiguration.Active seems to be the solution for now when using Azure Service Fabric.

@tibitoth
Copy link

tibitoth commented Mar 6, 2020

Hi, any update on this issue? I've faced the same problem here.

@James-Jackson-South
Copy link

James-Jackson-South commented Jul 16, 2020

Am I missing something or has the API changed again? 3.4.0-dev-00173 doesn't offer the service collection in the action

public static IWebHostBuilder UseSerilog(this IWebHostBuilder builder, Action<WebHostBuilderContext, LoggerConfiguration> configureLogger, bool preserveStaticLogger = false, bool writeToProviders = false)

Ah... This all works with ASP.NET Core 3+

@DalSoft
Copy link

DalSoft commented Jul 31, 2020

Any chance 3.4.0-dev-00173 could be published to nuget 'pre-release' please? Where I work allows pre-release packages but only from MS nuget not third parties.

@cmeeren
Copy link

cmeeren commented Jul 31, 2020

Any chance 3.4.0-dev-00173 could be published to nuget 'pre-release' please?

The service provider overload exists in 3.1.0 and works great. Note that it's on IHostBuilder and not IWebHostBuilder. See the readme for details.

@nblumhardt
Copy link
Contributor

Should we close this now, or are there doc tasks yet to pick up?

@cmeeren
Copy link

cmeeren commented Aug 1, 2020

@DalSoft @nblumhardt Sorry, I got the packages mixed up. My previous comment was about Serilog.AspNetCore 3.4.0. Not sure though what changes are needed/commited in this repo; with the service provider overload in Serilog.AspNetCore 3.4.0 I don't have the problem mentioned in this issue.

@goblinfactory
Copy link

Anyone using Serilog with AzureFunctions? No-one mentions azure functions in this whole thread, and the Serilog documentation for azurefunctions specifically says to use .Active. So... if we're using Azure functions and can't use any of the suggested work-arounds above, are we back to square one?

@johnknoop
Copy link

I've tried with the various suggestions that have come up in this thread, but I can't get it to work. Created a separate issue: #142

@dustinmoris
Copy link

Do I need to literally read this entire thread in order to work out how to setup ApplicationInsights with Serilog in ASP.NET Core 5 or can the README maybe get updated with a small section describing all the 20 million configuration steps that one has to go through in .NET Core in order to just get logging working?

@nblumhardt
Copy link
Contributor

Would be awesome if someone can bake this down into some README updates; thanks for the nudge @dustinmoris 👍

I don't use AppInsights and don't work on this sink, personally - happy to help where I can, though! Anyone keen to give this a shot?

@eluchsinger
Copy link

If we mark something as deprecated can we put a link to a solution or at least to this thread?

@nelsonwellswku
Copy link

It seems like this can work

private static void ConfigureLogger(HostBuilderContext context, IServiceProvider services, LoggerConfiguration loggerConfiguration) =>
loggerConfiguration
	.ReadFrom.Configuration(context.Configuration)
	.WriteTo.ApplicationInsights(services.GetRequiredService<TelemetryConfiguration>(), TelemetryConverter.Events);

But is there a way to configure the sink to use the IoC container provided TelemetryConfiguration while using the appSettings.json configuration? We currently use the appSettings so that configuring the sinks do not require a code change but we can't do that if we need to take this ☝️ programmatic approach.

@nblumhardt
Copy link
Contributor

@eluchsinger hi!

If we mark something as deprecated can we put a link to a solution or at least to this thread?

"We" would be the App Insights SDK team in this case - the deprecated API isn't one that Serilog controls.

@jernejk
Copy link

jernejk commented Jan 8, 2021

Do I need to literally read this entire thread in order to work out how to setup ApplicationInsights with Serilog in ASP.NET Core 5 or can the README maybe get updated with a small section describing all the 20 million configuration steps that one has to go through in .NET Core in order to just get logging working?

I was considering writing a blog post about this in the style I have written for Serilog + Seq on ASP.NET Core.
https://jkdev.me/asp-net-core-serilog/

I had a blog post ready over a year ago but it was hacky and I didn't felt like it was a good solution.
The metadata on Seq and AppInsights were inconsistent depending on how they were logged and I couldn't get them consistent with the issues we had a year ago. I think some of the logs no longer went to Serilog/Seq.

I can revisit this, see if I can make it work well and I'll post it here.

@cmeeren
Copy link

cmeeren commented Jan 8, 2021

In case it's helpful, here is what I'm doing (simplified):

open System
open Microsoft.ApplicationInsights.Extensibility
open Microsoft.AspNetCore.Hosting
open Microsoft.Extensions.DependencyInjection
open Microsoft.Extensions.Hosting
open Serilog
  
let initLogger (ctx: HostBuilderContext) (services: IServiceProvider) (loggerConfig: LoggerConfiguration) =
  loggerConfig.WriteTo.ApplicationInsights(
    services.GetRequiredService<TelemetryConfiguration>(),
    TelemetryConverter.Traces)
  |> ignore

[<EntryPoint>]
let main args =
  Host
    .CreateDefaultBuilder(args)
    .UseSerilog(initLogger)   // This is the relevant line here
    .ConfigureWebHostDefaults(..)
    .Build()
    .Run()
  0

@sander1095
Copy link

Hey everyone.

I am currently using the solution posted by @jbrezina (And others): #121 (comment).

Does anyone know if it is possible to have my app insights logging config in appsettings.json with the right telemtryconfiguration? Right now I have my console config in appsettings.json, but thanks to the issue mentioned above I don't have the app insights in my appsettings. If I could have both that would be great!

@accarin
Copy link

accarin commented Feb 27, 2021

Hey everyone.

I am currently using the solution posted by @jbrezina (And others): #121 (comment).

Does anyone know if it is possible to have my app insights logging config in appsettings.json with the right telemtryconfiguration? Right now I have my console config in appsettings.json, but thanks to the issue mentioned above I don't have the app insights in my appsettings. If I could have both that would be great!

Hi, it's a bit of shame the documentation is kind of unhelpful in this regard.

The following works for me as a workaround. Because the respective extension method in LoggerConfigurationApplicationInsightsExtensions.cs is called.

"Serilog": {
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "Microsoft": "Information",
        "Microsoft.Hosting.Lifetime": "Information",
        "Microsoft.EntityFrameworkCore": "Warning"
      }
    },
    "WriteTo": [
      {
        "Name": "ApplicationInsights",
        "Args": {
          "restrictedToMinimumLevel": "Information",
          "telemetryClient": {},
          "telemetryConverter": "Serilog.Sinks.ApplicationInsights.Sinks.ApplicationInsights.TelemetryConverters.TraceTelemetryConverter, Serilog.Sinks.ApplicationInsights"
        }
      }
    ]
  }

If an application instrumentation key is necessary or not available through other means, you will also need to set the env variable APPINSIGHTS_INSTRUMENTATIONKEY for this approach.

Or provide it manually through:

        "telemetryClient": {
          "configuration": {
            "instrumentationKey": "xxx"
          }
        }

@nblumhardt
Copy link
Contributor

We're still keen to update the README if someone familiar with AppInsights can PR some improvements :-)

@simbaja
Copy link

simbaja commented Mar 14, 2021

This is pretty hacky (sorry), but just wanted to post a possible solution to getting DI configured telemetry into the App Insights sink. Not sure if we need to actually get the TelemetryClient from DI or the TelemetryConfiguration - should be easy enough to change the code to do either. Basically, the code iterates through the configured sinks and uses reflection to find any instance of the App Insights sink. It then injects the client into the sink. When the configuration is built, the logger has the DI configuration instead of whatever defaults. This is especially useful because if you use the Microsoft.ApplicationInsights.WorkerService package, it doesn't set the Active configuration, which causes the the sink to use a basically unconfigured default value.

This should allow you to fully configure App Insights/Serilog in appSettings.json separately and per recommendations for both products and make them work together while not relying on the TelemetryConfiguration.Active singleton.

        public static LoggerConfiguration WithTelemetryConfiguration(this LoggerConfiguration loggerConfiguration, IServiceProvider services)
        {
            //get the DI configuration
            TelemetryConfiguration telemetryConfiguration = services.GetRequiredService<TelemetryConfiguration>();
            TelemetryClient telemetryClient = new TelemetryClient(telemetryConfiguration);

            Log.Information("Attempting to inject AI telemetryConfiguration");
            Log.Information("AI Instrumentation Key Present: {keyExists}", !String.IsNullOrEmpty(telemetryConfiguration.InstrumentationKey));
            Log.Information("AI Endpoint: {endpoint}", telemetryConfiguration.TelemetryChannel.EndpointAddress);

            var sinksInfo = loggerConfiguration.GetType().GetField("_logEventSinks", BindingFlags.Instance | BindingFlags.NonPublic);
            if(sinksInfo != null)
            {
                var sinks = (List<Serilog.Core.ILogEventSink>)sinksInfo.GetValue(loggerConfiguration);
                foreach(Serilog.Core.ILogEventSink sink in sinks)
                {
                    var possibleSink = sink;

                    var wSinkInfo = sink.GetType().GetField("_sink", BindingFlags.Instance | BindingFlags.NonPublic);
                    if (wSinkInfo != null)
                    {
                        var wSink = wSinkInfo.GetValue(sink) as Serilog.Core.ILogEventSink;
                        if (wSink != null)
                            possibleSink = wSink;
                    }

                    if (possibleSink is Serilog.Sinks.ApplicationInsights.ApplicationInsightsSink)
                    {
                        possibleSink.GetType()
                                .GetField("_telemetryClient", BindingFlags.Instance | BindingFlags.NonPublic)
                                .SetValue(possibleSink, telemetryClient);

                        Log.Information("Injected telemetry client into sink!");
                    }
                }
            }

            return loggerConfiguration;
        }

Just put this into a static class, and then call it at the end of your logger configuration. Make sure to call the appropriate App Insights telemetry method in your services code so that App Insights is properly initialized by time the logger is created. This code isn't extensively tested, and won't handle all wrappers/sink aggregates, but I think it gets many of the main wrappers that I saw in the core source.

Though Serilog itself isn't built with introspection in mind, it may make sense to modify the configuration extensions to allow introspection, it would be pretty powerful for making things all work together. Alternatively, it would be nice for the Serilog App Insights Sink to be able to grab the TelemetryConfiguration/Client from the service provider, not sure of the order of operations that would be needed to make that happen though.

@cmeeren
Copy link

cmeeren commented Mar 14, 2021

For ASP.NET Core, I think the problem is solved by the new version of Serilog.AspNetCore: https://nblumhardt.com/2020/10/bootstrap-logger

This allows configuring a bootstrap logger to capture startup errors, and then replace this logger with DI-dependent config later on.

@simbaja
Copy link

simbaja commented Mar 14, 2021

For ASP.NET Core, I think the problem is solved by the new version of Serilog.AspNetCore: https://nblumhardt.com/2020/10/bootstrap-logger

This allows configuring a bootstrap logger to capture startup errors, and then replace this logger with DI-dependent config later on.

Unfortunately, the new bootstrapper doesn't solve this particular issue. The only reason it works (for now) is because deep in the bowels of the MSFT App Insights package for ASP.NET, they are actually initializing TelemetryConfig.Active. Other versions of the MSFT App Insights packages (for example WorkerService), don't initialize TelemetryConfig.Active. The current sink design either uses TelemetyConfig.Active or you have to pass in the config through code. The former case won't always work, the latter forces you to define the sink in code. Either way, it causes problems.

I'm actually using the bootstrapper in my code, and while it's good for its purpose of capturing logs earlier, it doesn't resolve this issue.

@veikkoeeva
Copy link

veikkoeeva commented Mar 15, 2021

Getting to the end of this long thread, oh man! Has someone succeeded in using Serilog + AppInsight as sink in WebJob?

I wonder since .ConfigureWebJobs & AddApplicationInsightsWebJobs is not compatible with the ordinary AppInsights addition and for a reason or another I fail to add the configuration so it both works and has the instrumentation key provided. I may be missing something simple, though...

@simbaja
Copy link

simbaja commented Mar 15, 2021

Getting to the end of this long thread, oh man! Has someone succeeded in using Serilog + AppInsight as sink in WebJob?

I wonder since .ConfigureWebJobs & AddApplicationInsightsWebJobs is not compatible with the ordinary AppInsights addition and for a reason or another I fail to add the configuration so it both works and has the instrumentation key provided. I may be missing something simple, though...

I haven't tried, but it looks like ultimately WebJobs sets up the TelemetryConfiguration.Active singleton (see https://github.com/Azure/azure-webjobs-sdk/blob/6799c9f04c0244c9d0ce51fe2386bd7322a20a5e/src/Microsoft.Azure.WebJobs.Logging.ApplicationInsights/Extensions/ApplicationInsightsServiceCollectionExtensions.cs#L189). So, in theory, if you're using the bootstrapper, it should work since the ApplicationInsightsSink will take the configuration from the active configuration if nothing else is provided, and this should be configured by time you're initializing the logger in the UseSerilog extension. In this case, the bootstrapper may be the solution to your issue. Might want to debug after your ReadFrom.Configuration call and look at whether the Sink has the configuration you expect.

If they ever stop initializing TelemetryConfig.Active, we run into the same problem though.

@veikkoeeva
Copy link

veikkoeeva commented Mar 15, 2021

@simbaja Do you mean with the UseSerilog? Heh, I feel a bit thick. It also may be this is too fragile to production. In any event, I record here I tried the following in vain, but the solution presented here in this thread earlier for WebAPI works.

The one that follows next logs to a console locally, but in Azure the complaint is AI: TelemetryChannel found a telemetry item without an InstrumentationKey. This is a required field and must be set in either your config file or at application startup.. Puzzling to say the least and I found also microsoft/ApplicationInsights-dotnet#2070 related to this error message.

using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Serilog;
using System;

namespace LoggingText.WebJob
{
   public class Functions
    {
        public async Task SomeTimerTriggerOperation([TimerTrigger("0 */1 * * * *", RunOnStartup = true)] TimerInfo timerInfo, CancellationToken cancellationToken, ILogger log)
        {
            await Task.Delay(TimeSpan.FromMilliseconds(100), cancellationToken).ConfigureAwait(false);
            log.LogWarning("Ohoy!");
        }
    }

    class Program
    {
        private const string AppInsightKey = "InstrumentationKey=<the rest removed>";
        public static IHostBuilder CreateHostBuilder(string[] args)
        {
            return Host.CreateDefaultBuilder(args)
                .ConfigureLogging((hostContext, logBuilder) =>
                {
                    logBuilder.AddApplicationInsightsWebJobs(o => o.InstrumentationKey = AppInsightKey);
                })
                .ConfigureAppConfiguration((context, config) => config.AddJsonFile("appsettings.Development.json"))
                .ConfigureWebJobs(webJobsBuilder =>
                {
                    webJobsBuilder
                        .AddTimers()
                        .AddAzureStorage()
                        .AddAzureStorageCoreServices();
                })
                .ConfigureServices((ctx, services) =>
                {
                    services.AddApplicationInsightsTelemetry(AppInsightKey);
                })
                .UseSerilog((HostBuilderContext context, IServiceProvider services, LoggerConfiguration loggerConfiguration) =>
                    loggerConfiguration
                    .ReadFrom
                    .Configuration(context.Configuration)
                    .WriteTo
                    .ApplicationInsights(
                        services.GetRequiredService<TelemetryConfiguration>(), TelemetryConverter.Traces),
                        preserveStaticLogger: false, writeToProviders: false);
        }

        public static void Main(string[] args)
        {
            var host = CreateHostBuilder(args).Build();
            host.Run();
        }
    }
}

@simbaja
Copy link

simbaja commented Mar 16, 2021

@veikkoeeva, Looking through the source code, and if I'm reading it correctly, it looks like logBuilder.AddApplicationInsightsWebJobs internally calls the services.AddApplicationInsights: https://github.com/Azure/azure-webjobs-sdk/blob/6930c4ea9ec78db12a05a2d0bf38cce42ab04e13/src/Microsoft.Azure.WebJobs.Logging.ApplicationInsights/Extensions/ApplicationInsightsServiceCollectionExtensions.cs#L32.

However, if you're using Serilog, you probably don't need the logBuilder.AddApplicationInsightsWebJobs call, as that's setting up the MS loggers from what I see (and possibly adding duplicate services). Your call to services.AddApplicationInsightsTelemetry(AppInsightKey) should set it up for Serilog. If it doesn't perhaps change it to services.AddApplicationInsights(o => o.InstrumentationKey = "key") based on the call here: https://github.com/Azure/azure-webjobs-sdk/blob/6930c4ea9ec78db12a05a2d0bf38cce42ab04e13/src/Microsoft.Azure.WebJobs.Logging.ApplicationInsights/Extensions/ApplicationInsightsServiceCollectionExtensions.cs#L30 (which is what is called internally by WebJobs).

Not sure if this helps. It's a bit of a mess at the moment, too many extensions doing different things...

@nblumhardt
Copy link
Contributor

Hi all,

Thanks for all your input on this thread 😃

As @cmeeren points out the original problems with TelemetryConfiguration.Active appears to be mitigated by recent changes to Serilog.AspNetCore.

It's difficult, because of the length of the thread, to tell what the state of any related peripheral issue is.

I'm closing and locking the thread to avoid losing valuable information in the clutter. Things you can do:

  • If you have a question on how to set up Serilog with Application Insights, please open a question on Stack Overflow
  • If you have a working configuration you'd like to share, please send a pull request to this repo with updates to README.md
  • If you think there's a scenario still not supported, or a bug, please raise an issue in this repo with a full description so that it can be addressed accurately

Thanks again!

@serilog-contrib serilog-contrib locked as resolved and limited conversation to collaborators Mar 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests