Logformatter null check (#2200)

src/OpenTelemetry/CHANGELOG.md

@@ -5,6 +5,9 @@
 * Removes upper constraint for Microsoft.Extensions.Logging
   dependencies. ([#2179](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2179))
 
+* OpenTelemetryLogger modified to not throw, when the
+  formatter supplied in ILogger.Log call is null. ([#2200](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2200))
+
 ## 1.2.0-alpha1
 
 Released 2021-Jul-23

src/OpenTelemetry/Logs/OpenTelemetryLogger.cs

@@ -53,7 +53,7 @@ public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Except
                     this.categoryName,
                     logLevel,
                     eventId,
-                    options.IncludeFormattedMessage ? formatter(state, exception) : null,
+                    options.IncludeFormattedMessage ? formatter?.Invoke(state, exception) : null,
                     options.ParseStateValues ? null : (object)state,
                     exception,
                     options.ParseStateValues ? this.ParseState(state) : null);

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs

@@ -31,9 +31,7 @@
 using OpenTelemetry.Instrumentation.AspNetCore.Implementation;
 using OpenTelemetry.Tests;
 using OpenTelemetry.Trace;
-#if NETCOREAPP2_1
-using TestApp.AspNetCore._2._1;
-#elif NETCOREAPP3_1
+#if NETCOREAPP3_1
 using TestApp.AspNetCore._3._1;
 #else
 using TestApp.AspNetCore._5._0;

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/DependencyInjectionConfigTests.cs

@@ -20,9 +20,7 @@
 using Microsoft.AspNetCore.TestHost;
 using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry.Trace;
-#if NETCOREAPP2_1
-using TestApp.AspNetCore._2._1;
-#elif NETCOREAPP3_1
+#if NETCOREAPP3_1
 using TestApp.AspNetCore._3._1;
 #else
 using TestApp.AspNetCore._5._0;

test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs

@@ -25,9 +25,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using Moq;
 using OpenTelemetry.Trace;
-#if NETCOREAPP2_1
-using TestApp.AspNetCore._2._1;
-#elif NETCOREAPP3_1
+#if NETCOREAPP3_1
 using TestApp.AspNetCore._3._1;
 #else
 using TestApp.AspNetCore._5._0;

test/OpenTelemetry.Tests/Logs/LogRecordTest.cs

@@ -13,11 +13,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 // </copyright>
-
 #if !NET461
-#if NETCOREAPP2_1
-using Microsoft.Extensions.DependencyInjection;
-#endif
 using System;
 using System.Collections;
 using System.Collections.Generic;
@@ -36,11 +32,7 @@ public sealed class LogRecordTest : IDisposable
     {
         private readonly ILogger logger;
         private readonly List<LogRecord> exportedItems = new List<LogRecord>();
-#if NETCOREAPP2_1
-        private readonly ServiceProvider serviceProvider;
-#else
         private readonly ILoggerFactory loggerFactory;
-#endif
         private readonly BaseExportProcessor<LogRecord> processor;
         private readonly BaseExporter<LogRecord> exporter;
         private OpenTelemetryLoggerOptions options;
@@ -49,11 +41,7 @@ public LogRecordTest()
         {
             this.exporter = new InMemoryExporter<LogRecord>(this.exportedItems);
             this.processor = new TestLogRecordProcessor(this.exporter);
-#if NETCOREAPP2_1
-            var serviceCollection = new ServiceCollection().AddLogging(builder =>
-#else
             this.loggerFactory = LoggerFactory.Create(builder =>
-#endif
             {
                 builder.AddOpenTelemetry(options =>
                 {
@@ -64,12 +52,7 @@ public LogRecordTest()
                 builder.AddFilter(typeof(LogRecordTest).FullName, LogLevel.Trace);
             });
 
-#if NETCOREAPP2_1
-            this.serviceProvider = serviceCollection.BuildServiceProvider();
-            this.logger = this.serviceProvider.GetRequiredService<ILogger<LogRecordTest>>();
-#else
             this.logger = this.loggerFactory.CreateLogger<LogRecordTest>();
-#endif
         }
 
         [Fact]
@@ -339,6 +322,32 @@ public void IncludeFormattedMessageTest()
             }
         }
 
+        [Fact]
+        public void IncludeFormattedMessageTestWhenFormatterNull()
+        {
+            this.logger.Log(LogLevel.Information, default, "Hello World!", null, null);
+            var logRecord = this.exportedItems[0];
+            Assert.Null(logRecord.FormattedMessage);
+
+            this.options.IncludeFormattedMessage = true;
+            try
+            {
+                // Pass null as formatter function
+                this.logger.Log(LogLevel.Information, default, "Hello World!", null, null);
+                logRecord = this.exportedItems[1];
+                Assert.Null(logRecord.FormattedMessage);
+
+                var expectedFormattedMessage = "formatted message";
+                this.logger.Log(LogLevel.Information, default, "Hello World!", null, (state, ex) => expectedFormattedMessage);
+                logRecord = this.exportedItems[2];
+                Assert.Equal(expectedFormattedMessage, logRecord.FormattedMessage);
+            }
+            finally
+            {
+                this.options.IncludeFormattedMessage = false;
+            }
+        }
+
         [Fact]
         public void IncludeScopesTest()
         {
@@ -598,11 +607,7 @@ public void ParseStateValuesUsingCustomTest()
 
         public void Dispose()
         {
-#if NETCOREAPP2_1
-            this.serviceProvider?.Dispose();
-#else
             this.loggerFactory?.Dispose();
-#endif
         }
 
         internal struct Food