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

Rolling log files #1573 #1763

Closed
wants to merge 5 commits into from

Conversation

DannyHinshaw
Copy link
Contributor

This PR brings in the feature for rolling logs. Implemented by bringing in lumberjack as was suggested on the issue.

Additionally, new config file properties have been added to further configure the lumberjack output:

// logSettings
type logSettings struct {
	LogCompress   bool   `yaml:"log_compress"`    // Compress determines if the rotated log files should be compressed using gzip (default: false)
	LogLocalTime  bool   `yaml:"log_localtime"`   // If the time used for formatting the timestamps in is the computer's local time (default: false [UTC])
	LogMaxBackups int    `yaml:"log_max_backups"` // Maximum number of old log files to retain (MaxAge may still cause them to get deleted)
	LogMaxSize    int    `yaml:"log_max_size"`    // Maximum size in megabytes of the log file before it gets rotated (default 100 MB)
	LogMaxAge     int    `yaml:"log_max_age"`     // MaxAge is the maximum number of days to retain old log files
	LogFile       string `yaml:"log_file"`        // Path to the log file. If empty, write to stdout. If "syslog", writes to syslog
	Verbose       bool   `yaml:"verbose"`         // If true, verbose logging is enabled
}

@DannyHinshaw
Copy link
Contributor Author

+golangci-lint run
home/home.go:9: File is not `goimports`-ed (goimports)
	"gopkg.in/natefinch/lumberjack.v2"

CI fail looks to be linter related. What is the proper way to fix this error?

@ameshkov
Copy link
Member

ameshkov commented Jun 2, 2020

CI fail looks to be linter related. What is the proper way to fix this error?

You may need to write something like

import (
    lumberjack "gopkg.in/natefinch/lumberjack.v2"
)

Also, I suggest installing golangci-lint locally and running it to see if there are any other linter issues like this.

@codecov-commenter
Copy link

Codecov Report

Merging #1763 into master will increase coverage by 0.00%.
The diff coverage is 29.41%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1763   +/-   ##
=======================================
  Coverage   38.76%   38.77%           
=======================================
  Files          61       61           
  Lines        8206     8220   +14     
=======================================
+ Hits         3181     3187    +6     
- Misses       4624     4630    +6     
- Partials      401      403    +2     
Impacted Files Coverage Δ
home/config.go 47.47% <ø> (ø)
home/home.go 31.26% <29.41%> (+0.17%) ⬆️
util/auto_hosts.go 72.05% <0.00%> (+0.49%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ecd385...a066c95. Read the comment docs.

@ameshkov ameshkov requested a review from szolin June 3, 2020 08:56
@adguard adguard closed this in 5ce98bd Jun 5, 2020
@ameshkov
Copy link
Member

ameshkov commented Jun 5, 2020

@DannyHinshaw thank you!

@DannyHinshaw
Copy link
Contributor Author

@ameshkov you're welcome! Is it going to be merged? I'm unfamiliar with how the team processes PR's.

@szolin
Copy link
Contributor

szolin commented Jun 5, 2020

It's already merged!

@Freebase394
Copy link

Guys! Thanks so much for this "Feature"
Its so nice to have it on this project AdGuardHome and it's so usefull.

I love it!
image

I apologize for the inconvenience out of the blue, or for no reason!
I just wanted to say it, because those who contribute sometimes do not always receive the necessary recognition!
Thank you

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.

None yet

5 participants