feat: add extension to rm microsoft appinsights logprovider (#374)

* feat: add extension to rm microsoft appinsights logprovider * pr-fix: update w logging * pr-fix: update w docker logs * pr-fix: add appinsights instrumentation key to docker * Update docs/preview/03-Guidance/use-with-dotnet-and-functions.md Co-authored-by: Frederik Gheysels <frederik.gheysels@telenet.be> * Update src/Arcus.Observability.Tests.Integration/Serilog/Sinks/ApplicationInsights/AzureFunctionsDockerTests.cs Co-authored-by: Frederik Gheysels <frederik.gheysels@telenet.be> * Update docs/preview/03-Guidance/use-with-dotnet-and-functions.md Co-authored-by: Frederik Gheysels <frederik.gheysels@telenet.be> Co-authored-by: Frederik Gheysels <frederik.gheysels@telenet.be>
arcus-azure · Jun 1, 2022 · 63929c5 · 63929c5
1 parent 96cb7eb
commit 63929c5
Show file tree

Hide file tree

Showing 17 changed files with 661 additions and 8 deletions.
diff --git a/build/ci-build.yml b/build/ci-build.yml
@@ -31,6 +31,7 @@ variables:
   - group: 'Arcus - GitHub Package Registry'
   - group: 'Build Configuration'
   - template: ./variables/build.yml
+  - template: ./variables/test.yml
 
 stages:
   - stage: Build
@@ -113,11 +114,37 @@ stages:
               projectName: '$(Project).Tests.Integration'
               category: 'Integration'
 
+  - stage: DockerTests
+    displayName: Docker Tests
+    dependsOn: Build
+    condition: succeeded()
+    jobs:
+      - job: DockerTests
+        displayName: 'Run Docker tests'
+        pool:
+          vmImage: '$(Vm.Image)'
+        steps:
+          - task: DownloadPipelineArtifact@2
+            displayName: 'Download build artifacts'
+            inputs:
+              artifact: 'Build'
+              path: '$(Build.SourcesDirectory)'
+          - task: UseDotNet@2
+            displayName: 'Import .NET Core SDK ($(DotNet.Sdk.PreviousVersion))'
+            inputs:
+              packageType: 'sdk'
+              version: '$(DotNet.Sdk.PreviousVersion)'
+          - template: templates/run-docker-integration-tests.yml
+            parameters:
+              dockerProjectName: '$(Project).Tests.Runtimes.AzureFunction'
+              httpPort: '$(AzureFunctions.HttpPort)'
+
   - stage: ReleaseToMyget
     displayName: 'Release to MyGet'
     dependsOn:
       - UnitTests
       - IntegrationTests
+      - DockerTests
     condition: succeeded()
     jobs:
       - job: PushToMyGet

diff --git a/build/nuget-release.yml b/build/nuget-release.yml
@@ -18,6 +18,7 @@ variables:
   - group: 'Arcus Observability - Integration Testing'
   - group: 'Build Configuration'
   - template: ./variables/build.yml
+  - template: ./variables/test.yml
   - name: 'Repository'
     value: 'arcus-azure/arcus.observability'
   - name: 'Package.Version'
@@ -100,9 +101,37 @@ stages:
               dotnetSdkVersion: '$(DotNet.Sdk.Version)'
               projectName: '$(Project).Tests.Integration'
 
+  - stage: DockerTests
+    displayName: Docker Tests
+    dependsOn: Build
+    condition: succeeded()
+    jobs:
+      - job: DockerTests
+        displayName: 'Run Docker tests'
+        pool:
+          vmImage: '$(Vm.Image)'
+        steps:
+          - task: DownloadPipelineArtifact@2
+            displayName: 'Download build artifacts'
+            inputs:
+              artifact: 'Build'
+              path: '$(Build.SourcesDirectory)'
+          - task: UseDotNet@2
+            displayName: 'Import .NET Core SDK ($(DotNet.Sdk.PreviousVersion))'
+            inputs:
+              packageType: 'sdk'
+              version: '$(DotNet.Sdk.PreviousVersion)'
+          - template: templates/run-docker-project.yml
+            parameters:
+              dockerProjectName: '$(Project).Tests.Runtimes.AzureFunction'
+              httpPort: '$(AzureFunctions.HttpPort)'
+
   - stage: Release
     displayName: 'Release to NuGet.org'
-    dependsOn: IntegrationTests
+    dependsOn:
+      - UnitTests
+      - IntegrationTests
+      - DockerTests
     condition: succeeded()
     jobs:
       - job: PushToNuGet

diff --git a/build/templates/run-docker-integration-tests.yml b/build/templates/run-docker-integration-tests.yml
@@ -0,0 +1,51 @@
+parameters:
+  dockerProjectName: ''
+  httpPort: ''
+
+steps:
+  - bash: |
+      if [ -z "$PROJECT_NAME" ]; then
+        echo "##vso[task.logissue type=error;]Missing template parameter \"dockerProjectName\""
+        echo "##vso[task.complete result=Failed;]"
+      fi
+       if [ -z "$HTTP_PORT" ]; then
+        echo "##vso[task.logissue type=error;]Missing template parameter \"httpPort\""
+        echo "##vso[task.complete result=Failed;]"
+      fi
+    env:
+      PROJECT_NAME: ${{ parameters.dockerProjectName }}
+      HTTP_PORT: ${{ parameters.httpPort }}
+  - task: UseDotNet@2
+    displayName: 'Import .NET Core SDK ($(DotNet.Sdk.Version))'
+    inputs:
+      packageType: 'sdk'
+      version: '$(DotNet.Sdk.Version)'
+  - task: Docker@1
+    displayName: 'Build Docker image from ${{ parameters.dockerProjectName }}'
+    inputs:
+      dockerFile: src/${{ parameters.dockerProjectName }}/Dockerfile
+      imageName: '${{ parameters.dockerProjectName }}:$(Build.BuildId)'
+      useDefaultContext: false
+      buildContext: src
+  - task: Docker@1
+    displayName: 'Run new  project Docker image from ${{ parameters.dockerProjectName }}'
+    inputs:
+      command: 'Run an image'
+      imageName: '${{ parameters.dockerProjectName }}:$(Build.BuildId)'
+      containerName: '${{ parameters.dockerProjectName }}'
+      ports: '${{ parameters.httpPort }}:80'
+      envVars: |
+        APPINSIGHTS_INSTRUMENTATIONKEY=$(ApplicationInsights.InstrumentationKey)
+  - template: test/run-integration-tests.yml@templates
+    parameters:
+      dotnetSdkVersion: '$(DotNet.Sdk.Version)'
+      projectName: '$(Project).Tests.Integration'
+      category: 'Docker'
+  - task: Bash@3
+    inputs:
+      targetType: 'inline'
+      script: |
+        docker logs ${{ parameters.dockerProjectName }}
+      failOnStderr: true
+    displayName: Show ${{ parameters.dockerProjectName }} logs
+    condition: always()
diff --git a/build/variables/test.yml b/build/variables/test.yml
@@ -0,0 +1,2 @@
+variables:
+  AzureFunctions.HttpPort: 5000
diff --git a/docs/preview/03-Guidance/use-with-dotnet-and-functions.md b/docs/preview/03-Guidance/use-with-dotnet-and-functions.md
@@ -12,7 +12,10 @@ We encourage you to follow the standard [Serilog instructions](https://github.co
 Some aspects we would like to highlight are:
 
 - Make sure to call [`UseSerilog`](https://www.nuget.org/packages/Serilog.AspNetCore) when creating a `IHostBuilder`
-- Remove default for logging including its configuration in `appsettings.json` *(if applicable)*
+- Remove the default Microsoft's `ApplicationInsightsLoggerProvider` via the `RemoveMicrosoftApplicationInsightsLoggerProvider` extension
+- Remove the `Logging` section from the `appsettings.json` *(if applicable)* as this is not used by Serilog
+
+> We need to call `RemoveMicrosoftApplicationInsightsLoggerProvider` to remove Microsoft's `ApplicationInsightsLoggerProvider` because it would conflict with our own Serilog Application Insights sink. We can't guarantee stable telemetry if Microsoft's logger provider is registered as this provider manipulates the telemetry before it get's send out to Application Insights. Removing it ensure that Arcus is in full control of the send-out telemetry.
 
 ## Setting up Serilog with Azure Functions
 
@@ -52,7 +55,8 @@ namespace Arcus.Samples.AzureFunction
 
             builder.Services.AddLogging(loggingBuilder =>
             {
-                loggingBuilder.AddSerilog(logger);
+                loggingBuilder.RemoveMicrosoftApplicationInsightsLoggerProvider()
+                              .AddSerilog(logger);
             });
         }
     }

diff --git a/src/.dockerignore b/src/.dockerignore
@@ -0,0 +1,25 @@
+**/.classpath
+**/.dockerignore
+**/.env
+**/.git
+**/.gitignore
+**/.project
+**/.settings
+**/.toolstarget
+**/.vs
+**/.vscode
+**/*.*proj.user
+**/*.dbmdl
+**/*.jfm
+**/azds.yaml
+**/bin
+**/charts
+**/docker-compose*
+**/Dockerfile*
+**/node_modules
+**/npm-debug.log
+**/obj
+**/secrets.dev.yaml
+**/values.dev.yaml
+LICENSE
+README.md
diff --git a/src/Arcus.Observability.Telemetry.AzureFunctions/Extensions/ILoggerBuilderExtensions.cs b/src/Arcus.Observability.Telemetry.AzureFunctions/Extensions/ILoggerBuilderExtensions.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using GuardNet;
 using Microsoft.Extensions.DependencyInjection;
 
@@ -37,5 +38,32 @@ public static ILoggingBuilder ClearProvidersExceptFunctionProviders(this ILoggin
 
             return loggingBuilder;
         }
+
+        /// <summary>
+        /// Removes Microsoft's 'ApplicationInsightsLoggerProvider' type from the registered logging providers
+        /// so that Arcus' Serilog Application Insights sink is the only one place telemetry is created and sent to Microsoft Application Insights.
+        /// </summary>
+        /// <param name="builder">The builder instance to add and remove logging providers.</param>
+        /// <returns>A <see cref="ILoggingBuilder"/> without Microsoft's 'ApplicationInsightsLoggerProvider' registration.</returns>
+        /// <exception cref="ArgumentNullException">Thrown when the <paramref name="builder"/> is <c>null</c>.</exception>
+        /// <exception cref="InvalidOperationException">Thrown when no 'ApplicationInsightsLoggerProvider' instance can be found in the <paramref name="builder"/>.</exception>
+        public static ILoggingBuilder RemoveMicrosoftApplicationInsightsLoggerProvider(this ILoggingBuilder builder)
+        {
+            Guard.NotNull(builder, nameof(builder), "Requires an instance of the logging builder to remove Microsoft's 'ApplicationInsightsLoggerProvider'");
+
+            ServiceDescriptor descriptor = 
+                builder.Services.FirstOrDefault(service => service.ImplementationType?.Name == "ApplicationInsightsLoggerProvider");
+
+            if (descriptor is null)
+            {
+                throw new InvalidOperationException(
+                    "Cannot find the type 'ApplicationInsightsLoggerProvider' in the registered logging providers " 
+                    + "so can't guarantee a correct Arcus implementation that sinks telemetry to Microsoft Application Insights , "
+                    + "please remove this logger provider before sending telemetry via the Arcus Application Insights logging");
+            }
+
+            builder.Services.Remove(descriptor);
+            return builder;
+        }
     }
 }
diff --git a/...vability.Tests.Integration/Serilog/Sinks/ApplicationInsights/AzureFunctionsDockerTests.cs b/...vability.Tests.Integration/Serilog/Sinks/ApplicationInsights/AzureFunctionsDockerTests.cs
@@ -0,0 +1,59 @@
+using System;
+using System.Net;
+using System.Net.Http;
+using System.Threading.Tasks;
+using Microsoft.Azure.ApplicationInsights.Query;
+using Microsoft.Azure.ApplicationInsights.Query.Models;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Logging;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Arcus.Observability.Tests.Integration.Serilog.Sinks.ApplicationInsights
+{
+    [Collection("Docker")]
+    [Trait("Category", "Docker")]
+    public class AzureFunctionsDockerTests : ApplicationInsightsSinkTests
+    {
+        private static readonly HttpClient HttpClient = new HttpClient();
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AzureFunctionsDockerTests" /> class.
+        /// </summary>
+        public AzureFunctionsDockerTests(ITestOutputHelper outputWriter) 
+            : base(outputWriter)
+        {
+        }
+
+        [Fact]
+        public async Task LogRequest_WithRequestsOperationName_SinksToApplicationInsights()
+        {
+            // Arrange
+            int httpPort = Configuration.GetValue<int>("AzureFunctions:HttpPort");
+            string? requestUri = $"http://localhost:{httpPort}/api/order";
+            Logger.LogInformation("GET -> {URI}", requestUri);
+
+            using (HttpResponseMessage response = await HttpClient.GetAsync(requestUri))
+            {
+                Logger.LogInformation("{StatusCode} <- {URI}", response.StatusCode, requestUri);
+                Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+
+                using (ApplicationInsightsDataClient client = CreateApplicationInsightsClient())
+                {
+                    await RetryAssertUntilTelemetryShouldBeAvailableAsync(async () =>
+                    {
+                        EventsResults<EventsRequestResult> results = await client.Events.GetRequestEventsAsync(ApplicationId, PastHalfHourTimeSpan);
+                        Assert.NotEmpty(results.Value);
+                        AssertX.Any(results.Value, result =>
+                        {
+                            Assert.Contains("order", result.Request.Url);
+                            Assert.Equal("200", result.Request.ResultCode);
+                            Assert.True(Guid.TryParse(result.Request.Id, out Guid _));
+                            Assert.Equal(HttpMethod.Get.Method + " /api/order", result.Operation.Name);
+                        });
+                    });
+                }
+            }
+        }
+    }
+}
diff --git a/src/Arcus.Observability.Tests.Integration/appsettings.json b/src/Arcus.Observability.Tests.Integration/appsettings.json
@@ -2,6 +2,9 @@
   "ApplicationInsights": {
     "InstrumentationKey": "#{ApplicationInsights.InstrumentationKey}#",
     "ApiKey": "#{ApplicationInsights.ApiKey}#",
-    "ApplicationId": "#{ApplicationInsights.ApplicationId}#" 
-  }
+    "ApplicationId": "#{ApplicationInsights.ApplicationId}#"
+  },
+  "AzureFunctions": {
+    "HttpPort":  "#{AzureFunctions.HttpPort}#" 
+  } 
 }