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

sfc-gh-ext-simba-lf · 2024-11-09T00:52:48Z

Description

Remove the log4net dependency and replace with the Microsoft logging interface so users can plug in their own custom loggers

Checklist

Code compiles correctly
Code is formatted according to Coding Conventions
Created tests which fail without the change (if possible)
All tests passing (dotnet test)
Extended the README / documentation, if necessary
Provide JIRA issue id (if possible) or GitHub issue id in PR name

…nector-net into SNOW-834781-Remove-log4net # Conflicts: # Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs # Snowflake.Data.Tests/UnitTests/Configuration/EasyLoggingConfigFinderTest.cs # Snowflake.Data/Core/ArrowResultSet.cs # Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs # Snowflake.Data/Core/HttpUtil.cs # Snowflake.Data/Core/SFBlockingChunkDownloaderV3.cs # Snowflake.Data/Core/SFStatement.cs

codecov · 2024-11-13T00:06:25Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2858	1	2857	115

View the top 1 failed test(s) by shortest run time

Snowflake.Data.Tests.IntegrationTests.SFConnectionITAsync::TestAsyncLoginTimeout

Stack Traces | 8.42s run time

at Snowflake.Data.Tests.IntegrationTests.SFConnectionITAsync.TestAsyncLoginTimeout() in D:\a\snowflake-connector-net\snowflake-connector-net\Snowflake.Data.Tests\IntegrationTests\SFConnectionIT.cs:line 2010

1)    at Snowflake.Data.Tests.IntegrationTests.SFConnectionITAsync.TestAsyncLoginTimeout() in D:\a\snowflake-connector-net\snowflake-connector-net\Snowflake.Data.Tests\IntegrationTests\SFConnectionIT.cs:line 2010

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

…nector-net into SNOW-834781-Remove-log4net

…nector-net into SNOW-834781-Remove-log4net # Conflicts: # Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs

…nector-net into SNOW-834781-Remove-log4net

Snowflake.Data/Logger/SFLoggerPair.cs

Snowflake.Data/Client/SnowflakeDbCommand.cs

Snowflake.Data/Logger/SFLoggerFactory.cs

Snowflake.Data/Logger/SFLoggerImpl.cs

…nector-net into SNOW-834781-Remove-log4net

sfc-gh-knozderko · 2025-06-10T12:54:15Z

Snowflake.Data/Snowflake.Data.csproj

@@ -24,9 +24,10 @@
    <PackageReference Include="Google.Cloud.Storage.V1" Version="4.10.0" />
    <PackageReference Include="Azure.Storage.Blobs" Version="12.13.0" />
    <PackageReference Include="Azure.Storage.Common" Version="12.12.0" />
+    <PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.1" />


Changed to 9.0.5

sfc-gh-knozderko · 2025-06-10T12:54:46Z

Snowflake.Data/Snowflake.Data.csproj

@@ -24,9 +24,10 @@
    <PackageReference Include="Google.Cloud.Storage.V1" Version="4.10.0" />
    <PackageReference Include="Azure.Storage.Blobs" Version="12.13.0" />
    <PackageReference Include="Azure.Storage.Common" Version="12.12.0" />
+    <PackageReference Include="Microsoft.Extensions.Logging" Version="9.0.1" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="9.0.1" />


I think this is not needed?

sfc-gh-knozderko · 2025-06-10T12:55:48Z

Snowflake.Data.Tests/Snowflake.Data.Tests.csproj

    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.1" />
+    <PackageReference Include="NLog.Extensions.Logging" Version="5.3.14" />


Changed to 5.5.0

sfc-gh-knozderko · 2025-06-10T12:56:06Z

Snowflake.Data.Tests/Snowflake.Data.Tests.csproj

    <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" />


Changed to 4.3.0

sfc-gh-knozderko · 2025-06-10T12:56:32Z

Snowflake.Data.Tests/Snowflake.Data.Tests.csproj

    <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" />


Changed to 9.0.2

sfc-gh-knozderko · 2025-06-10T12:56:51Z

Snowflake.Data.Tests/Snowflake.Data.Tests.csproj

+    <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" />


Changed to 7.0.0

…nector-net into SNOW-834781-Remove-log4net

sfc-gh-knozderko · 2025-06-18T15:04:30Z

Snowflake.Data/Logger/SFRollingFileAppender.cs

+            else
+            {
+                var fileInfo = new UnixFileInfo(path: LogFilePath);
+                using (var handle = fileInfo.Open(FileMode.Append, FileAccess.ReadWrite, FilePermissions.S_IWUSR | FilePermissions.S_IRUSR))


Actually this appender is used only by easy logging. And I think some users would find 600 permissions too restrictive. So I would like to make it configurable in EasyLogging config.
Currently we have the configuration file like that:

{ "common": { "log_level": "DEBUG", "log_path": "/path/to/log/directory" } }

we could add a "dotnet" section and "log_file_unix_permissions" field in it:

{ "common": { "log_level": "DEBUG", "log_path": "/path/to/log/directory" }, "dotnet": { "log_file_unix_permissions": "640" } }

and then we could create the file with such permissions and verify if the permissions are not broader that specified in the config file.
If such permissions would not be configured in the config file then we could assume 600 as the default value.

I updated the PR to include the changes mentioned above:

Add dotnet section and log_file_unix_permissions to config file

Create file with log_file_unix_permissions permissions and use those permissions for validating when reading/writing the log file

Use 600 as the default value for log_file_unix_permissions if not configured. Also uses 600 if the value in the config is invalid

Please let me know if there's any edits you'd like to make

…nector-net into SNOW-834781-Remove-log4net

sfc-gh-pbulawa · 2025-07-29T11:50:21Z

Snowflake.Data/Logger/SFLogger.cs

Why the chmod of the file is changed?

I've reverted the mode change. It shouldn't have been added in the changes

sfc-gh-pbulawa

Why the chmods of the files change?

sfc-gh-pbulawa · 2025-07-29T11:50:32Z

Snowflake.Data/Logger/SFLoggerFactory.cs

Why the chmod of the file is changed?

I've reverted the mode change for this one as well

…lakedb/snowflake-connector-net into SNOW-834781-Remove-log4net

sfc-gh-ext-simba-lf added 3 commits November 8, 2024 16:21

SNOW-834781: Remove log4net and delegate logging to consumers

650bb43

SNOW-834781: Refactor/add tests

f0484f6

sfc-gh-dszmolka mentioned this pull request Nov 11, 2024

SNOW-834781: Can we remove log4net and delegate logging to library consumers #204

Open

Set config from code

0ab8c66

sfc-gh-ext-simba-lf marked this pull request as ready for review November 13, 2024 19:23

sfc-gh-ext-simba-lf requested a review from a team as a code owner November 13, 2024 19:23

sfc-gh-ext-simba-lf requested a review from sfc-gh-knozderko November 13, 2024 19:24

sfc-gh-ext-simba-lf added 18 commits November 13, 2024 17:04

Use constant file names for tests

15090b4

Remove unused method

1b80c55

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

1a0a0f6

…nector-net into SNOW-834781-Remove-log4net

Add tests for home directory tool

00d0e39

Re-add Easy Logging

1036d56

Refactor original logger to simple logger

3f3a15c

Add initial impl. for own Snowflake logger

188f82a

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

0ec8b4d

…nector-net into SNOW-834781-Remove-log4net # Conflicts: # Snowflake.Data/Core/FileTransfer/StorageClient/SFGCSClient.cs

Fix log

26ef937

Add tests for new log functions

1c7afd3

Retrigger checks

1d4e732

Retrigger checks

c1b8d5a

Refactor classes and tests

2fe82b1

Rename log functions and add test for BeginScope

7bb3f98

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

f1fb9c2

…nector-net into SNOW-834781-Remove-log4net

Refactor logger functions add logger pair class

c9d8d92

Replace ILogger with logger pair

f0da3e7

Fix format error with bracket

7e8af5c

sfc-gh-knozderko requested changes Dec 6, 2024

View reviewed changes

sfc-gh-ext-simba-lf added 2 commits December 6, 2024 14:38

Move masking secrets

d6090aa

Fix typo

a7535dc

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

ea4ae7b

…nector-net into SNOW-834781-Remove-log4net

sfc-gh-dszmolka mentioned this pull request Jun 10, 2025

SNOW-2052266: Logs not generated using Easy Logging #1149

Open

sfc-gh-knozderko reviewed Jun 10, 2025

View reviewed changes

sfc-gh-ext-simba-lf added 5 commits June 11, 2025 12:48

Update Microsoft.Extensions.Logging to version 9.0.5

31fdd7b

Remove unneeded package

122192f

Update logger versions

f357d80

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

5ade1af

…nector-net into SNOW-834781-Remove-log4net

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

0d11bfa

…nector-net into SNOW-834781-Remove-log4net

sfc-gh-knozderko reviewed Jun 18, 2025

View reviewed changes

sfc-gh-ext-simba-lf and others added 10 commits June 18, 2025 14:33

Add unix file permission configuration to config file

e41accf

Refactor log file unix permission field

046919d

Refactor default permission value

89f1d07

Temp lines

a92f11b

Revert lines

b62519e

Fix comparison

8b39f0e

Add checks when getting config permission value

38f0ea1

Add test for log file security exception

93136e0

Fix order of bitwise comparison

5d0822d

Merge branch 'master' into SNOW-834781-Remove-log4net

a878803

sfc-gh-ext-simba-hx requested review from sfc-gh-pbulawa and sfc-gh-knozderko July 16, 2025 15:38

sfc-gh-ext-simba-lf and others added 2 commits July 16, 2025 10:40

Merge branch 'master' of https://github.com/snowflakedb/snowflake-con…

0e5b9fb

…nector-net into SNOW-834781-Remove-log4net

Merge branch 'master' into SNOW-834781-Remove-log4net

a8c0af4

sfc-gh-pbulawa reviewed Jul 29, 2025

View reviewed changes

sfc-gh-pbulawa requested changes Jul 29, 2025

View reviewed changes

sfc-gh-ext-simba-lf added 5 commits August 20, 2025 10:57

Revert mode

a423383

Re-add changes

222d1b5

Merge branch 'SNOW-834781-Remove-log4net' of https://github.com/snowf…

d134bba

…lakedb/snowflake-connector-net into SNOW-834781-Remove-log4net

Revert mode

dfddbd6

Re-add changes

7a6864c

		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.1" />
		<PackageReference Include="NLog.Extensions.Logging" Version="5.3.14" />

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

Are you sure you want to change the base?

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

Conversation

sfc-gh-ext-simba-lf commented Nov 9, 2024

Description

Checklist

Uh oh!

codecov bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

sfc-gh-knozderko Jun 18, 2025 • 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!

sfc-gh-pbulawa 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!

Uh oh!

codecov bot commented Nov 13, 2024 •

edited

Loading

sfc-gh-knozderko Jun 18, 2025 •

edited

Loading