Ensure that health reporter file is only created when there are any r…

…eports
Azure · Oct 26, 2018 · 6fa6d80 · 6fa6d80
1 parent cc2a173
commit 6fa6d80
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 11 deletions.
diff --git a/...Microsoft.Diagnostics.EventFlow.Core/Implementations/HealthReporters/CsvHealthReporter.cs b/...Microsoft.Diagnostics.EventFlow.Core/Implementations/HealthReporters/CsvHealthReporter.cs
@@ -131,15 +131,21 @@ public void Activate()
 
             try
             {
-                SetNewStreamWriter(Directory.Exists, Directory.CreateDirectory);
-                if (StreamWriter == null)
+                if (EnsureOutputCanBeSaved)
                 {
-                    message = $"Fail to set new stream writer for {nameof(CsvHealthReporter)}.";
-                    if (EnsureOutputCanBeSaved)
+                    SetNewStreamWriter(Directory.Exists, Directory.CreateDirectory);
+                    if (StreamWriter == null)
                     {
+                        message = $"Fail to set new stream writer for {nameof(CsvHealthReporter)}.";
                         throw new InvalidOperationException(message);
                     }
                 }
+                else
+                {
+                    // Delay creation of the health reporter file until there are some reports to write.
+                    this.newStreamRequested = true;
+                }
+
                 this.flushTime = this.getCurrentTime().AddMilliseconds(this.flushPeriodMsec);
 
                 // Start the consumer of the report items in the collection.

diff --git a/test/Microsoft.Diagnostics.EventFlow.Core.Tests/CsvHealthReporterTests.cs b/test/Microsoft.Diagnostics.EventFlow.Core.Tests/CsvHealthReporterTests.cs
@@ -421,10 +421,11 @@ public void ShouldNoOpIfFilesystemIsReadOnly()
             var configuration = BuildTestConfigration();
             using (var reporter = new ReadOnlyFilesystemHealthReporter(configuration))
             {
-                reporter.Activate(); // No exception for the user of the reporter
-                Assert.True(reporter.LogRotationAttempted);
-
+                reporter.Activate();
                 reporter.ReportProblem("Not sure if anyone will see this"); // No exception
+
+                Assert.True(reporter.LogRotationAttempted.WaitOne(StreamOperationTimeoutMsec), 
+                    "The log rotation was never attempted; unable to verify the read-only file system exception was handled");
             }
         }
 
@@ -493,6 +494,8 @@ public void ShouldRandomizeLogFileNameIfRequested()
             using (SharedFolderTestHealthReporter reporter = new SharedFolderTestHealthReporter(configuration))
             {
                 reporter.Activate();
+                reporter.ReportProblem("Something important");
+                reporter.LogFileCreated.WaitOne(StreamOperationTimeoutMsec);
                 // Expect "HealthReporter_", followed by 12-char randomizer string, followed by underscore, followed by date in yyyymmdd format, followed by ".csv"
                 Assert.Matches($"{DefaultReporterPrefix}_[a-zA-Z0-9]{{12}}_20[0-9]{{6}}.csv", reporter.CurrentLogFilePath);
             }

diff --git a/test/TestHelpers/ReadOnlyFilesystemHealthReporter.cs b/test/TestHelpers/ReadOnlyFilesystemHealthReporter.cs
@@ -5,24 +5,28 @@
 
 using System;
 using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.Diagnostics.EventFlow.HealthReporters;
 using Microsoft.Extensions.Configuration;
 
 namespace Microsoft.Diagnostics.EventFlow.TestHelpers
 {
     public class ReadOnlyFilesystemHealthReporter: CsvHealthReporter
     {
-        public bool LogRotationAttempted { get; private set; }
+        public AutoResetEvent LogRotationAttempted { get; private set; }
 
         public ReadOnlyFilesystemHealthReporter(IConfiguration configuration) 
             : base(configuration.ToCsvHealthReporterConfiguration())
         {
-            LogRotationAttempted = false;
+            this.LogRotationAttempted = new AutoResetEvent(false);
         }
 
         internal override string RotateLogFileImp(string logFileFolder, Func<string, bool> fileExist, Action<string> fileDelete, Action<string, string> fileMove)
         {
-            LogRotationAttempted = true;
+            // Delay the notification so that the exception can bubble up the stack and the test does not end prematurely.
+            Task.Delay(TimeSpan.FromMilliseconds(200)).ContinueWith((t) => this.LogRotationAttempted.Set());
+
             // See https://github.com/Azure/diagnostics-eventflow/issues/255 for how this may happen
             throw new IOException("File system is read only");
         }

diff --git a/test/TestHelpers/SharedFolderTestHealthReporter.cs b/test/TestHelpers/SharedFolderTestHealthReporter.cs
@@ -5,6 +5,7 @@
 
 using System;
 using System.IO;
+using System.Threading;
 using Microsoft.Diagnostics.EventFlow.HealthReporters;
 using Microsoft.Extensions.Configuration;
 
@@ -13,15 +14,18 @@ namespace Microsoft.Diagnostics.EventFlow.TestHelpers
     public class SharedFolderTestHealthReporter : CsvHealthReporter
     {
         public string CurrentLogFilePath { get; private set; }
+        public AutoResetEvent LogFileCreated { get; private set; }
 
         public SharedFolderTestHealthReporter(IConfiguration configuration)
             : base(configuration.ToCsvHealthReporterConfiguration())
         {
+            this.LogFileCreated = new AutoResetEvent(false);
         }
 
         internal override FileStream CreateFileStream(string logFilePath)
         {
-            CurrentLogFilePath = logFilePath;
+            this.CurrentLogFilePath = logFilePath;
+            this.LogFileCreated.Set();
             return null;
         }