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

Update our test environment to use log4net 2.0.10 or later #895

Closed
4 tasks
angelatan2 opened this issue Jan 27, 2022 · 4 comments
Closed
4 tasks

Update our test environment to use log4net 2.0.10 or later #895

angelatan2 opened this issue Jan 27, 2022 · 4 comments

Comments

@angelatan2
Copy link
Contributor

angelatan2 commented Jan 27, 2022

Description
Please update our agent and test environment projects to use log4net version 2.0.10 or later. For example:

Referenced in:

  • src/Agent/NewRelic/Agent/Core/Core.csproj
  • tests/Agent/UnitTests/NewRelic.Agent.TestUtilities/NewRelic.Agent.TestUtilities.csproj
  • tests/Agent/UnitTests/Core.UnitTest/Core.UnitTest.csproj
  • tests/Agent/IntegrationTests/UnboundedIntegrationTests/UnboundedIntegrationTests.csproj

Notification originated from Dependabot Alert

@vuqtran88 vuqtran88 linked a pull request Jan 28, 2022 that will close this issue
@angelatan2 angelatan2 removed this from the Logging Initiative - Log4Net milestone Feb 3, 2022
@angelatan2 angelatan2 added version: major To tag issues that may cause breaking changes or removal of deprecated apis/config and removed technical debt labels Mar 25, 2022
@angelatan2 angelatan2 added this to the Major Release 10.0 milestone Mar 28, 2022
@vuqtran88 vuqtran88 self-assigned this Jul 5, 2022
@vuqtran88 vuqtran88 removed this from the Major Release 10.0 milestone Jul 5, 2022
@vuqtran88
Copy link
Contributor

vuqtran88 commented Jul 5, 2022

When upgrading the agent to use log4net version 2.0.10 or later, even though the application and the agent still function properly (instrumentations work as expected, logging works as expected), some log4net specific exceptions were logged to STD output. This only happens to .NET Core application. It would be hard to detect this, but luckily our integration test InstrumentationLoaderTestsCore does validate console output so that it detected this and failed.

After digging, part of the issue is that the log4net 2.0.10 or later started using the Configuration.ConfigurationManager for .Net Core applications to read log4net configuration from different places (.ie appsettings.config, web.config, machine.config), but this library doesn't work correctly with the agent when it is ILRepacked with the agent. To be specific, any access to AppSettings object from this call ConfigurationManager.AppSettings["some setting"] from within the agent will throw the following exception:

System.IO.FileNotFoundException: Could not load file or assembly 'System.Configuration.ConfigurationManager, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.

I found a related issue with MS and it is still not being resolved: dotnet/runtime#32307

I haven't yet figured out a workaround to get the Configuration.ConfigurationManager to work in the agent yet, but a real workaround may be to use something else rather than log4net. Even though the exception was thrown, caught, logged to STD out, and the agent and the app work normally, I'm still hesitate to upgrade because I don't know what could happen in the wild.

@vuqtran88 vuqtran88 removed their assignment Jul 5, 2022
@workato-integration
Copy link

@angelatan2 angelatan2 removed the version: major To tag issues that may cause breaking changes or removal of deprecated apis/config label Mar 10, 2023
@workato-integration
Copy link

Jira CommentId: 204390
Commented by mtippin:

Closing this story, it's been superseded by the log4net to Serilog migration feature.

@workato-integration
Copy link

This issue won't be actioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants