From 275894876cb70a794a8d23e4dd43aeaa0cbe6c4e Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 25 Aug 2020 01:01:34 -0700 Subject: [PATCH 1/2] ServerTelemetryChannel to not use any storage if configured StorageFolder has issues. --- .../ApplicationFolderProviderTest.cs | 86 ++++++++++++++++--- .../ApplicationFolderProvider.cs | 24 ++++-- .../TelemetryChannelEventSource.cs | 27 ++++-- CHANGELOG.md | 1 + 4 files changed, 112 insertions(+), 26 deletions(-) diff --git a/BASE/Test/ServerTelemetryChannel.Test/TelemetryChannel.Tests/Implementation/ApplicationFolderProviderTest.cs b/BASE/Test/ServerTelemetryChannel.Test/TelemetryChannel.Tests/Implementation/ApplicationFolderProviderTest.cs index 087931c9d7..c1b21fa8ec 100644 --- a/BASE/Test/ServerTelemetryChannel.Test/TelemetryChannel.Tests/Implementation/ApplicationFolderProviderTest.cs +++ b/BASE/Test/ServerTelemetryChannel.Test/TelemetryChannel.Tests/Implementation/ApplicationFolderProviderTest.cs @@ -57,6 +57,60 @@ public void GetApplicationFolderReturnsSubfolderFromLocalAppDataFolder() localAppData.Delete(true); } + [TestCategory("WindowsOnly")] + [TestMethod] + public void GetApplicationFolderReturnsCustomFolderWhenConfiguredAndExists() + { + DirectoryInfo localAppData = this.CreateTestDirectory(@"AppData\Local"); + DirectoryInfo customFolder = this.CreateTestDirectory(@"Custom"); + + try + { + var environmentVariables = new Hashtable { { "LOCALAPPDATA", localAppData.FullName } }; + var provider = new ApplicationFolderProvider(environmentVariables, customFolder.FullName); + IPlatformFolder applicationFolder = provider.GetApplicationFolder(); + Assert.IsNotNull(applicationFolder); + Assert.AreEqual(customFolder.Name, ((PlatformFolder)applicationFolder).Folder.Name, "Configured custom folder should be used when available."); + } + finally + { + DeleteIfExists(localAppData); + DeleteIfExists(customFolder); + } + } + + [TestMethod] + [TestCategory("WindowsOnly")] + public void GetApplicationFolderReturnsNullWhenCustomFolderConfiguredAndNotExists() + { + DirectoryInfo localAppData = this.CreateTestDirectory(@"AppData\Local"); + DirectoryInfo temp = this.CreateTestDirectory(@"AppData\Temp"); + DirectoryInfo customFolder = new DirectoryInfo(@"Custom"); + Assert.IsFalse(customFolder.Exists); + try + { + var environmentVariables = new Hashtable(); + environmentVariables.Add("LOCALAPPDATA", localAppData.FullName); + environmentVariables.Add("TEMP", localAppData.FullName); + var provider = new ApplicationFolderProvider(environmentVariables, customFolder.FullName); + IPlatformFolder applicationFolder = provider.GetApplicationFolder(); + + // VALIDATE + // That the applicationfolder returned is null. + Assert.IsNull(applicationFolder); + + // Also validate that SDK does not + // create the customFolder. + Assert.IsFalse(customFolder.Exists); + } + finally + { + DeleteIfExists(localAppData); + DeleteIfExists(temp); + DeleteIfExists(customFolder); + } + } + [TestMethod] [TestCategory("WindowsOnly")] public void GetApplicationFolderReturnsSubfolderFromCustomFolderFirst() @@ -98,8 +152,8 @@ public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsAv { DirectoryInfo localAppData = this.CreateTestDirectory(@"AppData\Local", FileSystemRights.CreateDirectories, AccessControlType.Deny); DirectoryInfo temp = this.CreateTestDirectory("Temp"); - var environmentVariables = new Hashtable - { + var environmentVariables = new Hashtable + { { "LOCALAPPDATA", localAppData.FullName }, { "TEMP", temp.FullName }, }; @@ -119,8 +173,8 @@ public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsAv public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsInvalid() { DirectoryInfo temp = this.CreateTestDirectory("Temp"); - var environmentVariables = new Hashtable - { + var environmentVariables = new Hashtable + { { "LOCALAPPDATA", " " }, { "TEMP", temp.FullName }, }; @@ -139,8 +193,8 @@ public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsIn public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsUnmappedDrive() { DirectoryInfo temp = this.CreateTestDirectory("Temp"); - var environmentVariables = new Hashtable - { + var environmentVariables = new Hashtable + { { "LOCALAPPDATA", @"L:\Temp" }, { "TEMP", temp.FullName }, }; @@ -163,8 +217,8 @@ public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsTo DirectoryInfo temp = this.CreateTestDirectory("Temp"); // Initialize ApplicationfolderProvider - var environmentVariables = new Hashtable - { + var environmentVariables = new Hashtable + { { "LOCALAPPDATA", longDirectoryName }, { "TEMP", temp.FullName }, }; @@ -184,8 +238,8 @@ public void GetApplicationFolderReturnsSubfolderFromTempFolderIfLocalAppDataIsTo [TestCategory("WindowsOnly")] public void GetApplicationFolderReturnsNullWhenNeitherLocalAppDataNorTempFoldersAreAccessible() { - var environmentVariables = new Hashtable - { + var environmentVariables = new Hashtable + { { "LOCALAPPDATA", this.CreateTestDirectory(@"AppData\Local", FileSystemRights.CreateDirectories, AccessControlType.Deny).FullName }, { "TEMP", this.CreateTestDirectory("Temp", FileSystemRights.CreateDirectories, AccessControlType.Deny).FullName }, }; @@ -234,7 +288,7 @@ public void GetApplicationFolderReturnsNullWhenFolderAlreadyExistsButDeniesRight [TestCategory("WindowsOnly")] public void GetApplicationFolderReturnsNullWhenFolderAlreadyExistsButDeniesRightToWrite() { - this.GetApplicationFolderReturnsNullWhenFolderAlreadyExistsButNotAccessible(FileSystemRights.Write); + this.GetApplicationFolderReturnsNullWhenFolderAlreadyExistsButNotAccessible(FileSystemRights.Write); } [TestMethod] @@ -439,7 +493,7 @@ private void GetApplicationFolderReturnsNullWhenFolderAlreadyExistsButNotAccessi DirectoryInfo applicationInsights = microsoft.GetDirectories().Single(); DirectoryInfo application = applicationInsights.GetDirectories().Single(); using (new DirectoryAccessDenier(application, rights)) - { + { // Try getting the inaccessible folder Assert.IsNull(provider.GetApplicationFolder()); } @@ -453,5 +507,13 @@ private DirectoryInfo CreateTestDirectory(string path, FileSystemRights rights = directory.SetAccessControl(security); return directory; } + + private void DeleteIfExists(DirectoryInfo directoryToDelete) + { + if (directoryToDelete.Exists) + { + directoryToDelete.Delete(true); + } + } } } diff --git a/BASE/src/ServerTelemetryChannel/Implementation/ApplicationFolderProvider.cs b/BASE/src/ServerTelemetryChannel/Implementation/ApplicationFolderProvider.cs index 605f3c9cdc..67af5cbd70 100644 --- a/BASE/src/ServerTelemetryChannel/Implementation/ApplicationFolderProvider.cs +++ b/BASE/src/ServerTelemetryChannel/Implementation/ApplicationFolderProvider.cs @@ -12,7 +12,7 @@ using System.Security.Cryptography; using System.Security.Principal; using System.Text; - using Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.Shared.Implementation; + using Microsoft.ApplicationInsights.WindowsServer.TelemetryChannel.Shared.Implementation; internal class ApplicationFolderProvider : IApplicationFolderProvider { @@ -48,17 +48,25 @@ internal ApplicationFolderProvider(IDictionary environment, string folderName = this.identityProvider = new NonWindowsIdentityProvider(); this.ApplySecurityToDirectory = this.SetSecurityPermissionsToAdminAndCurrentUserNonWindows; } - + this.environment = environment; this.customFolderName = folderName; } public IPlatformFolder GetApplicationFolder() - { + { var errors = new List(this.environment.Count + 1); var result = this.CreateAndValidateApplicationFolder(this.customFolderName, createSubFolder: false, errors: errors); + // User configured custom folder and SDK is unable to use it. + // Log the error message and return without attempting any other folders. + if (!string.IsNullOrEmpty(this.customFolderName) && result == null) + { + TelemetryChannelEventSource.Log.TransmissionCustomStorageError(string.Join(Environment.NewLine, errors), this.identityProvider.GetName(), this.customFolderName); + return result; + } + if (IsWindowsOperatingSystem()) { if (result == null) @@ -80,7 +88,7 @@ public IPlatformFolder GetApplicationFolder() } } else - { + { if (result == null) { object tmpdir = this.environment["TMPDIR"]; @@ -154,7 +162,7 @@ private static void CheckAccessPermissions(DirectoryInfo telemetryDirectory) testFile.Write(new[] { default(byte) }, 0, 1); } - // FileSystemRights.ListDirectory and FileSystemRights.Read + // FileSystemRights.ListDirectory and FileSystemRights.Read telemetryDirectory.GetFiles(testFileName); // FileSystemRights.DeleteSubdirectoriesAndFiles @@ -201,7 +209,7 @@ private IPlatformFolder CreateAndValidateApplicationFolder(string rootPath, bool { telemetryDirectory = this.CreateTelemetrySubdirectory(telemetryDirectory); if (!this.ApplySecurityToDirectory(telemetryDirectory)) - { + { throw new SecurityException("Unable to apply security restrictions to the storage directory."); } } @@ -264,14 +272,14 @@ private DirectoryInfo CreateTelemetrySubdirectory(DirectoryInfo root) string subdirectoryName = GetSHA256Hash(appIdentity); string subdirectoryPath = Path.Combine("Microsoft", "ApplicationInsights", subdirectoryName); DirectoryInfo subdirectory = root.CreateSubdirectory(subdirectoryPath); - + return subdirectory; } private bool SetSecurityPermissionsToAdminAndCurrentUserNonWindows(DirectoryInfo subdirectory) { // For non-windows simply return true to skip security policy. - // This is until .net core exposes an Api to do this. + // This is until .net core exposes an Api to do this. return true; } diff --git a/BASE/src/ServerTelemetryChannel/Implementation/TelemetryChannelEventSource.cs b/BASE/src/ServerTelemetryChannel/Implementation/TelemetryChannelEventSource.cs index 602d095cae..8694a12202 100644 --- a/BASE/src/ServerTelemetryChannel/Implementation/TelemetryChannelEventSource.cs +++ b/BASE/src/ServerTelemetryChannel/Implementation/TelemetryChannelEventSource.cs @@ -36,8 +36,8 @@ public void BackoffDisabled(string appDomainName = "Incorrect") } // Verbosity is Error - so it is always sent to portal; Keyword is Diagnostics so throttling is not applied. - [Event(2, - Message = "Diagnostic message: backoff logic was enabled. Backoff internal exceeded {0} min. Last status code received from the backend was {1}.", + [Event(2, + Message = "Diagnostic message: backoff logic was enabled. Backoff internal exceeded {0} min. Last status code received from the backend was {1}.", Level = EventLevel.Error, Keywords = Keywords.Diagnostics | Keywords.UserActionable)] public void BackoffEnabled(double intervalInMin, int statusCode, string appDomainName = "Incorrect") @@ -146,7 +146,7 @@ public void TelemetryChannelFlush(string appDomainName = "Incorrect") { this.WriteEvent(19, this.ApplicationName); } - + [Event(20, Message = "{0} passed to channel with iKey {1}...", Level = EventLevel.Verbose)] public void TelemetryChannelSend(string type, string key, string appDomainName = "Incorrect") { @@ -368,9 +368,9 @@ public void TransmissionSendingFailedWarning(string transmissionId, string excep } [Event( - 55, + 55, Keywords = Keywords.UserActionable, - Message = "Local storage access has resulted in an error (User: {1}) (CustomFolder: {2}). If you want Application Insights SDK to store telemetry locally on disk in case of transient network issues please give the process access to %LOCALAPPDATA% or %TEMP% folder. If application is running in non-windows platform, create StorageFolder yourself, and set ServerTelemetryChannel.StorageFolder to the custom folder name. After you gave access to the folder you need to restart the process. Currently monitoring will continue but if telemetry cannot be sent it will be dropped. Error message: {0}.", + Message = "Local storage access has resulted in an error (User: {1}) (CustomFolder: {2}). If you want Application Insights SDK to store telemetry locally on disk in case of transient network issues please give the process access to %LOCALAPPDATA% or %TEMP% folder. If application is running in non-windows platform, create StorageFolder yourself, and set ServerTelemetryChannel.StorageFolder to the custom folder name. After you gave access to the folder you need to restart the process. Currently monitoring will continue but if telemetry cannot be sent it will be dropped. Error message: {0}.", Level = EventLevel.Error)] public void TransmissionStorageAccessDeniedError(string error, string user, string customFolder, string appDomainName = "Incorrect") { @@ -538,11 +538,26 @@ public void TelemetryChannelNoInstrumentationKey(string appDomainName = "Incorre this.WriteEvent(74, this.ApplicationName); } + [Event( + 75, + Keywords = Keywords.UserActionable, + Message = "Unable to use configured StorageFolder: {2}. Please make sure the folder exist and the application has read/write permissions to the same. Currently monitoring will continue but if telemetry cannot be sent it will be dropped. User: {1} Error message: {0}.", + Level = EventLevel.Error)] + public void TransmissionCustomStorageError(string error, string user, string customFolder, string appDomainName = "Incorrect") + { + this.WriteEvent( + 75, + error ?? string.Empty, + user ?? string.Empty, + customFolder ?? string.Empty, + this.ApplicationName); + } + private static string GetApplicationName() { //// We want to add application name to all events BUT //// It is prohibited by EventSource rules to have more parameters in WriteEvent that in event source method - //// Parameter will be available in payload but in the next versions EventSource may + //// Parameter will be available in payload but in the next versions EventSource may //// start validating that number of parameters match //// It is not allowed to call additional methods, only WriteEvent diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c8082e976..2a51145162 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - [Fix broken correlation and missing in-proc dependency Azure Blob SDK v12](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1915) - [Fix Heartbeat interval not applied until after first heartbeat](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1298) - [Fix: ApplicationInsightsLoggerProvider does not catch exceptions](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1969) +- [ServerTelemetryChannel does not fall back to any default directory if user explicitly configures StorageFolder, and have trouble read/write to it](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2000) ## Version 2.15.0-beta2 - [Read all properties of ApplicationInsightsServiceOptions from IConfiguration](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1882) From c0206ac9ca5f93878554d98dacd6db146c0dbc5a Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 25 Aug 2020 01:10:37 -0700 Subject: [PATCH 2/2] log fix --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91df07bc30..1619e85780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - [Fix Heartbeat interval not applied until after first heartbeat](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1298) - [Fix: ApplicationInsightsLoggerProvider does not catch exceptions](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1969) - Update AppInsights JS snippet used in the code to [latest version](https://github.com/microsoft/ApplicationInsights-JS) -- [ServerTelemetryChannel does not fall back to any default directory if user explicitly configures StorageFolder, and have trouble read/write to it](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2000) +- [ServerTelemetryChannel does not fall back to any default directory if user explicitly configures StorageFolder, and have trouble read/write to it](https://github.com/microsoft/ApplicationInsights-dotnet/pull/2002) ## Version 2.15.0-beta2 - [Read all properties of ApplicationInsightsServiceOptions from IConfiguration](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1882)