Skip to content

Commit

Permalink
SNOW-990111: Add easy logging improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-ext-simba-lf committed Jan 13, 2024
1 parent 3ccdab3 commit 5bd6504
Show file tree
Hide file tree
Showing 6 changed files with 186 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public class EasyLoggingConfigFinderTest
{
private const string InputConfigFilePath = "input_config.json";
private const string EnvironmentalConfigFilePath = "environmental_config.json";
private const string HomeDirectory = "/home/user";
private static readonly string HomeDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
private static readonly string s_driverConfigFilePath = Path.Combine(".", EasyLoggingConfigFinder.ClientConfigFileName);
private static readonly string s_homeConfigFilePath = Path.Combine(HomeDirectory, EasyLoggingConfigFinder.ClientConfigFileName);
private static readonly string s_tempConfigFilePath = Path.Combine(Path.GetTempPath(), EasyLoggingConfigFinder.ClientConfigFileName);
Expand Down Expand Up @@ -104,7 +104,20 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN
}

[Test]
public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible()
public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible()
{
// arrange
MockFileOnHomePath();

// act
var filePath = t_finder.FindConfigFilePath(null);

// assert
Assert.AreEqual(s_homeConfigFilePath, filePath);
}

[Test]
public void TestThatDoesNotTakeFilePathFromTempDirectoryWhenNoOtherWaysPossible()
{
// arrange
MockFileOnTempPath();
Expand All @@ -113,7 +126,7 @@ public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible()
var filePath = t_finder.FindConfigFilePath(null);

// assert
Assert.AreEqual(s_tempConfigFilePath, filePath);
Assert.Null(filePath);
}

[Test]
Expand Down
20 changes: 20 additions & 0 deletions Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using NUnit.Framework;
using Snowflake.Data.Configuration;
using Snowflake.Data.Core;
Expand Down Expand Up @@ -92,6 +93,25 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly()
Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(FatalMessage)));
}

[Test]
//[Ignore("This test requires manual interaction and therefore cannot be run in CI")]
public void TestThatPermissionsFollowUmask()
{
// Note: To test with a different value than the default umask, it will have to be set before running this test
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
// arrange
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Debug, t_directoryLogPath);

// act
var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask"));
var dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {t_directoryLogPath}");

// assert
Assert.IsTrue(umask >= int.Parse(dirPermissions));
}
}

private static string RandomLogsDirectoryPath()
{
var randomName = Path.GetRandomFileName();
Expand Down
65 changes: 54 additions & 11 deletions Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

using System;
using System.IO;
using System.Runtime.InteropServices;
using Snowflake.Data.Client;
using Snowflake.Data.Core;
using Snowflake.Data.Core.Tools;
using Snowflake.Data.Log;

Expand Down Expand Up @@ -33,24 +36,36 @@ internal EasyLoggingConfigFinder()

public virtual string FindConfigFilePath(string configFilePathFromConnectionString)
{
return GetFilePathFromInputParameter(configFilePathFromConnectionString)
return GetFilePathFromInputParameter(configFilePathFromConnectionString, "connection string")
?? GetFilePathEnvironmentVariable()
?? GetFilePathFromDriverLocation()
?? GetFilePathFromHomeDirectory()
?? GetFilePathFromTempDirectory();
?? GetFilePathFromHomeDirectory();
}

private string GetFilePathEnvironmentVariable()
{
var filePath = _environmentOperations.GetEnvironmentVariable(ClientConfigEnvironmentName);
return GetFilePathFromInputParameter(filePath);
return GetFilePathFromInputParameter(filePath, "environment variable");
}

private string GetFilePathFromTempDirectory() => SearchForConfigInDirectory(Path.GetTempPath, "temp");

private string GetFilePathFromHomeDirectory() => SearchForConfigInDirectory(GetHomeDirectory, "home");

private string GetFilePathFromInputParameter(string filePath) => string.IsNullOrEmpty(filePath) ? null : filePath;

private string GetFilePathFromInputParameter(string filePath, string inputDescription)
{
if (!string.IsNullOrEmpty(filePath))
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
CheckIfValidPermissions(filePath);
}

Check warning on line 60 in Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs#L58-L60

Added lines #L58 - L60 were not covered by tests
s_logger.Info($"Using config file specified from {inputDescription}");
return filePath;
}
else
{
return null;
}
}

private string GetHomeDirectory() =>_environmentOperations.GetFolderPath(Environment.SpecialFolder.UserProfile);

Expand All @@ -61,13 +76,13 @@ private string SearchForConfigInDirectory(Func<string> directoryProvider, string
try
{
var directory = directoryProvider.Invoke();
if (string.IsNullOrEmpty(directory))
if (string.IsNullOrEmpty(directory) || !Directory.Exists(directory))
{
return null;
}

var filePath = Path.Combine(directory, ClientConfigFileName);
return OnlyIfFileExists(filePath);
return OnlyIfFileExists(filePath, directoryDescription);
}
catch (Exception e)
{
Expand All @@ -76,6 +91,34 @@ private string SearchForConfigInDirectory(Func<string> directoryProvider, string
}
}

private string OnlyIfFileExists(string filePath) => _fileOperations.Exists(filePath) ? filePath : null;
private string OnlyIfFileExists(string filePath, string directoryDescription)
{
if (_fileOperations.Exists(filePath))
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
CheckIfValidPermissions(filePath);
}

Check warning on line 101 in Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs#L99-L101

Added lines #L99 - L101 were not covered by tests
s_logger.Info($"Using config file specified from {directoryDescription} directory");
return filePath;
}
else
{
return null;
}
}

private void CheckIfValidPermissions(string filePath)
{

Check warning on line 112 in Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs#L112

Added line #L112 was not covered by tests
// Check if others have permissions to modify the file and fail if so
string filePermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {filePath}");

Check warning on line 114 in Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs#L114

Added line #L114 was not covered by tests
if (int.Parse(filePermissions) > EasyLoggerUtil.OnlyUserHasPermissionToWrite)
{
s_logger.Error($"Error due to other users having permission to modify the config file");
throw new SnowflakeDbException(
SFError.INTERNAL_ERROR,
"The config file is modifiable by other users and will not be used.");

Check warning on line 120 in Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs#L116-L120

Added lines #L116 - L120 were not covered by tests
}
}

Check warning on line 122 in Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigFinder.cs#L122

Added line #L122 was not covered by tests
}
}
28 changes: 28 additions & 0 deletions Snowflake.Data/Configuration/EasyLoggingConfigParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

using System;
using System.IO;
using System.Reflection;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Snowflake.Data.Log;

namespace Snowflake.Data.Configuration
Expand Down Expand Up @@ -44,6 +46,7 @@ private ClientConfig TryToParseFile(string fileContent)
try {
var config = JsonConvert.DeserializeObject<ClientConfig>(fileContent);
Validate(config);
CheckForUnknownFields(fileContent, config);
return config;
}
catch (Exception e)
Expand All @@ -61,5 +64,30 @@ private void Validate(ClientConfig config)
EasyLoggingLogLevelExtensions.From(config.CommonProps.LogLevel);
}
}

private void CheckForUnknownFields(string fileContent, ClientConfig config)
{
// Parse the specified config file and get the key-value pairs from the "common" section
JObject obj = (JObject)(JObject.Parse(fileContent).First.First);
bool isUnknownField = true;
foreach (var keyValuePair in obj)
{
foreach(var property in config.CommonProps.GetType().GetProperties())
{
var jsonPropertyAttribute = property.GetCustomAttribute<JsonPropertyAttribute>();
if (keyValuePair.Key.Equals(jsonPropertyAttribute.PropertyName))
{
isUnknownField = false;
break;
}
}
if (isUnknownField)
{
s_logger.Warn($"Unknown field from config: {keyValuePair.Key}");
}

Check warning on line 87 in Snowflake.Data/Configuration/EasyLoggingConfigParser.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Configuration/EasyLoggingConfigParser.cs#L85-L87

Added lines #L85 - L87 were not covered by tests

isUnknownField = true;
}
}
}
}
40 changes: 38 additions & 2 deletions Snowflake.Data/Core/Session/EasyLoggingStarter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
* Copyright (c) 2023 Snowflake Computing Inc. All rights reserved.
*/

using System;
using System.IO;
using System.Runtime.InteropServices;
using System.Security.AccessControl;
using System.Security.Principal;
using Snowflake.Data.Client;
using Snowflake.Data.Configuration;
using Snowflake.Data.Core.Tools;
using Snowflake.Data.Log;
Expand Down Expand Up @@ -42,6 +47,15 @@ internal EasyLoggingStarter()

public virtual void Init(string configFilePathFromConnectionString)
{
if (string.IsNullOrEmpty(configFilePathFromConnectionString))
{
s_logger.Info($"Attempting to enable easy logging without a config file specified from connection string");
}
else
{
s_logger.Info($"Attempting to enable easy logging using config file specified from connection string: {configFilePathFromConnectionString}");
}

lock (_lockForExclusiveInit)
{
if (!AllowedToInitialize(configFilePathFromConnectionString))
Expand All @@ -56,6 +70,8 @@ public virtual void Init(string configFilePathFromConnectionString)
}
var logLevel = GetLogLevel(config.CommonProps.LogLevel);
var logPath = GetLogPath(config.CommonProps.LogPath);
s_logger.Info($"LogLevel set to {logLevel}");
s_logger.Info($"LogPath set to {logPath}");
_easyLoggerManager.ReconfigureEasyLogging(logLevel, logPath);
_initTrialParameters = new EasyLoggingInitTrialParameters(configFilePathFromConnectionString);
}
Expand Down Expand Up @@ -90,13 +106,33 @@ private string GetLogPath(string logPath)
var logPathOrDefault = logPath;
if (string.IsNullOrEmpty(logPath))
{
s_logger.Warn("LogPath in client config not found. Using temporary directory as a default value");
logPathOrDefault = Path.GetTempPath();
s_logger.Warn("LogPath in client config not found. Using home directory as a default value");
logPathOrDefault = EnvironmentOperations.Instance.GetFolderPath(Environment.SpecialFolder.UserProfile);

Check warning on line 110 in Snowflake.Data/Core/Session/EasyLoggingStarter.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Session/EasyLoggingStarter.cs#L109-L110

Added lines #L109 - L110 were not covered by tests
if (string.IsNullOrEmpty(logPathOrDefault))
{
throw new SnowflakeDbException(
SFError.INTERNAL_ERROR,
"No log path found for easy logging. Home directory is not configured and log path is not provided.");

Check warning on line 115 in Snowflake.Data/Core/Session/EasyLoggingStarter.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Session/EasyLoggingStarter.cs#L112-L115

Added lines #L112 - L115 were not covered by tests
}
}
var pathWithDotnetSubdirectory = Path.Combine(logPathOrDefault, "dotnet");
if (!_directoryOperations.Exists(pathWithDotnetSubdirectory))
{
_directoryOperations.CreateDirectory(pathWithDotnetSubdirectory);

if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
var umask = EasyLoggerUtil.AllPermissions - int.Parse(EasyLoggerUtil.CallBash("umask"));
string dirPermissions = EasyLoggerUtil.CallBash($"stat -c '%a' {pathWithDotnetSubdirectory}");

Check warning on line 126 in Snowflake.Data/Core/Session/EasyLoggingStarter.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Session/EasyLoggingStarter.cs#L124-L126

Added lines #L124 - L126 were not covered by tests
if (int.Parse(dirPermissions) > umask)
{
EasyLoggerUtil.CallBash($"chmod -R {EasyLoggerUtil.AllUserPermissions} {pathWithDotnetSubdirectory}");
}

Check warning on line 130 in Snowflake.Data/Core/Session/EasyLoggingStarter.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Session/EasyLoggingStarter.cs#L128-L130

Added lines #L128 - L130 were not covered by tests
if (int.Parse(dirPermissions) != EasyLoggerUtil.AllUserPermissions)
{
s_logger.Warn($"Access permission for the logs directory is {dirPermissions}");
}
}

Check warning on line 135 in Snowflake.Data/Core/Session/EasyLoggingStarter.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Session/EasyLoggingStarter.cs#L132-L135

Added lines #L132 - L135 were not covered by tests
}

return pathWithDotnetSubdirectory;
Expand Down
30 changes: 30 additions & 0 deletions Snowflake.Data/Logger/EasyLoggerUtil.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2024 Snowflake Computing Inc. All rights reserved.
*/

using System.Diagnostics;

namespace Snowflake.Data.Log
{
internal class EasyLoggerUtil
{
internal static int AllPermissions = 777;

internal static int AllUserPermissions = 700;

internal static int OnlyUserHasPermissionToWrite = 644;

internal static string CallBash(string command)
{
using (Process process = new Process())
{
process.StartInfo.FileName = "/bin/bash";
process.StartInfo.Arguments = $"-c \"{command}\"";
process.StartInfo.UseShellExecute = false;
process.StartInfo.RedirectStandardOutput = true;
process.Start();
return process.StandardOutput.ReadToEnd().Replace("\n", string.Empty);

Check warning on line 26 in Snowflake.Data/Logger/EasyLoggerUtil.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Logger/EasyLoggerUtil.cs#L18-L26

Added lines #L18 - L26 were not covered by tests
}
}

Check warning on line 28 in Snowflake.Data/Logger/EasyLoggerUtil.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Logger/EasyLoggerUtil.cs#L28

Added line #L28 was not covered by tests
}
}

0 comments on commit 5bd6504

Please sign in to comment.