Finbuckle / Finbuckle.MultiTenant

Finbuckle.MultiTenant is an open-source multitenancy middleware library for .NET. It enables tenant resolution, per-tenant app behavior, and per-tenant data isolation.
https://www.finbuckle.com/multitenant
Apache License 2.0
1.32k stars 267 forks source link

Application Insight - ITelemetryInitializer - Exception #371

Closed bedeche1 closed 3 years ago

bedeche1 commented 3 years ago

I am trying to get the MultiTenantContext from the IHttpContextAccessor during an ITelemetryInitializer.Initialize of ApplicationInsight to add the tenant properties (name and id) but I still get an exception of type ArgumentNullException during the GetMultiTenantContext.

Is there a way to get this information at this time?

Step to reproduce the problem: Includes Nuget to your WebApi

Route on controller

[Route("{tenant=}/[controller]")]

public class TenantInitializer : ITelemetryInitializer
    {
        private readonly IHttpContextAccessor _httpContextAccessor;

        public IntelliumAppInsightsInitializer(IHttpContextAccessor httpContextAccessor)
        {
            _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException("httpContextAccessor");
        }

        public void Initialize(ITelemetry telemetry)
        {

            //the following line raises an exception
            var tenantInfo = _httpContextAccessor.HttpContext
                                                                                                                                                                                                             ?.GetMultiTenantContext<TenantInfo>()
                                                                                                                                                                                                             ?.TenantInfo;
                                                               if (tenantInfo != null)
                                                               {
                                                                               telemetry.Context.GlobalProperties.TryAdd("Tenant Id", tenantInfo.Id);
                                                                               telemetry.Context.GlobalProperties.TryAdd("Tenant Name", tenantInfo.Name);
                                                               }
        }
    }
            Startup.cs
namespace WebApplication1
{
    public class Startup
    {
        public Startup(IConfiguration configuration)
        {
            Configuration = configuration;
        }

        public IConfiguration Configuration { get; }

        // This method gets called by the runtime. Use this method to add services to the container.
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddApplicationInsightsTelemetry();
            services.AddSingleton<ITelemetryInitializer, AppInsightsInitializer>();
            services.AddControllers();

            services.AddMultiTenant<TenantInfo>()
                .WithConfigurationStore()
                .WithRouteStrategy();
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
        public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
        {
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }

            app.UseHttpsRedirection();

            app.UseRouting();
            app.UseMultiTenant();

            app.UseAuthorization();

            app.UseEndpoints(endpoints =>
            {
                endpoints.MapControllers();
                endpoints.MapControllerRoute("default", "{__tenant__=}/{controller=Home}/{action=Index}");
            });
        }
    }
}
AndrewTriesToCode commented 3 years ago

@bedeche1 I haven't seen this before, but something that jumps out is services.AddSingleton<ITelemetryInitializer, AppInsightsInitializer>(); which means it gets initialized once and typically tenant changes per request. Would AddScoped work for that service or is that not how it works with AppInsights?

bedeche1 commented 3 years ago

I've tried this before to put AddScoped instead of AddSingleton but ApplicationInsight requires it to be a services.AddSingleton <ITelemetryInitializer, AppInsightsInitializer> (). I did not specify I am using AspNetCore 3.1

natelaff commented 3 years ago

Yes, I ran into a similar thing a few weeks ago. Luckily, I have the tenant name inside of a claim as well so was able to pull from that and it works well. I didn't see any other way around it with it being Singleton.

AndrewTriesToCode commented 3 years ago

Can you try moving the

services.AddApplicationInsightsTelemetry();
            services.AddSingleton<ITelemetryInitializer, AppInsightsInitializer>();

after Add MultiTenant? It's possible that initialize is called there and the services for registered yet that Finbuckle uses. I'll try to recreate in a project and dig into the error further.

natelaff commented 3 years ago

I just tried again so you could see the stack. Honestly pulling it out of the claim worked better for me anyways so I didn't dig too deep and moved on. That telemetryinitializer is the last thing in my ConfigureServices

So it's tanking on: https://github.com/Finbuckle/Finbuckle.MultiTenant/blob/612889bdddce4ab9143bc2f0988145fce12afa35/src/Finbuckle.MultiTenant.AspNetCore/Extensions/HttpContextExtensions.cs#L31

at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider) at Finbuckle.MultiTenant.FinbuckleHttpContextExtensions.GetMultiTenantContext[T](HttpContext httpContext) at CloudApp.Api.Filters.HttpContextRequestTelemetryInitializer.Initialize(ITelemetry telemetry) in HttpContextRequestTelemetryInitializer.cs:line 33 at Microsoft.ApplicationInsights.TelemetryClient.Initialize(ITelemetry telemetry)

bedeche1 commented 3 years ago

Thanks for the suggestions, I will abandon the use of _httpContextAccessor.HttpContext? .GetMultiTenantContext <TenantInfo> () ?. TenantInfo; for now and I'll instantiate and directly use the basepathstrategy to get the idetifier. the important thing is to have at least one piece of information that allows me to identify my tenant.

AndrewTriesToCode commented 3 years ago

I’m going to still look a little deeper into this. I’ll let you know what I find out.

On Dec 11, 2020, at 4:21 AM, bedeche1 notifications@github.com wrote:

 Thanks for the suggestions, I will abandon the use of _httpContextAccessor.HttpContext? .GetMultiTenantContext () ?. TenantInfo; for now and I'll instantiate and directly use the basepathstrategy to get the idetifier. the important thing is to have at least one piece of information that allows me to identify my tenant.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

Nisden commented 3 years ago

@bedeche1 If you still wanna use Application Insights, you could do the following.

app.Use(async (context, next) =>
{
    var multiTenantContextAccessor = context.RequestServices.GetRequiredService<IMultiTenantContextAccessor>();
    var requestTelemetry = context.Features.Get<Microsoft.ApplicationInsights.DataContracts.RequestTelemetry>();

    // Application insights
    requestTelemetry.Context.User.AccountId = multiTenantContextAccessor.MultiTenantContext?.TenantInfo?.Id ?? null;
    requestTelemetry.Context.User.AuthenticatedUserId = context.User.Identity.IsAuthenticated ? context.User.GetUserId().ToString() : null;
    requestTelemetry.Properties.TryAdd("TenantUrl", multiTenantContextAccessor.MultiTenantContext?.TenantInfo?.Identifier);

    await next.Invoke();
});

Just make sure its called after app.UseMultiTenant();

AndrewTriesToCode commented 3 years ago

I haven't found a better approach than what @Nisden shows above. App Insights seems to hide a lot of its setup behind the scenes so its hard to say. I'm going to close this out for now. Thanks!