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.

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

SNOW-834781: Remove log4net and delegate logging to consumers #1057

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
650bb43
SNOW-834781: Remove log4net and delegate logging to consumers
sfc-gh-ext-simba-lf Nov 9, 2024
f0484f6
SNOW-834781: Refactor/add tests
sfc-gh-ext-simba-lf Nov 9, 2024
83e56ef
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Nov 9, 2024
0ab8c66
Set config from code
sfc-gh-ext-simba-lf Nov 12, 2024
15090b4
Use constant file names for tests
sfc-gh-ext-simba-lf Nov 14, 2024
1b80c55
Remove unused method
sfc-gh-ext-simba-lf Nov 18, 2024
1a0a0f6
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Nov 19, 2024
00d0e39
Add tests for home directory tool
sfc-gh-ext-simba-lf Nov 19, 2024
1036d56
Re-add Easy Logging
sfc-gh-ext-simba-lf Nov 21, 2024
3f3a15c
Refactor original logger to simple logger
sfc-gh-ext-simba-lf Nov 21, 2024
188f82a
Add initial impl. for own Snowflake logger
sfc-gh-ext-simba-lf Nov 29, 2024
0ec8b4d
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Nov 29, 2024
26ef937
Fix log
sfc-gh-ext-simba-lf Nov 29, 2024
1c7afd3
Add tests for new log functions
sfc-gh-ext-simba-lf Nov 30, 2024
1d4e732
Retrigger checks
sfc-gh-ext-simba-lf Nov 30, 2024
c1b8d5a
Retrigger checks
sfc-gh-ext-simba-lf Dec 2, 2024
2fe82b1
Refactor classes and tests
sfc-gh-ext-simba-lf Dec 3, 2024
7bb3f98
Rename log functions and add test for BeginScope
sfc-gh-ext-simba-lf Dec 4, 2024
f1fb9c2
Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…
sfc-gh-ext-simba-lf Dec 4, 2024
c9d8d92
Refactor logger functions add logger pair class
sfc-gh-ext-simba-lf Dec 5, 2024
f0da3e7
Replace ILogger with logger pair
sfc-gh-ext-simba-lf Dec 5, 2024
7e8af5c
Fix format error with bracket
sfc-gh-ext-simba-lf Dec 5, 2024
d6090aa
Move masking secrets
sfc-gh-ext-simba-lf Dec 6, 2024
a7535dc
Fix typo
sfc-gh-ext-simba-lf Dec 6, 2024
26a5d53
Refactor SFLoggerPair
sfc-gh-ext-simba-lf Dec 6, 2024
247df19
Replace ILogger with SFLogger in tests
sfc-gh-ext-simba-lf Dec 6, 2024
44ab254
Remove unused package
sfc-gh-ext-simba-lf Dec 6, 2024
e1a97f5
Revert BOM
sfc-gh-ext-simba-lf Dec 7, 2024
7d4c4ae
Remove unused package and revert BOM
sfc-gh-ext-simba-lf Dec 7, 2024
9038aaa
Revert logger name
sfc-gh-ext-simba-lf Dec 7, 2024
d2f94f4
Revert logger name
sfc-gh-ext-simba-lf Dec 7, 2024
6affd0a
Revert logger name
sfc-gh-ext-simba-lf Dec 7, 2024
c4d358a
Revert BOM
sfc-gh-ext-simba-lf Dec 7, 2024
b9e74da
Revert to original
sfc-gh-ext-simba-lf Dec 7, 2024
956249e
Revert eof
sfc-gh-ext-simba-lf Dec 7, 2024
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
21 changes: 21 additions & 0 deletions Snowflake.Data.Tests/Snowflake.Data.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@
<ItemGroup>
<PackageReference Include="JunitXml.TestLogger" Version="3.1.12" />
<PackageReference Include="Moq" Version="4.18.4" />
<PackageReference Include="Microsoft.Extensions.Logging.Log4Net.AspNetCore" Version="8.0.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.1" />
<PackageReference Include="NLog.Extensions.Logging" Version="5.3.14" />
<PackageReference Include="NUnit" Version="3.14.0" />
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
<PackageReference Include="RichardSzalay.MockHttp" Version="6.0.0" />
<PackageReference Include="Serilog" Version="4.0.1" />
<PackageReference Include="Serilog.Extensions.Logging" Version="8.0.0" />
<PackageReference Include="Serilog.Settings.Xml" Version="1.1.0" />
<PackageReference Include="Serilog.Sinks.File" Version="6.0.0" />
<PackageReference Include="System.Net.Http" Version="4.3.4" />
<PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />
<PackageReference Include="Tomlyn.Signed" Version="0.17.0" />
Expand All @@ -36,6 +42,21 @@
<Reference Include="System.Net.Http.WebRequest" />
</ItemGroup>

<ItemGroup>
<None Update="log4net.config">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Update="TestSerilog.config">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Update="TestLog4Net.config">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
<None Update="TestNLog.config">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</None>
</ItemGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
<DebugType>full</DebugType>
<DebugSymbols>True</DebugSymbols>
Expand Down
20 changes: 20 additions & 0 deletions Snowflake.Data.Tests/TestLog4Net.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="utf-8" ?>
<log4net>
<appender name="MyRollingFileAppender" type="log4net.Appender.RollingFileAppender">
<file value="${TEST_LOG4NET_FILE_NAME}"/>
<appendToFile value="true"/>
<rollingStyle value="Size" />
<maximumFileSize value="10MB" />
<staticLogFileName value="true" />
<maxSizeRollBackups value="10" />
<lockingModel type="log4net.Appender.FileAppender+MinimalLock" />
<layout type="log4net.Layout.PatternLayout">
<conversionPattern value="[%date] [%t] [%-5level] [%logger] %message%newline" />
</layout>
</appender>

<root>
<level value="ALL" />
<appender-ref ref="MyRollingFileAppender" />
</root>
</log4net>
23 changes: 23 additions & 0 deletions Snowflake.Data.Tests/TestNLog.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="utf-8" ?>
<!-- XSD manual extracted from package NLog.Schema: https://www.nuget.org/packages/NLog.Schema-->
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd" xsi:schemaLocation="NLog NLog.xsd"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
autoReload="true"
internalLogLevel="Trace" >

<!-- the targets to write to -->
<targets>
<!-- write logs to file -->
<target xsi:type="File"
name="logfile"
fileName="${environment:TEST_NLOG_FILE_NAME}"
keepFileOpen="false"
concurrentWrites="true"
layout="${longdate}|${level}|${message} |${all-event-properties} ${exception:format=tostring}"/>
</targets>

<!-- rules to map from logger name to target -->
<rules>
<logger name="*" minlevel="Trace" writeTo="logfile" />
</rules>
</nlog>
15 changes: 15 additions & 0 deletions Snowflake.Data.Tests/TestSerilog.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="utf-8" ?>
<serilog>
<using>
<add name="Serilog.Sinks.File" />
</using>

<minimumLevel default="Verbose"></minimumLevel>

<writeTo>
<sink name="File">
<arg name="path" value="test_serilog.log" />
<arg name="shared" value="true" />
</sink>
</writeTo>
</serilog>
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public static void AfterAll()
{
Directory.Delete(s_workingDirectory, true);
}

[Test]
public void TestThatParsesConfigFile()
{
Expand All @@ -59,12 +59,12 @@ public void TestThatParsesConfigFileWithNullValues(string filePath)

// act
var config = parser.Parse(filePath);

// assert
Assert.IsNotNull(config);
Assert.IsNotNull(config.CommonProps);
Assert.IsNull(config.CommonProps.LogLevel);
Assert.IsNull(config.CommonProps.LogPath);
Assert.IsNull(config.CommonProps.LogPath);
}

[Test]
Expand All @@ -77,20 +77,20 @@ public void TestThatReturnsNullWhenNothingToParse(string noFilePath)

// act
var config = parser.Parse(noFilePath);

// assert
Assert.IsNull(config);
}

[Test]
public void TestThatFailsWhenTheFileDoesNotExist()
{
// arrange
var parser = new EasyLoggingConfigParser();

// act
var thrown = Assert.Throws<Exception>(() => parser.Parse(NotExistingFilePath));

// assert
Assert.IsNotNull(thrown);
Assert.AreEqual("Finding easy logging configuration failed", thrown.Message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public class EasyLoggingConfigProviderTest
{
private const string FilePathFromConnectionString = "/Users/dotnet/config.json";
private const string FilePathToUse = "/home/config.json";

[Test]
public void TestThatProvidesConfiguration()
{
Expand All @@ -31,7 +31,7 @@ public void TestThatProvidesConfiguration()

// act
var result = configProvider.ProvideConfig(FilePathFromConnectionString);

// assert
Assert.AreSame(config, result);
}
Expand All @@ -45,11 +45,11 @@ public void TestThatReturnsNullWhenNoConfigurationFound()
var configProvider = new EasyLoggingConfigProvider(configFinder.Object, configParser.Object);
configFinder
.Setup(finder => finder.FindConfigFilePath(FilePathFromConnectionString))
.Returns((string) null);
.Returns((string)null);

// act
var result = configProvider.ProvideConfig(FilePathFromConnectionString);

// assert
Assert.IsNull(result);
configParser.Verify(parser => parser.Parse(It.IsAny<string>()), Times.Never);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public void TestThatGetsLogLevelValueIgnoringLetterCase(string loglevelString, E
{
// act
var logLevel = EasyLoggingLogLevelExtensions.From(loglevelString);

// assert
Assert.AreEqual(expectedLogLevel, logLevel);
}
Expand All @@ -29,7 +29,7 @@ public void TestThatFailsToParseLogLevelFromUnknownValue()
{
// act
var thrown = Assert.Throws<ArgumentException>(() => EasyLoggingLogLevelExtensions.From("unknown"));

// assert
Assert.IsNotNull(thrown);
Assert.AreEqual("Requested value 'unknown' was not found.", thrown.Message);
Expand Down
94 changes: 85 additions & 9 deletions Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Snowflake.Data.Tests.UnitTests.Logger
[TestFixture, NonParallelizable]
public class EasyLoggerManagerTest
{

private const string InfoMessage = "Easy logging Info message";
private const string DebugMessage = "Easy logging Debug message";
private const string WarnMessage = "Easy logging Warn message";
Expand All @@ -26,7 +26,7 @@ public class EasyLoggerManagerTest

[ThreadStatic]
private static string t_directoryLogPath;

[OneTimeTearDown]
public static void CleanUp()
{
Expand All @@ -44,14 +44,46 @@ public void AfterEach()
{
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Warn, t_directoryLogPath);
}

[Test]
public void TestThatChangesLogLevel()
{
// arrange
var logger = SFLoggerFactory.GetLogger<SFBlockingChunkDownloaderV3>();
var logger = SFLoggerFactory.GetSFLogger<SFBlockingChunkDownloaderV3>();

// act
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Off, t_directoryLogPath);

// assert
Assert.IsFalse(logger.IsDebugEnabled());
Assert.IsFalse(logger.IsInfoEnabled());
Assert.IsFalse(logger.IsWarnEnabled());
Assert.IsFalse(logger.IsErrorEnabled());
Assert.IsFalse(logger.IsFatalEnabled());

// act
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Fatal, t_directoryLogPath);

// assert
Assert.IsFalse(logger.IsDebugEnabled());
Assert.IsFalse(logger.IsInfoEnabled());
Assert.IsFalse(logger.IsWarnEnabled());
Assert.IsFalse(logger.IsErrorEnabled());
Assert.IsTrue(logger.IsFatalEnabled());

// act
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Error, t_directoryLogPath);

// assert
Assert.IsFalse(logger.IsDebugEnabled());
Assert.IsFalse(logger.IsInfoEnabled());
Assert.IsFalse(logger.IsWarnEnabled());
Assert.IsTrue(logger.IsErrorEnabled());
Assert.IsTrue(logger.IsFatalEnabled());

// act
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Warn, t_directoryLogPath);

// assert
Assert.IsFalse(logger.IsDebugEnabled());
Assert.IsFalse(logger.IsInfoEnabled());
Expand All @@ -74,9 +106,9 @@ public void TestThatChangesLogLevel()
public void TestThatLogsToProperFileWithProperLogLevelOnly()
{
// arrange
var logger = SFLoggerFactory.GetLogger<SFBlockingChunkDownloaderV3>();
var logger = SFLoggerFactory.GetSFLogger<SFBlockingChunkDownloaderV3>();
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Info, t_directoryLogPath);

// act
logger.Debug(DebugMessage);
logger.Info(InfoMessage);
Expand All @@ -91,8 +123,52 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly()
Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(WarnMessage)));
Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(ErrorMessage)));
Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(FatalMessage)));

// arrange
File.Delete(FindLogFilePath(t_directoryLogPath));
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Debug, t_directoryLogPath);

// act
logger.Debug(DebugMessage);

// assert
logLines = File.ReadLines(FindLogFilePath(t_directoryLogPath));
Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(DebugMessage)));
}


[Test]
public void TestThatRollsLogIfSizeIsTooBig()
{
// arrange
const int expecetedBackupLogCount = 2;
var logger = SFLoggerFactory.GetSFLogger<SFBlockingChunkDownloaderV3>();
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Trace, t_directoryLogPath);

var appenders = SFLogRepository.s_rootLogger.GetAppenders();
appenders.Remove(appenders[0]);
var randomFileName = $"snowflake_dotnet_{Path.GetRandomFileName()}";
var logFileName = randomFileName.Substring(0, randomFileName.Length - 4) + ".log";
appenders.Add(new SFRollingFileAppender()
{
_name = "RollingFileAppender",
_logFilePath = Path.Combine(t_directoryLogPath, logFileName),
_maximumFileSizeInBytes = 1,
_maxSizeRollBackups = expecetedBackupLogCount,
_patternLayout = EasyLoggerManager.PatternLayout()
});

// act
for (int i = 0; i < 5; i++)
{
logger.Debug(DebugMessage);
System.Threading.Thread.Sleep(1000);
}
var backupLogs = Directory.GetFiles(t_directoryLogPath, $"{logFileName}.*.bak");

// assert
Assert.AreEqual(expecetedBackupLogCount, backupLogs.Length);
}

[Test]
public void TestThatOnlyUnknownFieldsAreLogged()
{
Expand Down Expand Up @@ -138,7 +214,7 @@ private static string FindLogFilePath(string directoryLogPath)
Assert.AreEqual(1, files.Length);
return files.First();
}

private static void RemoveEasyLoggingLogFiles()
{
Directory.GetFiles(s_logsDirectory)
Expand Down
Loading
Loading