Tests for telemetry enriching middleware, make ASP.NET Core spans roo…

…t, misc improvements (#659)

AppLibDotnet.sln

@@ -18,6 +18,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{7AD5FADE-607
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.App.Core", "src\Altinn.App.Core\Altinn.App.Core.csproj", "{1745B251-BD5C-43B7-BA7D-9C4BFAB37535}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Altinn.App.Common.Tests", "test\Altinn.App.Common.Tests\Altinn.App.Common.Tests.csproj", "{D5838692-2703-4E42-8802-6E1FA7F1B50B}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -40,6 +42,10 @@ Global
 		{1745B251-BD5C-43B7-BA7D-9C4BFAB37535}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{1745B251-BD5C-43B7-BA7D-9C4BFAB37535}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{1745B251-BD5C-43B7-BA7D-9C4BFAB37535}.Release|Any CPU.Build.0 = Release|Any CPU
+		{D5838692-2703-4E42-8802-6E1FA7F1B50B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{D5838692-2703-4E42-8802-6E1FA7F1B50B}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{D5838692-2703-4E42-8802-6E1FA7F1B50B}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{D5838692-2703-4E42-8802-6E1FA7F1B50B}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -49,6 +55,7 @@ Global
 		{17D7DCE9-7797-4BC1-B448-D0529FD6FB3D} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
 		{2FD56505-1DB2-4AE1-8911-E076E535EAC6} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
 		{1745B251-BD5C-43B7-BA7D-9C4BFAB37535} = {7AD5FADE-607F-4D5F-8511-6647D0C1AA1C}
+		{D5838692-2703-4E42-8802-6E1FA7F1B50B} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {4584C6E1-D5B4-40B1-A8C4-CF4620EB0896}

src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs

@@ -1,3 +1,4 @@
+using System.Diagnostics;
 using Altinn.App.Api.Controllers;
 using Altinn.App.Api.Helpers;
 using Altinn.App.Api.Infrastructure.Filters;
@@ -15,6 +16,8 @@
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.FeatureManagement;
 using Microsoft.IdentityModel.Tokens;
+using OpenTelemetry;
+using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Logs;
 using OpenTelemetry.Metrics;
 using OpenTelemetry.Resources;
@@ -148,6 +151,14 @@ IWebHostEnvironment env
             services.AddHostedService<TelemetryInitialization>();
             services.AddSingleton<Telemetry>();
 
+            // This bit of code makes ASP.NET Core spans always root.
+            // Depending on infrastructure used and how the application is exposed/called,
+            // it might be a good idea to be in control of the root span (and therefore the size, baggage etch)
+            // Taken from: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1773
+            _ = Sdk.SuppressInstrumentation; // Just to trigger static constructor. The static constructor in Sdk initializes Propagators.DefaultTextMapPropagator which we depend on below
+            Sdk.SetDefaultTextMapPropagator(new OtelPropagator(Propagators.DefaultTextMapPropagator));
+            DistributedContextPropagator.Current = new AspNetCorePropagator();
+
             var appInsightsConnectionString = GetAppInsightsConnectionStringForOtel(config, env);
 
             services
@@ -232,7 +243,7 @@ private sealed class TelemetryInitialization(
             MeterProvider meterProvider
         ) : IHostedService
         {
-            public Task StartAsync(CancellationToken cancellationToken)
+            public async Task StartAsync(CancellationToken cancellationToken)
             {
                 // This codepath for initialization is here only because it makes it a lot easier to
                 // query the metrics from Prometheus using 'increase' without the appearance of a "missed" sample.
@@ -244,21 +255,100 @@ public Task StartAsync(CancellationToken cancellationToken)
                 telemetry.Init();
                 try
                 {
-                    if (!meterProvider.ForceFlush(10_000))
+                    var task = Task.Factory.StartNew(
+                        () =>
+                        {
+                            if (!meterProvider.ForceFlush(10_000))
+                            {
+                                logger.LogInformation("Failed to flush metrics after 10 seconds");
+                            }
+                        },
+                        cancellationToken,
+                        // Long running to avoid doing this blocking on a "normal" thread pool thread
+                        TaskCreationOptions.LongRunning | TaskCreationOptions.DenyChildAttach,
+                        TaskScheduler.Default
+                    );
+                    if (await Task.WhenAny(task, Task.Delay(500, cancellationToken)) != task)
                     {
-                        logger.LogWarning("Failed to flush metrics after 10 seconds");
+                        logger.LogInformation(
+                            "Tried to flush metrics within 0.5 seconds but it was taking too long, proceeding with startup"
+                        );
                     }
                 }
                 catch (Exception ex)
                 {
+                    if (ex is OperationCanceledException)
+                        return;
                     logger.LogWarning(ex, "Failed to flush metrics");
                 }
-                return Task.CompletedTask;
             }
 
             public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
         }
 
+        internal sealed class OtelPropagator : TextMapPropagator
+        {
+            private readonly TextMapPropagator _inner;
+
+            public OtelPropagator(TextMapPropagator inner) => _inner = inner;
+
+            public override ISet<string> Fields => _inner.Fields;
+
+            public override PropagationContext Extract<T>(
+                PropagationContext context,
+                T carrier,
+                Func<T, string, IEnumerable<string>> getter
+            )
+            {
+                if (carrier is HttpRequest)
+                    return default;
+                return _inner.Extract(context, carrier, getter);
+            }
+
+            public override void Inject<T>(PropagationContext context, T carrier, Action<T, string, string> setter) =>
+                _inner.Inject(context, carrier, setter);
+        }
+
+        internal sealed class AspNetCorePropagator : DistributedContextPropagator
+        {
+            private readonly DistributedContextPropagator _inner;
+
+            public AspNetCorePropagator() => _inner = CreateDefaultPropagator();
+
+            public override IReadOnlyCollection<string> Fields => _inner.Fields;
+
+            public override IEnumerable<KeyValuePair<string, string?>>? ExtractBaggage(
+                object? carrier,
+                PropagatorGetterCallback? getter
+            )
+            {
+                if (carrier is IHeaderDictionary)
+                    return null;
+
+                return _inner.ExtractBaggage(carrier, getter);
+            }
+
+            public override void ExtractTraceIdAndState(
+                object? carrier,
+                PropagatorGetterCallback? getter,
+                out string? traceId,
+                out string? traceState
+            )
+            {
+                if (carrier is IHeaderDictionary)
+                {
+                    traceId = null;
+                    traceState = null;
+                    return;
+                }
+
+                _inner.ExtractTraceIdAndState(carrier, getter, out traceId, out traceState);
+            }
+
+            public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) =>
+                _inner.Inject(activity, carrier, setter);
+        }
+
         private static void AddAuthorizationPolicies(IServiceCollection services)
         {
             services.AddAuthorization(options =>

src/Altinn.App.Api/Infrastructure/Middleware/TelemetryEnrichingMiddleware.cs

@@ -7,10 +7,7 @@
 
 namespace Altinn.App.Api.Infrastructure.Middleware;
 
-/// <summary>
-/// Middleware for adding telemetry to the request.
-/// </summary>
-public class TelemetryEnrichingMiddleware
+internal sealed class TelemetryEnrichingMiddleware
 {
     private readonly RequestDelegate _next;
     private readonly ILogger<TelemetryEnrichingMiddleware> _logger;
@@ -41,6 +38,10 @@ static TelemetryEnrichingMiddleware()
                     }
                 }
             },
+            {
+                AltinnCoreClaimTypes.AuthenticateMethod,
+                static (claim, activity) => activity.SetAuthenticationMethod(claim.Value)
+            },
             {
                 AltinnCoreClaimTypes.AuthenticationLevel,
                 static (claim, activity) =>
@@ -50,7 +51,9 @@ static TelemetryEnrichingMiddleware()
                         activity.SetAuthenticationLevel(result);
                     }
                 }
-            }
+            },
+            { AltinnCoreClaimTypes.Org, static (claim, activity) => activity.SetOrganisationName(claim.Value) },
+            { AltinnCoreClaimTypes.OrgNumber, static (claim, activity) => activity.SetOrganisationNumber(claim.Value) },
         };
 
         ClaimActions = actions.ToFrozenDictionary();
@@ -73,17 +76,15 @@ public TelemetryEnrichingMiddleware(RequestDelegate next, ILogger<TelemetryEnric
     /// <param name="context">The HTTP context.</param>
     public async Task InvokeAsync(HttpContext context)
     {
-        var trace = context.Features.Get<IHttpActivityFeature>();
-        if (trace is null)
+        var activity = context.Features.Get<IHttpActivityFeature>()?.Activity;
+        if (activity is null)
         {
             await _next(context);
             return;
         }
 
         try
         {
-            var activity = trace.Activity;
-
             foreach (var claim in context.User.Claims)
             {
                 if (ClaimActions.TryGetValue(claim.Type, out var action))

src/Altinn.App.Core/Configuration/AppSettings.cs

@@ -215,5 +215,11 @@ public class AppSettings
         /// Enable the functionality to run expression validation in backend
         /// </summary>
         public bool ExpressionValidation { get; set; } = false;
+
+        /// <summary>
+        /// Enables OpenTelemetry as a substitute for Application Insights SDK
+        /// Improves instrumentation throughout the Altinn app libraries.
+        /// </summary>
+        public bool UseOpenTelemetry { get; set; }
     }
 }

src/Altinn.App.Core/Features/Telemetry.cs

@@ -117,7 +117,7 @@ public static class Labels
         /// <summary>
         /// Label for the party ID of the instance owner.
         /// </summary>
-        public static readonly string InstanceOwnerPartyId = "instance.owner_party_id";
+        public static readonly string InstanceOwnerPartyId = "instance.owner.party.id";
 
         /// <summary>
         /// Label for the guid that identifies the instance.
@@ -147,12 +147,22 @@ public static class Labels
         /// <summary>
         /// Label for the ID of the party.
         /// </summary>
-        public const string UserPartyId = "user.party_id";
+        public const string UserPartyId = "user.party.id";
+
+        /// <summary>
+        /// Label for the authentication method of the user.
+        /// </summary>
+        public const string UserAuthenticationMethod = "user.authentication.method";
 
         /// <summary>
         /// Label for the authentication level of the user.
         /// </summary>
-        public const string UserAuthenticationLevel = "user.authentication_level";
+        public const string UserAuthenticationLevel = "user.authentication.level";
+
+        /// <summary>
+        /// Label for the organisation name.
+        /// </summary>
+        public const string OrganisationName = "organisation.name";
 
         /// <summary>
         /// Label for the organisation number.
@@ -166,7 +176,7 @@ internal static class InternalLabels
         internal const string Type = "type";
         internal const string AuthorizationAction = "authorization.action";
         internal const string AuthorizerAction = "authorization.authorizer.action";
-        internal const string AuthorizerTaskId = "authorization.authorizer.task_id";
+        internal const string AuthorizerTaskId = "authorization.authorizer.task.id";
         internal const string ValidatorType = "validator.type";
         internal const string ValidatorSource = "validator.source";
     }

src/Altinn.App.Core/Features/TelemetryActivityExtensions.cs

@@ -55,6 +55,21 @@ public static Activity SetUsername(this Activity activity, string? username)
         return activity;
     }
 
+    /// <summary>
+    /// Sets the user authentication method as a tag/attribute on the activity/span
+    /// </summary>
+    /// <param name="activity">Activity</param>
+    /// <param name="authenticationMethod">Authentication method</param>
+    /// <returns>Activity</returns>
+    public static Activity SetAuthenticationMethod(this Activity activity, string? authenticationMethod)
+    {
+        if (!string.IsNullOrWhiteSpace(authenticationMethod))
+        {
+            activity.SetTag(Labels.UserAuthenticationMethod, authenticationMethod);
+        }
+        return activity;
+    }
+
     /// <summary>
     /// Sets the user authentication level as a tag/attribute on the activity/span
     /// </summary>
@@ -217,7 +232,23 @@ public static Activity SetTaskId(this Activity activity, string? taskId)
     }
 
     /// <summary>
-    /// Sets the Process Task ID as a tag/attribute on the activity/span
+    /// Sets the Organisation name as a tag/attribute on the activity/span
+    /// </summary>
+    /// <param name="activity">Activity</param>
+    /// <param name="organisationName">Organisation name</param>
+    /// <returns>Activity</returns>
+    public static Activity SetOrganisationName(this Activity activity, string? organisationName)
+    {
+        if (!string.IsNullOrWhiteSpace(organisationName))
+        {
+            activity.SetTag(Labels.OrganisationName, organisationName);
+        }
+
+        return activity;
+    }
+
+    /// <summary>
+    /// Sets the Organisation number as a tag/attribute on the activity/span
     /// </summary>
     /// <param name="activity">Activity</param>
     /// <param name="organisationNumber">Organisation number</param>

test/Altinn.App.Api.Tests/Altinn.App.Api.Tests.csproj

@@ -28,6 +28,7 @@
 
 	<ItemGroup>
 		<ProjectReference Include="..\..\src\Altinn.App.Api\Altinn.App.Api.csproj" />
+		<ProjectReference Include="..\Altinn.App.Common.Tests\Altinn.App.Common.Tests.csproj" />
 	</ItemGroup>
 
 	<ItemGroup>