Add App Services Heartbeat with environment variable monitor by d3r3kk · Pull Request #873 · microsoft/ApplicationInsights-dotnet-server

d3r3kk · 2018-04-03T17:17:31Z

Fix Issue #868 and #872.

Add heartbeat properties when App Services environment is detected. Keep App Services-specific environment variables in sync across slot-swaps (and anything else that would update environment variables).

I ran Unit Tests locally.
Changes in public surface reviewed
- No change to public API
Design discussion issue Enabling Heartbeat in App Services for .NET and .NET Core #868, Verify that App Services slot swaps are being detected in Web SDK #872
CHANGELOG.md updated with one line description of the fix, and a link to the original issue.

…s) that will add some infrastructure-identifying properties. - Implements part of Option 1 in issue #868

…into WindowsServer. Update logic to ensure we keep heartbeat values updated. - Update logic in AzureWebAppRoleEnvironment to also signal AppServicesHeartbeat to update itself when changes are detected - Update AppServicesHeartbeat to expose an Instance variable to make it easier for update signals to be sent to it - Update CHANGELOG - Issue #868 discusses the need for the change to the AzureWebAppRoleEnvrionmeentInitializer

- make new AppServicesHeartbeat module public! - add try/catch scenarios around initialize - add further diagnostic messages where appropriate

- Use to keep the value of WEBSITE_SITE_NAME and other App Services values current during runime

…, update code for testability.

…erviceEnvVar monitor

…Server.Net45.Tests project

- Add heartbeat provider for Web Apps/App Services - Add monitor for App Services environment variables (which can change during the runtime of an AppServices app) - Ensure adequate testing is added, update current Env Var tests to be testable in a parallel environment

…ment variables

- Use env var monitor to check for udpates to environment in controlled/minimal fashion - default to 30 second interval for checking the changes to environment - corrections for stylecop

- PR suggestion

…nges with a subscriber model - Make env var monitor a singleton, and add an event handler for env var updates that consumers can subscribe to - Env var monitor updates according to its own timer - App services telemetry module no longer a singleton, and is no longer coupled to Web App role TelemetryInitializer - Web app role telemetry initializer only updates the environment variables when it has to now

…onment variable monitor - Make environment variable monitor an abstract class - App Services environment variable implements the abstract Environment variable monitor - Improves testability of the environment monitor - Handle proper disposal of the timer in environment monitor

- Tests center around the base class - No unit tests should be indeterminant

d3r3kk · 2018-04-03T17:19:10Z

This is option B, option A is PR #870, and we can go with either PR for 2.6.0-beta3.
Please review this code and we can discuss benefits of this subscriber vs. the prior polling solution.
Note that I am still working on getting the unit tests constructed and passing at the time I wrote this comment.

- ensure tests read a special environment variable specific to each test - Ensure parallel tests do not impede ability to test this class (Now that we have a monitor in place, we need to make the tests get around it)

pharring · 2018-04-03T20:34:42Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+        /// <summary>
+        /// Initializes a new instance of the <see cref="EnvironmentVariableMonitor" /> class.
+        /// </summary>
+        public EnvironmentVariableMonitor(IEnumerable<string> envVars, TimeSpan checkInterval)


Is there any advantage to pass the initial envVars to this constructor?
I mean, the monitor can build up its own list of monitored environment variables via the "GetOrAdd" within GetCurrentEnvironmentVariableValue.
So, why not simplify it and only monitor those variables that are actually queried via the public API.

Oh, I guess the difference is in the "OnEnvironmentVariableUpdated" call-back. You can get a call-back even for variables you didn't actually call GetCurrentEnvironmentVariableValue on.

Yes, I am optimizing for the Azure App Services scenario here.

pharring · 2018-04-03T20:35:03Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+        /// <summary>
+        /// Initializes a new instance of the <see cref="EnvironmentVariableMonitor" /> class.
+        /// </summary>
+        public EnvironmentVariableMonitor(IEnumerable<string> envVars, TimeSpan checkInterval)


Constructor of an abstract class should be protected

Yes of course, thanks for the catch.

…ableMonitor to be protected

Dmitry-Matveev

Overall, I like this PR more than #869. Couple comments and I think we can give it a go.

Dmitry-Matveev · 2018-04-03T21:54:38Z

Src/WindowsServer/WindowsServer.Shared/AppServicesHeartbeatTelemetryModule.cs

+        {
+            try
+            {
+                var hbeatManager = this.GetHeartbeatPropertyManager();


Minor: This will be called every time MonitoredAppServiceEnvVarUpdatedEvent fires, do we expect that GetHeartBeatPropertyManager() may yield new results across process lifetime or we can cache it?
No immediate need to cache it, though - it looks like operation is light enough due to insignificant amount of the telemetry modules we have in the list.

Right now, we are pretty much guaranteed that the HeartbeatPropertyManager is present (since it is the DiagnosticsTelemetryModule itself). And for the lifetime of the app it will either be there or it won't, no change. I think I will change this to cache it.

Please do cache. Otherwise we'll be logging 'AppServiceHeartbeatManagerNotAvailable' everytime this is called. (if hbeatmanager is not available)

d3r3kk: Done.

Dmitry-Matveev · 2018-04-03T22:32:12Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+                this.CheckedValues.TryAdd(varName, Environment.GetEnvironmentVariable(varName));
+            }
+
+            this.environmentCheckTimer = new Timer(this.CheckVariablesIntermittent, null, this.checkInterval, TimeSpan.FromMilliseconds(-1));


If this.checkInterval can ever be 0 (i hope not), then there is a race condition possible - this.CheckVariablesIntermittent can be executed in its entirety before the assignment above happens, so this.environmentCheckTimer() will still be null. The last thing this.CheckVariablesIntermittent() does is to change this.environmentCheckTimer that may cause NullReference.
If I remember correctly, the right pattern was to create Timer like Timer(callback, null, Timeout.Infinite, Timeout.Infinite) and immediately follow up with timer.Change(this.checkInterval, TimeoutInfinite).
Again, I think this only applies if you may have really-really low value as this.checkInterval.

Ok, I wasn't aware of that possibility. I will make the change to a 2-step initialization here.

I think I will enforce a minimum check interval as well - 5 seconds.

Dmitry-Matveev · 2018-04-03T22:36:07Z

Src/WindowsServer/WindowsServer.Shared/AzureWebAppRoleEnvironmentTelemetryInitializer.cs

        {
-            return Environment.GetEnvironmentVariable(WebAppHostNameEnvironmentVariable) ?? string.Empty;
+            string nodeName = string.Empty;
+            AppServiceEnvVarMonitor.Instance.GetCurrentEnvironmentVariableValue(this.WebAppHostNameEnvironmentVariable, ref nodeName);


Is this the quickest read possible out of ConcurrentDictionary (I see that internally it uses GetOrAdd()) - is there anything that reads faster than that for ConsurrentDictionary with no locks whatsoever?
We are OK if several items are still having old data, but having all ongoing requests/dependencies and other telemetry items to sync on read of that dictionary may lead to a bottleneck here.

Good catch to call this out. However, I think I can put any fears to rest.

Notice that by the time this call is being made, the cached value from the environment has already been stored for this update cycle, the key is already in place. This all works together to ensure the GetOrAdd call is simply a TryGetValue call (that has no lock on it as you can see here).

However, in the scenario you suggest (in some future class that we decide to tax the EnvironmentVariableMonitor class as much as possible), it absolutely could be possible to create a nasty bottleneck.

Even then, as far as I am aware, GetOrAdd is the best, fastest way to deal with updates of this nature.

ConcurrentDictionary actually does the only thing I can think of that might be faster, breaking the problem down into a TryGetValue and then using a call to TryUpdateValue only when things have changed.

In the scenario that you site though, where everything is all being updated at once, we still have a bottleneck (albeit one that is handled very nicely by ConcurrentDictionary without us having to write and maintain any complex code).

I probably could have made this shorter:

... We are OK if several items are still having old data...

This is exactly the path that we follow today.

Update all environment variables in a loop, detecting any change.

If we detected a change, notify subscribers to collect their new cached values, ensuring only TryGetValue operations on already tracked items.

But thanks for making me read into how ConcurrentDictionary works under the hood. I do feel better about using it now. I was worried it was locking things even in the case of reads!

@d3r3kk I do not understand how your reply address @Dmitry-Matveev request to avoid any locks in happy path. Simple code of taking current values from local variable will work perfectly fine here

Maybe I was diving too deep. I was investigating if there was a faster way to read from the ConcurrentDictionary whenever it was necessary to do so, I misunderstood the question regarding 'happy path'.

In the common case ('happy path'), we always read from the local variables:

From AzureWebAppRoleEnvironmentTelemetryInitializer.cs, line 45

public void Initialize(ITelemetry telemetry) { if (this.updateEnvVars) // <= this is set in the update callback when change is detected { this.roleName = this.GetRoleName(); this.nodeName = this.GetNodeName(); this.updateEnvVars = false; // <= this ensures we don't check again until another change } ...

Only when a change in the environment is detected do we ever go and get the information from the monitor.

I have no regrets of going down the rabbit hole I did though, now I know more about ConcurrentDictionary!

- Using signed package based off of PR microsoft/ApplicationInsights-dotnet-server#873

SergeyKanzhelev · 2018-04-04T16:44:48Z

Src/WindowsServer/WindowsServer.Shared/AppServicesHeartbeatTelemetryModule.cs

+        internal readonly KeyValuePair<string, string>[] WebHeartbeatPropertyNameEnvVarMap = new KeyValuePair<string, string>[]
+        {
+            new KeyValuePair<string, string>("appSrv_SiteName", "WEBSITE_SITE_NAME"),
+            new KeyValuePair<string, string>("appSrv_SiteName", "WEBSITE_SLOT_NAME"),


are those intentionally has the same key appSrv_SiteName?

No, that was a mistake, thank you for catching this.

SergeyKanzhelev · 2018-04-04T16:50:20Z

Src/WindowsServer/WindowsServer.Shared/AzureWebAppRoleEnvironmentTelemetryInitializer.cs

        {
-            return Environment.GetEnvironmentVariable(WebAppHostNameEnvironmentVariable) ?? string.Empty;
+            string nodeName = string.Empty;
+            AppServiceEnvVarMonitor.Instance.GetCurrentEnvironmentVariableValue(this.WebAppHostNameEnvironmentVariable, ref nodeName);


@d3r3kk I do not understand how your reply address @Dmitry-Matveev request to avoid any locks in happy path. Simple code of taking current values from local variable will work perfectly fine here

SergeyKanzhelev · 2018-04-04T16:55:16Z

Src/WindowsServer/WindowsServer.Shared/AppServicesHeartbeatTelemetryModule.cs

+        public void Initialize(TelemetryConfiguration configuration)
+        {
+            this.UpdateHeartbeatWithAppServiceEnvVarValues();
+            AppServiceEnvVarMonitor.Instance.MonitoredAppServiceEnvVarUpdatedEvent += this.UpdateHeartbeatWithAppServiceEnvVarValues;


You should only start this monitor if you detected that an application was actually deployed as Azure Web App. Otherwise the timer inside env var monitor will be firing and not do anything ever.

Agreed. I am going to make an assumption that the presence of one of these environment variables means we are running under Azure App Services.

SergeyKanzhelev · 2018-04-04T16:56:01Z

Src/WindowsServer/WindowsServer.Shared/Implementation/AppServiceEnvVarMonitor.cs

+    /// intermittently.
+    /// </summary>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    internal sealed class AppServiceEnvVarMonitor : EnvironmentVariableMonitor


minor: C# guidance on naming says not to use shortened name like EnvVar.

Will update this now, before we go stable. Consistency is good.

SergeyKanzhelev · 2018-04-04T16:59:12Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+            {
+                var kvp = iter.Current;
+
+                string envValue = Environment.GetEnvironmentVariable(kvp.Key);


this may throw exception in partially trusted environment. Which in turn will throw out of timer callback. Throwing from timer callback is bad as CLR may decide that an app crashed due to unhandled exception

Agreed. I capture the SecurityException explicitly and disable the EnvironmentVariableMonitor completely when it occurs. No need to continue generating exceptions/logs for this.

SergeyKanzhelev · 2018-04-04T16:59:50Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+        /// in the configured interval once complete.
+        /// </summary>
+        /// <param name="state">Variable left unused in this implementation of TimerCallback.</param>
+        protected void CheckVariablesIntermittent(object state)


see comment about try{}catch below. Timer methods needs to be safe - I'd suggest global catch for this method

Global try{}catch added, I disable the monitor in the specific case a SecurityException is caught when getting the environment variable value, but I only log a message for any other exception type.

SergeyKanzhelev · 2018-04-04T17:02:43Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+
+            if (this.isEnabled)
+            {
+                this.environmentCheckTimer.Change(this.checkInterval, TimeSpan.FromMilliseconds(-1));


single exception will stop timer and there will be no notification to the customer. This will be hard to troubleshoot.

Logged messages to the WindowsServerEventSource that should help with this.

SergeyKanzhelev · 2018-04-04T17:03:49Z

Src/WindowsServer/WindowsServer.Shared/Implementation/EnvironmentVariableMonitor.cs

+    internal abstract class EnvironmentVariableMonitor : IDisposable
+    {
+        // Environment variables tracked by this monitor.
+        protected readonly ConcurrentDictionary<string, string> CheckedValues;


Why is it a concurrent collection? Do you plan to run multiple instances of timer callback at the same time?

For this abstract class I set out to run it on a single timer callback, and I even guard specifically against re-entrancy with how I use the timer.

However, because it is possible that some subscriber to the event might exist on different threads, I wanted to ensure those cases were covered.

Perhaps I was being over cautious here.

@SergeyKanzhelev Would you like me to change this for this PR?

cijothomas

Please address the comments.

cijothomas · 2018-04-04T16:53:46Z

Src/WindowsServer/WindowsServer.Shared.Tests/AppServicesHeartbeatTelemetryModuleTests.cs

+        {
+            var hbeatProviderMock = new HeartbeatProviderMock();
+            var appSrvHbeatModule = this.GetAppServiceHeartbeatModuleWithUniqueTestEnvVars(hbeatProviderMock);
+            var envVars = this.GetEnvVarsAssociatedToModule(appSrvHbeatModule);


better to move these as TestInitialize

cijothomas · 2018-04-04T16:53:57Z

Src/WindowsServer/WindowsServer.Shared.Tests/AppServicesHeartbeatTelemetryModuleTests.cs

+                Assert.Equal(hbeatProviderMock.HbeatProps[kvp.Key], envVars[kvp.Value]);
+            }
+
+            this.RemoveTestEnvVarsAssociatedToModule(appSrvHbeatModule);


better to move these as TestCleanup

cijothomas · 2018-04-04T16:59:39Z

Src/WindowsServer/WindowsServer.Shared/AppServicesHeartbeatTelemetryModule.cs

+        {
+            try
+            {
+                var hbeatManager = this.GetHeartbeatPropertyManager();


Please do cache. Otherwise we'll be logging 'AppServiceHeartbeatManagerNotAvailable' everytime this is called. (if hbeatmanager is not available)

d3r3kk: Done.

cijothomas · 2018-04-04T17:01:53Z

Src/WindowsServer/WindowsServer.Shared/Implementation/AppServiceEnvVarMonitor.cs

+    /// <summary>
+    /// Utility to monitor the value of environment variables which may change 
+    /// during the run of an application. Checks the environment variables 
+    /// intermittently.


intermittently .> at regular fixed intervals may sound better.

cijothomas · 2018-04-04T17:03:20Z

Src/WindowsServer/WindowsServer.Shared/Implementation/WindowsServerEventSource.cs

+
+        [Event(35,
+            Message = "App Services Heartbeat Provider: Accessing the Hearbeat Manager failed as it is not in the list of available modules.",
+            Level = EventLevel.Informational)]


This could be Warning just like the above is warning.

All these messages are Warning I think.

cijothomas · 2018-04-04T17:25:02Z

Src/WindowsServer/WindowsServer.Shared/AzureWebAppRoleEnvironmentTelemetryInitializer.cs


+        /// <summary>
+        /// Value used for keeping track of when the hostname changes (slot swaps occur). We use this
+        /// to notify the other class that makes use of these environment variables.


misplaced comment?

Yes, removed.

cijothomas · 2018-04-04T17:33:44Z

Src/WindowsServer/WindowsServer.Shared/AzureWebAppRoleEnvironmentTelemetryInitializer.cs


        /// <summary>
-        /// Initializes <see cref="ITelemetry" /> device context.
+        /// Initializes <see cref="ITelemetry" /> device context and helps keep any heartbeat values in sync as well.


This initializer does not have any knowledge of heartbeat right? The comment look incorrect.

Yes, thanks for catching this. Leftover from when I updated it last.

cijothomas · 2018-04-04T17:36:44Z

Src/WindowsServer/WindowsServer.Shared/AppServicesHeartbeatTelemetryModule.cs

+        internal readonly KeyValuePair<string, string>[] WebHeartbeatPropertyNameEnvVarMap = new KeyValuePair<string, string>[]
+        {
+            new KeyValuePair<string, string>("appSrv_SiteName", "WEBSITE_SITE_NAME"),
+            new KeyValuePair<string, string>("appSrv_SiteName", "WEBSITE_SLOT_NAME"),


appSrv_SlotName?

Thanks, fixed.

cijothomas · 2018-04-04T17:39:19Z

Src/WindowsServer/WindowsServer.Shared.Tests/EnvironmentVariableMonitorTests.cs

+        [TestMethod]
+        public void EnsureEnvironmentVariablesAreCapturedImmediately()
+        {
+            var envVars = GetCurrentAppServiceEnvironmentVariableValues();


No cleanup of ENV vars?

Thanks for the catch. Oversight on my part (did it in one place, not the other). Updated to use [TestInitialize] and [TestCleanup] here as well.

cijothomas · 2018-04-04T17:42:27Z

...ndowsServer/WindowsServer.Shared.Tests/AzureWebAppRoleEnvironmentTelemetryInitializerTest.cs

        {
-            Environment.SetEnvironmentVariable("WEBSITE_HOSTNAME", "TestRoleName.azurewebsites.net");
-
+            var testVarName = "WEBSITE_" + Guid.NewGuid().ToString() + "_HOSTNAME";


Why do we change the ENV vars?

We now have 3 classes that all use the same environment variables and testing each of them means we update/clear/change that small set of environment variables as we go. If we run the tests in parallel, tests will have that value changed unexpectedly during their execution by other tests.

…re App Service environment

… guidance on not abbreviating names

…t variable monitor. - On SecurityExceptions, disable the monitor altogether

- Change logged event level for heartbeat manager missing from Informational to Warning - for environment variable monitor tests, update test classes to have a test init/cleanup that set up the environment

d3r3kk · 2018-04-04T23:03:08Z

Build failure exceeding time limit of 60 minutes...

cijothomas · 2018-04-04T23:59:27Z

Src/WindowsServer/WindowsServer.Shared/Implementation/AppServiceEnvironmentVariableMonitor.cs

+        };
+
+        // singleton pattern, this is the one instance of this class allowed
+        private static readonly AppServiceEnvironmentVariableMonitor SingletonInstance = new AppServiceEnvironmentVariableMonitor();


Is it possible to avoid this static initialization? If the app is not running on WebApp, then the entire effort of this class ctr() and reading multiple env vars is wasted and leading to increasing application startup time. This maybe very minimal but .net core apps could be running in containers and restarted very often.
Perhaps move the entire logic to a lazy initializer?
(Lets track it as separate issue)

Agreed. Let's track it in a separate issue. I've added it as #875.

cijothomas · 2018-04-05T00:00:34Z

Src/WindowsServer/WindowsServer.Shared/Implementation/WindowsServerEventSource.cs


+        [Event(33,
+            Message = "App Services Heartbeat Provider: Failed to obtain Azure App Services environment variable '{0}'. Exception raised: {1}",
+            Level = EventLevel.Informational)]


d3r3kk · 2018-04-05T02:31:29Z

@cijothomas I have altered the AI_WebSDK_All-external build to time out at 90 minutes, we were pushing the upper limit of that build prior to this change. Let's discuss ways we can reduce this going forward, perhaps parallelize the builds across a few machines?

cijothomas · 2018-04-05T16:18:32Z

@d3r3kk AI_WebSDK_All-external takes a long time because it has to download docker images everytime as it runs on 'throw away' machine. Not much we can do here. The internal builds are fast enough. -external is meant for PR from forked repos only. (which are rare)

- Using signed package based off of PR microsoft/ApplicationInsights-dotnet-server#873

d3r3kk added 21 commits March 26, 2018 15:45

Add heartbeat provider for Azure App Services (in particular, Web App…

514aa15

…s) that will add some infrastructure-identifying properties. - Implements part of Option 1 in issue #868

Fix stylecop issues

f2bd436

Update for PR suggestions

53fba65

- make new AppServicesHeartbeat module public! - add try/catch scenarios around initialize - add further diagnostic messages where appropriate

Fix for build - stylecop

710d4bb

Add monitor to keep track of current value of App Services env vars

d4a21c3

- Use to keep the value of WEBSITE_SITE_NAME and other App Services values current during runime

Add tests, clean up AppServiceHearbeatModule class (unused variables)…

0c261da

…, update code for testability.

Fix for installed modules count test

fc44730

Correct perf issues and suggestions from PR, add tests

aab6721

Make container of env vars cached into a readonly member

86911c0

Update AzureWebAppRoleEnvironmentTelemetryInitializer to use the AppS…

9c407f9

…erviceEnvVar monitor

Re-add the WindowsServer.Shared.Tests shared project into the Windows…

3f31500

…Server.Net45.Tests project

Merge PR #869 and #870 into a single PR for submission to 2.6.0-beta3

cb857fb

Fix tests to allow for parallel testing of methods that alter environ…

1bbe3a2

…ment variables

Finish merging PR #869 and #870

da7e4d0

- Use env var monitor to check for udpates to environment in controlled/minimal fashion - default to 30 second interval for checking the changes to environment - corrections for stylecop

Cleanup of some comments and usings

275271a

Update file name for tests to match module being tested

09a28fb

- PR suggestion

Construct tests for the subscriber-model environment variable monitor

571f92e

- Tests center around the base class - No unit tests should be indeterminant

d3r3kk requested review from Dmitry-Matveev, SergeyKanzhelev, TimothyMothra and cijothomas April 3, 2018 17:19

d3r3kk self-assigned this Apr 3, 2018

d3r3kk added this to the 2.6-Beta3 milestone Apr 3, 2018

Update tests for AzureWebRoleEnvironmentTelemetryInitializer module

8a1c079

- ensure tests read a special environment variable specific to each test - Ensure parallel tests do not impede ability to test this class (Now that we have a monitor in place, we need to make the tests get around it)

pharring reviewed Apr 3, 2018

View reviewed changes

PR suggestion: Correct access level of abstract class EnvironmentVari…

43d6bc8

…ableMonitor to be protected

Dmitry-Matveev reviewed Apr 3, 2018

View reviewed changes

d3r3kk added 2 commits April 3, 2018 23:34

PR Suggestions

3f9cc36

Fix PR CI Build break due to StyleCop rules

87c1fd7

d3r3kk added a commit to microsoft/ApplicationInsights-aspnetcore that referenced this pull request Apr 4, 2018

Update PR with packages only from MyGet

7412890

- Using signed package based off of PR microsoft/ApplicationInsights-dotnet-server#873

d3r3kk mentioned this pull request Apr 4, 2018

Ensure Heartbeat properties for App Services is included in AspNetCore microsoft/ApplicationInsights-aspnetcore#627

Merged

3 tasks

SergeyKanzhelev reviewed Apr 4, 2018

View reviewed changes

cijothomas approved these changes Apr 4, 2018

View reviewed changes

d3r3kk added 5 commits April 4, 2018 12:10

PR Suggestions: Disable monitor if we don't detect that we are in Azu…

74cb7d5

…re App Service environment

PR Suggestion: Correct AppServicesEnvVarMonitor to properly follow C#…

13715bc

… guidance on not abbreviating names

PR Suggestion: Handle exceptions during timer callback for environmen…

d31e0e9

…t variable monitor. - On SecurityExceptions, disable the monitor altogether

PR suggestions/fixes for tests and log warnings

50e876f

- Change logged event level for heartbeat manager missing from Informational to Warning - for environment variable monitor tests, update test classes to have a test init/cleanup that set up the environment

Ensure test for updated environment values operates correctly

be1ffe6

d3r3kk closed this Apr 4, 2018

d3r3kk reopened this Apr 4, 2018

cijothomas reviewed Apr 4, 2018

View reviewed changes

cijothomas reviewed Apr 5, 2018

View reviewed changes

d3r3kk mentioned this pull request Apr 5, 2018

Enable lazy initialization and checks for App Services environment in AppServicesEnvironmentVariableMonitor class. #875

Closed

PR Suggestion: Change informational message to warning.

c61fb45

d3r3kk merged commit 1582dfb into develop Apr 5, 2018

d3r3kk deleted the dekeeler/appsrv-heartbeat-timed_envmonitor branch April 5, 2018 04:14

d3r3kk mentioned this pull request Apr 5, 2018

Add Heartbeat properties for App Services #869

Closed

4 tasks

d3r3kk added a commit to microsoft/ApplicationInsights-aspnetcore that referenced this pull request Apr 7, 2018

Update PR with packages only from MyGet

be3bd3d

- Using signed package based off of PR microsoft/ApplicationInsights-dotnet-server#873

Conversation

d3r3kk commented Apr 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d3r3kk commented Apr 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dmitry-Matveev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cijothomas Apr 4, 2018 • edited by d3r3kk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3r3kk Apr 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cijothomas left a comment

d3r3kk commented Apr 3, 2018 •

edited

Loading

cijothomas Apr 4, 2018 •

edited by d3r3kk

Loading

d3r3kk Apr 4, 2018 •

edited

Loading

cijothomas Apr 4, 2018 •

edited by d3r3kk

Loading