-
Notifications
You must be signed in to change notification settings - Fork 68
Add monitor to keep track of current value of App Services env vars #870
Changes from all commits
d4a21c3
aab6721
86911c0
9c407f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| namespace Microsoft.ApplicationInsights.WindowsServer | ||
| { | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Net.Http; | ||
| using System.Runtime.Serialization.Json; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.ApplicationInsights.WindowsServer.Implementation; | ||
| using Microsoft.ApplicationInsights.WindowsServer.Implementation.DataContracts; | ||
| using Microsoft.ApplicationInsights.WindowsServer.Mock; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
| using Assert = Xunit.Assert; | ||
|
|
||
| [TestClass] | ||
| public class AppServiceEnvVarMonitorTests | ||
| { | ||
| // used to clean up the environment variables after we've run this test | ||
| static private Dictionary<string, string> environmentInitialState; | ||
|
|
||
| [ClassInitialize] | ||
| static public void InitializeTests(TestContext context) | ||
| { | ||
| environmentInitialState = GetCurrentAppServiceEnvironmentVariableValues(false); | ||
| } | ||
|
|
||
| [ClassCleanup] | ||
| static public void CleanupTests() | ||
| { | ||
| foreach (var kvp in environmentInitialState) | ||
| { | ||
| Environment.SetEnvironmentVariable(kvp.Key, kvp.Value); | ||
| } | ||
|
|
||
| environmentInitialState = null; | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void ConfirmIntervalCheckEnforced() | ||
| { | ||
| var envVars = GetCurrentAppServiceEnvironmentVariableValues(); | ||
|
|
||
| foreach (var kvp in envVars) | ||
| { | ||
| string val = string.Empty; | ||
| AppServiceEnvVarMonitor.GetUpdatedEnvironmentVariable(kvp.Key, ref val); | ||
| // set the value to something new | ||
| Environment.SetEnvironmentVariable(kvp.Key, string.Concat("UPDATED-", val, "-UPDATED")); | ||
| } | ||
|
|
||
| // set the next-check time to a value that won't get hit | ||
| AppServiceEnvVarMonitor.NextCheckTime = DateTime.MaxValue; | ||
|
|
||
| // ensure the current values are indeed different | ||
| var currentEnvVars = GetCurrentAppServiceEnvironmentVariableValues(); | ||
|
|
||
| // ensure the values are cached and aren't getting re-read at this time | ||
| foreach (var kvp in envVars) | ||
| { | ||
| string cachedVal = string.Empty; | ||
| AppServiceEnvVarMonitor.GetUpdatedEnvironmentVariable(kvp.Key, ref cachedVal); | ||
| Assert.Equal(kvp.Value, cachedVal, StringComparer.Ordinal); | ||
| Assert.NotEqual(cachedVal, currentEnvVars[kvp.Key], StringComparer.Ordinal); | ||
| } | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void ConfirmUpdatedEnvironmentIsCaptured() | ||
| { | ||
| var envVars = GetCurrentAppServiceEnvironmentVariableValues(); | ||
|
|
||
| foreach (var kvp in envVars) | ||
| { | ||
| string val = string.Empty; | ||
| AppServiceEnvVarMonitor.GetUpdatedEnvironmentVariable(kvp.Key, ref val); | ||
| // set the value to something new | ||
| Environment.SetEnvironmentVariable(kvp.Key, string.Concat("UPDATED-", val, "-UPDATED")); | ||
| } | ||
|
|
||
| // set the next-check time to a value that will re-read the values immediately | ||
| AppServiceEnvVarMonitor.NextCheckTime = DateTime.MinValue; | ||
|
|
||
| // ensure the current values are indeed different | ||
| var currentEnvVars = GetCurrentAppServiceEnvironmentVariableValues(); | ||
|
|
||
| // ensure the values are re-read | ||
| foreach (var kvp in envVars) | ||
| { | ||
| string cachedVal = string.Empty; | ||
| AppServiceEnvVarMonitor.GetUpdatedEnvironmentVariable(kvp.Key, ref cachedVal); | ||
| Assert.Equal(currentEnvVars[kvp.Key], cachedVal, StringComparer.Ordinal); | ||
| Assert.NotEqual(cachedVal, kvp.Value, StringComparer.Ordinal); | ||
| } | ||
| } | ||
|
|
||
| static private Dictionary<string,string> GetCurrentAppServiceEnvironmentVariableValues(bool supplyValue = true) | ||
| { | ||
| int testValueCount = 0; | ||
| Dictionary<string, string> envVars = new Dictionary<string, string>(); | ||
|
|
||
| foreach (var kvp in AppServiceEnvVarMonitor.CheckedValues) | ||
| { | ||
| string envVar = Environment.GetEnvironmentVariable(kvp.Key); | ||
| if (supplyValue && string.IsNullOrEmpty(envVar)) | ||
| { | ||
| envVar = $"{testValueCount}_Stand-inValue_{testValueCount}"; | ||
| testValueCount++; | ||
| Environment.SetEnvironmentVariable(kvp.Key, envVar); | ||
| } | ||
| envVars.Add(kvp.Key, envVar); | ||
| } | ||
|
|
||
| return envVars; | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| namespace Microsoft.ApplicationInsights.WindowsServer.Implementation | ||
| { | ||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| /// <summary> | ||
| /// Utility to monitor the value of environment variables which may change | ||
| /// during the run of an application. Checks the environment variables | ||
| /// intermittently. | ||
| /// </summary> | ||
| internal static class AppServiceEnvVarMonitor | ||
| { | ||
| // Environment variables tracked by this monitor. (internal to allow tests to modify them) | ||
| internal static readonly Dictionary<string, string> CheckedValues = new Dictionary<string, string>() | ||
| { | ||
| { "WEBSITE_SITE_NAME", string.Empty }, | ||
| { "WEBSITE_HOME_STAMPNAME", string.Empty }, | ||
| { "WEBSITE_HOSTNAME", string.Empty } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In comments for environment variable extraction in this PR, it's noted that WEBSITE_HOSTNAME is not a reliable source:
Do we want to rely on that field, or we may use some tricks, e.g. from the same PR to build something more reliable from what we have.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I say we should resort to these tactics if we have an issue or have proof that App Services are affected by the same failing that Azure Functions are. I have put an issue together to verify that we can detect slot swap using only WEBSITE_HOSTNAME, and to update to using the WEBSITE_SITE_NAME + WEBSITE_SLOT_NAME if the need arises. Note that I've included the WEBSITE_SLOT_NAME in the list of environment variables that the monitor watches for now. |
||
| }; | ||
|
|
||
| // When is the next time we will allow a check to occur? (internal to allow tests to modify this to avoid waits) | ||
| internal static DateTime NextCheckTime = DateTime.MinValue; | ||
|
|
||
| // how often we allow the code to re-check the environment | ||
| private static readonly TimeSpan CheckInterval = TimeSpan.FromSeconds(30); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let user configure it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could do, not sure how much value would be there. I propose we raise an issue and chat about it for the next release.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not overexpose API we are not certain in and may want to pull in the very next PR. We can make it configurable later if we stick with this solution, we cannot unmake public API once it's in stable release without the breaking change. |
||
|
|
||
| /// <summary> | ||
| /// Get the latest value assigned to an environment variable. | ||
| /// </summary> | ||
| /// <param name="envVarName">Name of the environment variable to acquire.</param> | ||
| /// <param name="value">Value of the environment variable (updated within the update interval).</param> | ||
| public static void GetUpdatedEnvironmentVariable(string envVarName, ref string value) | ||
| { | ||
| if (!string.IsNullOrEmpty(envVarName)) | ||
| { | ||
| CheckVariablesIntermittent(); | ||
| CheckedValues.TryGetValue(envVarName, out value); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Simply update the stored environment variables if the last time we | ||
| /// checked from now is greater than the check interval. | ||
| /// </summary> | ||
| private static void CheckVariablesIntermittent() | ||
| { | ||
| DateTime rightNow = DateTime.UtcNow; | ||
| if (rightNow > NextCheckTime) | ||
| { | ||
| NextCheckTime = rightNow + CheckInterval; | ||
|
|
||
| List<string> keys = new List<string>(CheckedValues.Keys); | ||
| foreach (var key in keys) | ||
| { | ||
| CheckedValues[key] = Environment.GetEnvironmentVariable(key); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer lazy initialized and we've added ~3 extra stack frames to every request/dependency in the customer app. Variable will be read synchronously every 30 seconds (may be fine if we do not
Expand..()it, though).We may think of an asynchronous approach here. Discussed offline with Derek.