Self Diagnostics bug fixes (#2095)

* Self Diagnostics bug fixes * Add comments * Remove unnecesary trait Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
open-telemetry · Jun 22, 2021 · db6dc75 · db6dc75
1 parent 8948fce
commit db6dc75
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 27 deletions.
diff --git a/src/OpenTelemetry/Internal/SelfDiagnosticsConfigRefresher.cs b/src/OpenTelemetry/Internal/SelfDiagnosticsConfigRefresher.cs
@@ -274,10 +274,18 @@ private void Dispose(bool disposing)
                         this.cancellationTokenSource.Dispose();
                     }
 
+                    // Dispose EventListner before files, because EventListner writes to files.
+                    if (this.eventListener != null)
+                    {
+                        this.eventListener.Dispose();
+                    }
+
                     // Ensure worker thread properly finishes.
                     // Or it might have created another MemoryMappedFile in that thread
                     // after the CloseLogFile() below is called.
                     this.CloseLogFile();
+
+                    // Dispose ThreadLocal variables after the file handles are disposed.
                     this.viewStream.Dispose();
                     this.memoryMappedFileCache.Dispose();
                 }

diff --git a/src/OpenTelemetry/Internal/SelfDiagnosticsEventListener.cs b/src/OpenTelemetry/Internal/SelfDiagnosticsEventListener.cs
@@ -67,6 +67,7 @@ public SelfDiagnosticsEventListener(EventLevel logLevel, SelfDiagnosticsConfigRe
         public override void Dispose()
         {
             this.Dispose(true);
+            base.Dispose();
             GC.SuppressFinalize(this);
         }
 
@@ -85,6 +86,11 @@ public override void Dispose()
         /// <returns>The position of the buffer after the last byte of the resulting sequence.</returns>
         internal static int EncodeInBuffer(string str, bool isParameter, byte[] buffer, int position)
         {
+            if (string.IsNullOrEmpty(str))
+            {
+                return position;
+            }
+
             int charCount = str.Length;
             int ellipses = isParameter ? "{...}\n".Length : "...\n".Length;
 
@@ -338,9 +344,6 @@ protected virtual void Dispose(bool disposing)
             }
 
             this.disposedValue = true;
-
-            // Should call base.Dispose(disposing) here, but EventListener doesn't have Dispose(bool).
-            base.Dispose();
         }
     }
 }
diff --git a/test/OpenTelemetry.Tests/Internal/SelfDiagnosticsConfigParserTest.cs b/test/OpenTelemetry.Tests/Internal/SelfDiagnosticsConfigParserTest.cs
@@ -21,7 +21,6 @@ namespace OpenTelemetry.Internal.Tests
     public class SelfDiagnosticsConfigParserTest
     {
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigParser_TryParseFilePath_Success()
         {
             string configJson = "{ \t \n "
@@ -33,7 +32,6 @@ public void SelfDiagnosticsConfigParser_TryParseFilePath_Success()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigParser_TryParseFilePath_MissingField()
         {
             string configJson = @"{
@@ -44,7 +42,6 @@ public void SelfDiagnosticsConfigParser_TryParseFilePath_MissingField()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigParser_TryParseFileSize()
         {
             string configJson = @"{
@@ -56,7 +53,6 @@ public void SelfDiagnosticsConfigParser_TryParseFileSize()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigParser_TryParseFileSize_CaseInsensitive()
         {
             string configJson = @"{
@@ -69,7 +65,6 @@ public void SelfDiagnosticsConfigParser_TryParseFileSize_CaseInsensitive()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigParser_TryParseFileSize_MissingField()
         {
             string configJson = @"{
@@ -80,7 +75,6 @@ public void SelfDiagnosticsConfigParser_TryParseFileSize_MissingField()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigParser_TryParseLogLevel()
         {
             string configJson = @"{

diff --git a/test/OpenTelemetry.Tests/Internal/SelfDiagnosticsConfigRefresherTest.cs b/test/OpenTelemetry.Tests/Internal/SelfDiagnosticsConfigRefresherTest.cs
@@ -36,7 +36,6 @@ public SelfDiagnosticsConfigRefresherTest(ITestOutputHelper output)
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigRefresher_OmitAsConfigured()
         {
             try
@@ -63,7 +62,6 @@ public void SelfDiagnosticsConfigRefresher_OmitAsConfigured()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsConfigRefresher_CaptureAsConfigured()
         {
             try

diff --git a/test/OpenTelemetry.Tests/Internal/SelfDiagnosticsEventListenerTest.cs b/test/OpenTelemetry.Tests/Internal/SelfDiagnosticsEventListenerTest.cs
@@ -31,7 +31,6 @@ public class SelfDiagnosticsEventListenerTest
         private const string EllipsesWithBrackets = "{...}\n";
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_constructor_Invalid_Input()
         {
             // no configRefresher object
@@ -42,7 +41,6 @@ public void SelfDiagnosticsEventListener_constructor_Invalid_Input()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EventSourceSetup_LowerSeverity()
         {
             var configRefresherMock = new Mock<SelfDiagnosticsConfigRefresher>();
@@ -54,7 +52,6 @@ public void SelfDiagnosticsEventListener_EventSourceSetup_LowerSeverity()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EventSourceSetup_HigherSeverity()
         {
             var configRefresherMock = new Mock<SelfDiagnosticsConfigRefresher>();
@@ -68,7 +65,6 @@ public void SelfDiagnosticsEventListener_EventSourceSetup_HigherSeverity()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_WriteEvent()
         {
             // Arrange
@@ -94,7 +90,6 @@ public void SelfDiagnosticsEventListener_WriteEvent()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_DateTimeGetBytes()
         {
             var configRefresherMock = new Mock<SelfDiagnosticsConfigRefresher>();
@@ -133,7 +128,6 @@ public void SelfDiagnosticsEventListener_DateTimeGetBytes()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EmitEvent_OmitAsConfigured()
         {
             // Arrange
@@ -159,7 +153,6 @@ public void SelfDiagnosticsEventListener_EmitEvent_OmitAsConfigured()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EmitEvent_CaptureAsConfigured()
         {
             // Arrange
@@ -183,7 +176,15 @@ public void SelfDiagnosticsEventListener_EmitEvent_CaptureAsConfigured()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
+        public void SelfDiagnosticsEventListener_EncodeInBuffer_Null()
+        {
+            byte[] buffer = new byte[20];
+            int startPos = 0;
+            int endPos = SelfDiagnosticsEventListener.EncodeInBuffer(null, false, buffer, startPos);
+            Assert.Equal(startPos, endPos);
+        }
+
+        [Fact]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_Empty()
         {
             byte[] buffer = new byte[20];
@@ -194,7 +195,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_Empty()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_EnoughSpace()
         {
             byte[] buffer = new byte[20];
@@ -208,7 +208,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_EnoughSpace()
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_NotEnoughSpaceForFullString()
         {
             byte[] buffer = new byte[20];
@@ -222,7 +221,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_NotEnoughSpaceForFullStr
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_NotEvenSpaceForTruncatedString()
         {
             byte[] buffer = new byte[20];
@@ -233,7 +231,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_NotEvenSpaceForTruncated
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_NotEvenSpaceForTruncationEllipses()
         {
             byte[] buffer = new byte[20];
@@ -243,7 +240,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_NotEvenSpaceForTruncatio
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_EnoughSpace()
         {
             byte[] buffer = new byte[20];
@@ -254,7 +250,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_EnoughSpace(
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_NotEnoughSpaceForFullString()
         {
             byte[] buffer = new byte[20];
@@ -265,7 +260,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_NotEnoughSpa
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_NotEvenSpaceForTruncatedString()
         {
             byte[] buffer = new byte[20];
@@ -276,7 +270,6 @@ public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_NotEvenSpace
         }
 
         [Fact]
-        [Trait("Platform", "Any")]
         public void SelfDiagnosticsEventListener_EncodeInBuffer_IsParameter_NotEvenSpaceForTruncationEllipses()
         {
             byte[] buffer = new byte[20];