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

feat: Add new configuration options for logging. #1880 #2264

Merged
merged 3 commits into from Feb 26, 2024

Conversation

tippmar-nr
Copy link
Member

@tippmar-nr tippmar-nr commented Feb 23, 2024

Thank you for submitting a pull request. Please review our contributing guidelines and code of conduct.

Description

Adds new configuration options for logging in the .NET agent, to enable control over max file size, number of log files retained and the log rollover strategy (by size or by day). Note that .NET Profiler logging is unaffected by these configuration options.

Adds 3 new optional attributes to the <log> element in newrelic.config:

  • logRollingStrategy - possible values are size and day, defaulting to size if not specified.
  • maxLogFiles - an integer specifying the maximum number of log files to retain, defaulting to 4 if not specified. A value of 0 will retain all log files.
  • maxLogFileSizeBytes - an integer specifying the maximum log file size in bytes, defaulting to 50MB. Only valid when logRollingStrategy is size. A value of 0 specifies that there is no maximum log file size.

Adds 3 new environment variables that will override the new attributes above if specified:

  • NEW_RELIC_LOG_ROLLING_STRATEGY
  • NEW_RELIC_LOG_MAX_FILES
  • NEW_RELIC_LOG_MAX_FILE_SIZE_BYTES

Resolves #1880.

Documentation preview is at https://deploy-preview-16283--docs-website-netlify.netlify.app/docs/apm/agents/net-agent/configuration/net-agent-configuration/#log-logRollingStrategy

Author Checklist

  • Unit tests, Integration tests, and Unbounded tests completed
  • Performance testing completed with satisfactory results (if required)

Reviewer Checklist

  • Perform code review
  • Pull request was adequately tested (new/existing tests, performance tests)

@tippmar-nr tippmar-nr enabled auto-merge (squash) February 26, 2024 16:20
Copy link
Member

@nrcventura nrcventura left a comment

Choose a reason for hiding this comment

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

We might want to clarify that this only applies to managed agent logs, and that profiler logs are not affected by these settings. I don't know if that information is best placed in this PR description, in the configuration.xsd, or elsewhere.

Copy link
Member

@chynesNR chynesNR left a comment

Choose a reason for hiding this comment

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

Looks good!

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 15.21739% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 81.33%. Comparing base (d4372ca) to head (b49153a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2264      +/-   ##
==========================================
+ Coverage   81.30%   81.33%   +0.02%     
==========================================
  Files         396      397       +1     
  Lines       24748    24755       +7     
  Branches     2993     2995       +2     
==========================================
+ Hits        20122    20135      +13     
+ Misses       3847     3841       -6     
  Partials      779      779              
Files Coverage Δ
.../NewRelic/Agent/Core/Logging/LoggerBootstrapper.cs 75.77% <87.50%> (-0.48%) ⬇️
src/Agent/NewRelic/Agent/Core/Config/ILogConfig.cs 0.00% <0.00%> (ø)
.../NewRelic/Agent/Core/Config/ConfigurationLoader.cs 82.83% <0.00%> (ø)

... and 4 files with indirect coverage changes

@tippmar-nr tippmar-nr merged commit d33714f into main Feb 26, 2024
71 checks passed
@tippmar-nr tippmar-nr deleted the feature/logger-config branch February 26, 2024 21:11
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 this pull request may close these issues.

Add configuration for agent log file size, rollover strategy and retention policy
4 participants