Skip to content
This repository was archived by the owner on Jul 5, 2020. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ public void Initialize(ITelemetry telemetry)

if (string.IsNullOrEmpty(telemetry.Context.GetInternalContext().NodeName))
{
string name = LazyInitializer.EnsureInitialized(ref this.nodeName, this.GetNodeName);
telemetry.Context.GetInternalContext().NodeName = name;
telemetry.Context.GetInternalContext().NodeName = this.GetNodeName();
}
}

Expand All @@ -62,7 +61,8 @@ private string GetRoleName()

private string GetNodeName()
{
return Environment.GetEnvironmentVariable(WebAppHostNameEnvironmentVariable) ?? string.Empty;
AppServiceEnvVarMonitor.GetUpdatedEnvironmentVariable(WebAppHostNameEnvironmentVariable, ref this.nodeName);
return this.nodeName;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
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
{
internal static TimeSpan CheckInterval = TimeSpan.FromSeconds(30);
Copy link
Copy Markdown
Member

@pharring pharring Mar 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: private static readonly (unless you need to change it in unit tests.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

internal static DateTime LastCheckTime = DateTime.MinValue;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: private (unless you need to change it in unit tests.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckInterval can be private, NextCheckTime internal to allow tests to work - added a line comment to call this intention out.


/// <summary>
/// Environment variables tracked by this monitor.
/// </summary>
internal static Dictionary<string, string> CheckedValues = new Dictionary<string, string>()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: private static readonly (unless you need to change it in unit tests.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, done. I am always thrown by the readonly keyword with containers...

{
{ "WEBSITE_SITE_NAME", string.Empty },
{ "WEBSITE_HOME_STAMPNAME", string.Empty },
{ "WEBSITE_HOSTNAME", string.Empty }
Copy link
Copy Markdown
Member

@pharring pharring Mar 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: remove extra space (alignment)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, this was breaking the build (stylecop).

};

/// <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 checkTime = DateTime.Now;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always use DateTime.UtcNow to avoid an expensive local timezone lookup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - good tip, thanks.

if (checkTime - LastCheckTime > CheckInterval)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For bonus perf points, instead of LastCheckTime compute and store NextCheckTime (i.e. the time after which you should check again) and avoid the subtraction in the common case and make it an addition in the rare case.
i.e.

DateTime now = DateTime.UtcNow;
if (now > NextCheckTime)
{
    NextCheckTime = now + CheckInterval;
   // etc.
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Done.

{
LastCheckTime = checkTime;
foreach (var kvp in CheckedValues)
{
CheckedValues[kvp.Key] = Environment.GetEnvironmentVariable(kvp.Key);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<Compile Include="$(MSBuildThisFileDirectory)BuildInfoConfigComponentVersionTelemetryInitializer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)DeveloperModeWithDebuggerAttachedTelemetryModule.cs" />
<Compile Include="$(MSBuildThisFileDirectory)DomainNameRoleInstanceTelemetryInitializer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Implementation\AppServiceEnvVarMonitor.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Implementation\AzureComputeMetadataHeartbeatPropertyProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Implementation\AzureMetadataRequestor.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Implementation\DataContracts\AzureInstanceComputeMetadata.cs" />
Expand Down