Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Telemetry loss on network/server errors in non-windows environment #1792

Closed
rajkumar-rangaraj opened this issue Apr 15, 2020 · 7 comments
Closed
Assignees
Milestone

Comments

@rajkumar-rangaraj
Copy link
Member

Issue

ServerTelemetryChannel channel retries sending telemetry if transient errors occur. This channel uses local disk storage to keep items on disk during network outages or high telemetry volumes. In Windows, either %LOCALAPPDATA% or %TEMP% is used if no custom path is specified explicitly. In environments other than Windows, custom folder location should be provided or telemetry won't be stored to local disk.

Below trace information is logged to Application Insights, if storage location is not specified in non-windows environments like Linux or MacOS

AI: Local storage access has resulted in an error (User: ) (CustomFolder: ). 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:

In Windows, SDK creates a storage folder in %LOCALAPPDATA% or %TEMP% and restricts access to Current User and Administrators. Restricting permissions in non-windows environment won’t work as there are no .NET APIs available to change permissions in Linux or MacOS. SDK does not add permission when custom folder name is used, hence providing customer folder name is the only solution to use StorageFolder in Linux or MacOS environment. Providing custom storage location requires code change and not a viable option for production applications.

Related Issue: #1067
Microsoft Doc: StorageFolder - Linux/MacOS

Proposal

Modify ServerTelemetryChannel to read StorageFolder from an environment variable named APPLICATIONINSIGHTS_SERVERCHANNEL_STORAGEFOLDER. When this specific environment variable is present, consider the case as same of custom folder and avoid setting permission on it.

@reyang
Copy link
Member

reyang commented Apr 15, 2020

Some principles I could think of:

  1. We don't want to use temp folders (%TEMP% or /tmp) in this scenario, temp folders are designed to store files that can be deleted by the OS during reboot or cleanup.
  2. We don't want to adjust directory permission from the SDK, it will be good that our permission model follows the general OS file permission model (e.g. when you create a note.txt under personal folder, by default the OS will give the right permission setting to the file, instead of asking you to do it explicitly).
  3. We need to pay attention to the potential breaking change, and if there has to be breaking change, we need to have a clear documentation and upgrade guidance.

@TimothyMothra
Copy link
Member

In general, I'm not a fan of adding new Environment Variables.
This will fracture SDK configuration and make supportability worse.

@rajkumar-rangaraj
Copy link
Member Author

IIS worker process runs under application pool identity which could either be service or user account. These accounts will not have access to the folder location %LOCALAPPDATA% or %TEMP%. If we decide to remove the logic of granting permission from SDK, it would completely break our local storage concept in Windows environment.

@cijothomas
Copy link
Contributor

This proposal is just adding ability to get ServerTelemetryChannels StorageFolder from Environment variable. Currently it can only be set in Code.

No changes to permissions models in this proposal. Retain the current behavior as is.
The current behavior of SDK is this -

  1. If user is providing StorageFolder, then SDK simply accepts it. No subfolder is created, no permissions applied. Same behavior in Win/Linux.

  2. If user is not providing StorageFolder -
    a) attempts to use either TMP or LocalAppData in windows. Creates a subfolder in it, and applies permission to this subfolder to have admin/currentuser access only.
    b) In Linux ServerChannel operates without local disk, and drops items on network/other errors.

This proposal makes 2b better, by allowing users to configure StorageFolder from Env variable.

There is a request from the team in Azure which provisions Linux machines - they can create a folder with right permissions, and set an Environment variable pointing to this. And SDK picks it up automatically. This saves user the trouble of creating the folder, and configuring it via code.

This issue is not addressing a permanent solution to the local storage issue. That needs a separate issue, with inputs from privacy team.

@SergeyKanzhelev
Copy link
Contributor

Name of environment variable - lets align with APPINSIGHTS_INSTRUMENTATIONKEY. Perhaps APPINSIGHTS_EVENTS_STORAGE_FOLDER. Also if we set a single folder for a container, it may be a good idea to mark environment variable with the flag whether different ikeys needs to have separate folders. Perhaps value may be APPINSIGHTS_EVENTS_STORAGE_FOLDER=/var/$ikey or APPINSIGHTS_EVENTS_STORAGE_FOLDER=/var/%ikey% if customer wants to split storages for separate ikeys. Recently we had an issue when one storage affected another channel that didn't have an issue.

IIS worker process runs under application pool identity which could either be service or user account. These accounts will not have access to the folder location %LOCALAPPDATA% or %TEMP%. If we decide to remove the logic of granting permission from SDK, it would completely break our local storage concept in Windows environment.

I don't think this is the reason SDK sets permissions. SDK runs under the mentioned identity and has permissions even to set permissions. So definitely has permissions to write.

We don't want to adjust directory permission from the SDK, it will be good that our permission model follows the general OS file permission model (e.g. when you create a note.txt under personal folder, by default the OS will give the right permission setting to the file, instead of asking you to do it explicitly).

Permissions are set to prevent information exposure when a host running different app pools. So app pools will not be able to access telemetry saved by another app.

@TimothyMothra
Copy link
Member

@SergeyKanzhelev we don't have the trademark on "AppInsights". all new Environment variables are using the "ApplicationInsights" name.
Connection Strings are using: "APPLICATIONINSIGHTS_CONNECTION_STRING" https://docs.microsoft.com/en-us/azure/azure-monitor/app/sdk-connection-string?tabs=net#environment-variable

@SergeyKanzhelev
Copy link
Contributor

@TimothyMothra sure, definitely go with ApplicationInsights than. I'm happy this was fixed =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants