From 47f0e807a4c98d74ac882a6d5b0ac95603cca9d5 Mon Sep 17 00:00:00 2001 From: xiang17 Date: Thu, 10 Jun 2021 14:46:10 -0700 Subject: [PATCH 1/2] Enable the self diagnostics and fix a NullReferenceException bug --- .../SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs | 9 +++++++++ .../SelfDiagnostics/SelfDiagnosticsEventListener.cs | 5 +++++ .../SelfDiagnostics/SelfDiagnosticsInitializer.cs | 6 +++--- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs b/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs index 6c71a752dc..59b0aa9057 100644 --- a/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs +++ b/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs @@ -84,6 +84,15 @@ public void SelfDiagnosticsEventListener_DateTimeGetBytes() CollectionAssert.AreEqual(expected, results); } + [TestMethod] + public void SelfDiagnosticsEventListener_EncodeInBuffer_Null() + { + byte[] buffer = new byte[20]; + int startPos = 0; + int endPos = SelfDiagnosticsEventListener.EncodeInBuffer(null, false, buffer, startPos); + Assert.AreEqual(startPos, endPos); + } + [TestMethod] public void SelfDiagnosticsEventListener_EncodeInBuffer_Empty() { diff --git a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs index 755112440c..63548a4156 100644 --- a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs +++ b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs @@ -71,6 +71,11 @@ public override void Dispose() /// The position of the buffer after the last byte of the resulting sequence. internal static int EncodeInBuffer(string str, bool isParameter, byte[] buffer, int position) { + if (str == null) + { + return position; + } + int charCount = str.Length; int ellipses = isParameter ? "{...}\n".Length : "...\n".Length; diff --git a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsInitializer.cs b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsInitializer.cs index 7d54854c46..2217766509 100644 --- a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsInitializer.cs +++ b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsInitializer.cs @@ -15,7 +15,7 @@ internal class SelfDiagnosticsInitializer : IDisposable // Long-living object that holds a refresher which checks whether the configuration file was updated // every 10 seconds. - // private readonly SelfDiagnosticsConfigRefresher configRefresher; + private readonly SelfDiagnosticsConfigRefresher configRefresher; static SelfDiagnosticsInitializer() { @@ -27,7 +27,7 @@ static SelfDiagnosticsInitializer() private SelfDiagnosticsInitializer() { - // this.configRefresher = new SelfDiagnosticsConfigRefresher(); + this.configRefresher = new SelfDiagnosticsConfigRefresher(); } /// @@ -54,7 +54,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - // this.configRefresher.Dispose(); + this.configRefresher.Dispose(); } } } From 146058a5af56b3893568189cbd6a0a584e41c475 Mon Sep 17 00:00:00 2001 From: xiang17 Date: Mon, 14 Jun 2021 14:11:53 -0700 Subject: [PATCH 2/2] Fix compilation warnings. --- .../SelfDiagnosticsEventListenerTest.cs | 12 ++- .../SelfDiagnosticsConfigRefresher.cs | 4 + .../SelfDiagnosticsEventListener.cs | 90 +++++++++---------- CHANGELOG.md | 2 +- 4 files changed, 54 insertions(+), 54 deletions(-) diff --git a/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs b/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs index 59b0aa9057..6eaf20ca21 100644 --- a/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs +++ b/BASE/Test/Microsoft.ApplicationInsights.Test/Microsoft.ApplicationInsights.Tests/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListenerTest.cs @@ -1,11 +1,9 @@ namespace Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing.SelfDiagnostics { using System; - using System.Collections.Generic; using System.Diagnostics.Tracing; - using System.Linq; + using System.Globalization; using System.Text; - using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -55,9 +53,9 @@ public void SelfDiagnosticsEventListener_DateTimeGetBytes() // Check DateTimeKind of Utc, Local, and Unspecified DateTime[] datetimes = new DateTime[] { - DateTime.SpecifyKind(DateTime.Parse("1996-12-01T14:02:31.1234567-08:00"), DateTimeKind.Utc), - DateTime.SpecifyKind(DateTime.Parse("1996-12-01T14:02:31.1234567-08:00"), DateTimeKind.Local), - DateTime.SpecifyKind(DateTime.Parse("1996-12-01T14:02:31.1234567-08:00"), DateTimeKind.Unspecified), + DateTime.SpecifyKind(DateTime.Parse("1996-12-01T14:02:31.1234567-08:00", CultureInfo.InvariantCulture), DateTimeKind.Utc), + DateTime.SpecifyKind(DateTime.Parse("1996-12-01T14:02:31.1234567-08:00", CultureInfo.InvariantCulture), DateTimeKind.Local), + DateTime.SpecifyKind(DateTime.Parse("1996-12-01T14:02:31.1234567-08:00", CultureInfo.InvariantCulture), DateTimeKind.Unspecified), DateTime.UtcNow, DateTime.Now, }; @@ -76,7 +74,7 @@ public void SelfDiagnosticsEventListener_DateTimeGetBytes() string[] results = new string[datetimes.Length]; for (int i = 0; i < datetimes.Length; i++) { - int len = listener.DateTimeGetBytes(datetimes[i], buffer, pos); + int len = SelfDiagnosticsEventListener.DateTimeGetBytes(datetimes[i], buffer, pos); results[i] = Encoding.Default.GetString(buffer, pos, len); pos += len; } diff --git a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsConfigRefresher.cs b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsConfigRefresher.cs index d31f23fde8..5ec4aa95be 100644 --- a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsConfigRefresher.cs +++ b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsConfigRefresher.cs @@ -105,6 +105,10 @@ private void Dispose(bool disposing) // Or it might have created another MemoryMappedFile in that thread // after the Dispose() below is called. this.memoryMappedFileHandler.Dispose(); + if (this.eventListener != null) + { + this.eventListener.Dispose(); + } } this.disposedValue = true; diff --git a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs index 63548a4156..8173c311db 100644 --- a/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs +++ b/BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/SelfDiagnostics/SelfDiagnosticsEventListener.cs @@ -53,6 +53,7 @@ public SelfDiagnosticsEventListener(EventLevel logLevel, MemoryMappedFileHandler public override void Dispose() { this.Dispose(true); + base.Dispose(); GC.SuppressFinalize(this); } @@ -71,7 +72,7 @@ public override void Dispose() /// The position of the buffer after the last byte of the resulting sequence. internal static int EncodeInBuffer(string str, bool isParameter, byte[] buffer, int position) { - if (str == null) + if (string.IsNullOrEmpty(str)) { return position; } @@ -115,47 +116,6 @@ internal static int EncodeInBuffer(string str, bool isParameter, byte[] buffer, return position; } - internal void WriteEvent(string eventMessage, ReadOnlyCollection payload) - { - try - { - var buffer = this.writeBuffer.Value; - if (buffer == null) - { - buffer = new byte[BUFFERSIZE]; - this.writeBuffer.Value = buffer; - } - - var pos = this.DateTimeGetBytes(DateTime.UtcNow, buffer, 0); - buffer[pos++] = (byte)':'; - pos = EncodeInBuffer(eventMessage, false, buffer, pos); - if (payload != null) - { - // Not using foreach because it can cause allocations - for (int i = 0; i < payload.Count; ++i) - { - object obj = payload[i]; - if (obj != null) - { - pos = EncodeInBuffer(obj.ToString(), true, buffer, pos); - } - else - { - pos = EncodeInBuffer("null", true, buffer, pos); - } - } - } - - buffer[pos++] = (byte)'\n'; - this.fileHandler.Write(buffer, pos - 0); - } - catch (Exception) - { - // Fail to allocate memory for buffer - // In this case, silently fail. - } - } - /// /// Write the datetime formatted string into bytes byte-array starting at byteIndex position. /// @@ -178,7 +138,7 @@ internal void WriteEvent(string eventMessage, ReadOnlyCollection payload /// Array of bytes to write. /// Starting index into bytes array. /// The number of bytes written. - internal int DateTimeGetBytes(DateTime datetime, byte[] bytes, int byteIndex) + internal static int DateTimeGetBytes(DateTime datetime, byte[] bytes, int byteIndex) { int num; int pos = byteIndex; @@ -261,6 +221,47 @@ internal int DateTimeGetBytes(DateTime datetime, byte[] bytes, int byteIndex) return pos - byteIndex; } + internal void WriteEvent(string eventMessage, ReadOnlyCollection payload) + { + try + { + var buffer = this.writeBuffer.Value; + if (buffer == null) + { + buffer = new byte[BUFFERSIZE]; + this.writeBuffer.Value = buffer; + } + + var pos = DateTimeGetBytes(DateTime.UtcNow, buffer, 0); + buffer[pos++] = (byte)':'; + pos = EncodeInBuffer(eventMessage, false, buffer, pos); + if (payload != null) + { + // Not using foreach because it can cause allocations + for (int i = 0; i < payload.Count; ++i) + { + object obj = payload[i]; + if (obj != null) + { + pos = EncodeInBuffer(obj.ToString(), true, buffer, pos); + } + else + { + pos = EncodeInBuffer("null", true, buffer, pos); + } + } + } + + buffer[pos++] = (byte)'\n'; + this.fileHandler.Write(buffer, pos - 0); + } + catch (Exception) + { + // Fail to allocate memory for buffer + // In this case, silently fail. + } + } + protected override void OnEventSourceCreated(EventSource eventSource) { if (eventSource.Name.StartsWith(EventSourceNamePrefix, StringComparison.Ordinal)) @@ -315,9 +316,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/CHANGELOG.md b/CHANGELOG.md index 8549f0d9e1..8ae22fbfef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog ## VNext - +- [Enable the self diagnostics and fix a NullReferenceException bug](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2302) ## Version 2.18.0-beta2 - Reduce technical debt: Use pattern matching