Non windows persistent storage short-term fix #655
Conversation
cijothomas
changed the title
|
Some notes: |
| @@ -369,7 +369,7 @@ public void TransmissionSendingFailedWarning(string transmissionId, string excep | |||
| [Event( | |||
| 55, | |||
| Keywords = Keywords.UserActionable, | |||
| Message = "Access to the local storage was denied (User: {1}). If you want Application Insights SDK to store telemetry locally on disk in case of transient network issues please give the process access either to %LOCALAPPDATA% or to %TEMP% folder. 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}). 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. 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}.", | |||
There was a problem hiding this comment.
might need to link to an actual documentation instead of this long but in-complete (due to lack of instruction for linux/docker) text.
|
based on offline dicussion with dmitry, |
| private readonly IDictionary environment; | ||
| private readonly string customFolderName; | ||
| private readonly WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); | ||
| private IIdentityProvider identityProvider; | ||
|
|
||
| public ApplicationFolderProvider(string folderName = null) | ||
| : this(Environment.GetEnvironmentVariables(), folderName) |
| private readonly IDictionary environment; | ||
| private readonly string customFolderName; | ||
| private readonly WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); | ||
| private IIdentityProvider identityProvider; |
There was a problem hiding this comment.
it can still be readonly. It's initialized in constructor
| @@ -128,6 +154,10 @@ private IPlatformFolder CreateAndValidateApplicationFolder(string rootPath, bool | |||
| if (createSubFolder) | |||
| { | |||
| telemetryDirectory = this.CreateTelemetrySubdirectory(telemetryDirectory); | |||
| if (!this.ApplySecurityToDirectory(telemetryDirectory)) | |||
| { | |||
| throw new SecurityException("Unable to apply security restrictions to the storage directory."); | |||
There was a problem hiding this comment.
It looks like this exception/message is going to be thrown every time this channel is created in non-Windows environment because this.ApplySecurityToDirectory() will return false there. Comment in the catch below states that "// The caller does not have code access permission to create the directory" is the reason for this SecurityException. However, the actual reason is that we simply do not support that platform.
| @@ -481,6 +482,12 @@ public void ItemRejectedNoInstrumentationKey(string item, string appDomainName = | |||
| this.WriteEvent(67, item ?? string.Empty, this.ApplicationName); | |||
| } | |||
|
|
|||
| [Event(68, Message = "Failed to set access permissions on storage directory {0}. Error : {1}.", Level = EventLevel.Warning)] | |||
There was a problem hiding this comment.
Should it direct user to the creation of the custom folder for the telemetry storage in case of non-Windows platforms?
There was a problem hiding this comment.
This is a warning message and won't be automatically picked up y SDK diagnostics. There is an error message which will inform the user to create folder themselves.
Fix issue microsoft/ApplicationInsights-aspnetcore#551
Short description of the fix:
For non-windows users ServerTelemetryChannel will not automatically create a directory and store telemetry items when network issues occur. This is because, currently .net core does not have an api to set file permissions, so this temp directory cannot be secured. Users can, however, set a flag AllowUnsecureLocalStorage to true, on ServerTelemetryChannel. to override this behavior. Alternatively, users can create a directory themselves, and pass it to ServerTelemetryChannel.StorageFolder. (this functionality existed originally)
Checklist:
I ran all unit tests locally
For significant contributions please make sure you have completed the following items:
Design discussion issue #
Changes in public surface reviewed
CHANGELOG.md updated with one line description of the fix, and a link to the original issue.
The PR will trigger build, unit test, and functional tests automatically. If your PR was submitted from fork - mention one of committers to initiate the build for you.