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.

Sign up for GitHub

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

[fleet installer] Add additional logging for app pool modifications #6700

Merged
merged 1 commit into from
Feb 25, 2025
Merged

[fleet installer] Add additional logging for app pool modifications #6700

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 31 additions & 15 deletions tracer/src/Datadog.FleetInstaller/AppHostHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static bool RemoveAllEnvironmentVariables(ILogger log)
private static bool ModifyEnvironmentVariablesWithRetry(
ILogger log,
ReadOnlyDictionary<string, string> envVars,
Action<ConfigurationElementCollection, ReadOnlyDictionary<string, string>> updateEnvVars)
Action<ILogger, ConfigurationElementCollection, ReadOnlyDictionary<string, string>> updateEnvVars)
{
// If the IIS host config is being modified, this may fail
// We retry multiple times, as the final update is atomic
Expand All @@ -57,7 +57,7 @@ private static bool ModifyEnvironmentVariablesWithRetry(
private static bool ModifyEnvironmentVariables(
ILogger log,
ReadOnlyDictionary<string, string> envVars,
Action<ConfigurationElementCollection, ReadOnlyDictionary<string, string>> updateEnvVars)
Action<ILogger, ConfigurationElementCollection, ReadOnlyDictionary<string, string>> updateEnvVars)
{
if (!SetEnvironmentVariables(log, envVars, updateEnvVars, out var appPoolsWeMustReenableRecycling))
{
Expand All @@ -73,7 +73,7 @@ private static bool ModifyEnvironmentVariables(
private static bool SetEnvironmentVariables(
ILogger log,
ReadOnlyDictionary<string, string> envVars,
Action<ConfigurationElementCollection, ReadOnlyDictionary<string, string>> updateEnvVars,
Action<ILogger, ConfigurationElementCollection, ReadOnlyDictionary<string, string>> updateEnvVars,
out HashSet<string> appPoolsWeMustReenableRecycling)
{
appPoolsWeMustReenableRecycling = [];
Expand All @@ -92,8 +92,8 @@ private static bool SetEnvironmentVariables(
var (applicationPoolDefaults, applicationPoolsCollection) = appPoolsSection.Value;

// Update defaults
log.WriteInfo($"Updating applicationPoolDefaults environment variables");
updateEnvVars(applicationPoolDefaults.GetCollection("environmentVariables"), envVars);
log.WriteInfo("Updating applicationPoolDefaults environment variables");
updateEnvVars(log, applicationPoolDefaults.GetCollection("environmentVariables"), envVars);

// Update app pools
foreach (var appPoolElement in applicationPoolsCollection)
Expand All @@ -116,16 +116,17 @@ private static bool SetEnvironmentVariables(
// if it was set to "true", as we don't want to accidentally revert that later.
if (appPoolElement.GetChildElement("recycling")["disallowRotationOnConfigChange"] as bool? ?? false)
{
// already disallowed, which is what we want, so don't need to modify it now _or_ later
log.WriteInfo($"App pool '{poolName}' recycling.disallowRotationOnConfigChange already set to true, skipping");
}
else
{
log.WriteInfo($"Setting app pool '{poolName}' recycling.disallowRotationOnConfigChange=true");
appPoolsWeMustReenableRecycling.Add(poolName);
appPoolElement.GetChildElement("recycling")["disallowRotationOnConfigChange"] = true;
}

// Set the pool-specific env variables
updateEnvVars(appPoolElement.GetCollection("environmentVariables"), envVars);
updateEnvVars(log, appPoolElement.GetCollection("environmentVariables"), envVars);
}
}

Expand Down Expand Up @@ -156,11 +157,17 @@ private static bool ReEnableRecycling(ILogger log, HashSet<string> appPoolsWhich
foreach (var appPoolElement in appPoolsSection.Value.AppPools)
{
if (string.Equals(appPoolElement.ElementTagName, "add", StringComparison.OrdinalIgnoreCase)
&& appPoolElement.GetAttributeValue("name") is string poolName
&& appPoolsWhichNeedToAllowRecycling.Contains(poolName))
&& appPoolElement.GetAttributeValue("name") is string poolName)
{
log.WriteInfo($"Re-enabling rotation on config change for app pool '{poolName}'");
appPoolElement.GetChildElement("recycling")["disallowRotationOnConfigChange"] = false;
if (appPoolsWhichNeedToAllowRecycling.Contains(poolName))
{
log.WriteInfo($"Re-enabling rotation on config change for app pool '{poolName}'");
appPoolElement.GetChildElement("recycling")["disallowRotationOnConfigChange"] = false;
}
else
{
log.WriteInfo($"Skipping re-enabling rotation for app pool '{poolName}'");
}
}
}

Expand All @@ -170,7 +177,7 @@ private static bool ReEnableRecycling(ILogger log, HashSet<string> appPoolsWhich
}
catch (Exception ex)
{
log.WriteError(ex, $"Error re-enabling application pool recycling");
log.WriteError(ex, "Error re-enabling application pool recycling");
return false;
}
}
Expand Down Expand Up @@ -209,7 +216,7 @@ private static (ConfigurationElement AppPoolDefaults, ConfigurationElementCollec
return (applicationPoolDefaults, applicationPoolsCollection);
}

private static void SetEnvVars(ConfigurationElementCollection envVars, ReadOnlyDictionary<string, string> requiredVariables)
private static void SetEnvVars(ILogger log, ConfigurationElementCollection envVars, ReadOnlyDictionary<string, string> requiredVariables)
{
// Try to find the value we need
// Update all the values we need
Expand All @@ -228,21 +235,28 @@ private static void SetEnvVars(ConfigurationElementCollection envVars, ReadOnlyD
continue;
}

// Should we record the previous value (so it's available in the logs later if necessary?)
// I opted not to, to avoid the risk of leaking anything sensitive. Currently there _shouldn't_
// be anything sensitive in there, but there _could_ be theoretically.
log.WriteInfo($"Found existing value for '{key}' - replacing value");
envVarEle["value"] = envVarValue;
// log.WriteInfo($"Updated environment variable {key} to {envVarValue}");
remainingValues.Remove(key);
}

foreach (var kvp in remainingValues)
{
// Similarly, not showing the value we're setting here. It isn't a sensitive value now, but
// that won't always be the case, so this avoids the risk. Obviously that's less useful
// for troubleshooting unfortunately.
log.WriteInfo($"Adding new variable '{kvp.Key}'");
Comment on lines +248 to +251
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

var addEle = envVars.CreateElement("add");
addEle["name"] = kvp.Key;
addEle["value"] = kvp.Value;
envVars.Add(addEle);
}
}

private static void RemoveEnvVars(ConfigurationElementCollection envVars, ReadOnlyDictionary<string, string> requiredVariables)
private static void RemoveEnvVars(ILogger log, ConfigurationElementCollection envVars, ReadOnlyDictionary<string, string> requiredVariables)
{
// Try to find the value we need
// Update all the values we need
Expand All @@ -262,6 +276,7 @@ private static void RemoveEnvVars(ConfigurationElementCollection envVars, ReadOn
}

envVarsToRemove ??= new();
log.WriteInfo($"Adding variable '{key}' to removal list");
envVarsToRemove.Add(envVarEle);
}

Expand All @@ -270,6 +285,7 @@ private static void RemoveEnvVars(ConfigurationElementCollection envVars, ReadOn
return;
}

log.WriteInfo($"Removing {envVarsToRemove.Count} variables");
foreach (var element in envVarsToRemove)
{
envVars.Remove(element);
Expand Down
Loading