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: write logs into file #673

Merged
merged 6 commits into from
Sep 12, 2023

Conversation

amirvalhalla
Copy link
Member

Description

write logs into files with rotation feature which provides optional parameters to configure log file based on time duration or file size

Related issue(s)

If this Pull Request is related to an issue, mention it here.

@codecov
Copy link

codecov bot commented Sep 2, 2023

Codecov Report

Merging #673 (7331e02) into main (1ab181d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #673      +/-   ##
==========================================
+ Coverage   83.43%   83.46%   +0.02%     
==========================================
  Files         168      168              
  Lines        7956     7970      +14     
==========================================
+ Hits         6638     6652      +14     
  Misses       1014     1014              
  Partials      304      304              

@b00f
Copy link
Collaborator

b00f commented Sep 3, 2023

@amirvalhalla

Configuration can be simplified.
User can just set if he want to write log to file and how much is the maximum size of the file.
So,

1- The directory always set to logs
2- File name should set always to pactus.log
3- set MaxAge to always zero. user can delete logs manually
We can set any other settings to default.

config/example_config.toml Outdated Show resolved Hide resolved
util/logger/config.go Outdated Show resolved Hide resolved
util/logger/logger.go Outdated Show resolved Hide resolved
cmd/cmd.go Outdated Show resolved Hide resolved
@amirvalhalla
Copy link
Member Author

@b00f
we can't remove WorkingDir because if we use os.GetWD which returns current directory, the logs directory will generate for each package because when you run os.GetWD it return the current directory that code is executing.
so what is your suggestion? should we revert WorkingDir etc...

.gitignore Outdated Show resolved Hide resolved
sync/firewall/logs/pactus.log Outdated Show resolved Hide resolved
util/logger/logger.go Outdated Show resolved Hide resolved
util/logger/logger.go Outdated Show resolved Hide resolved
@@ -1,5 +1,10 @@
package logger

const (
LogDirectory = "logs"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need it anymore.

@@ -1,5 +1,10 @@
package logger

const (
LogDirectory = "logs"
LogFilename = "pactus.log"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move this from config.go to logger.go
It is not something to be configured.

@amirvalhalla
Copy link
Member Author

@b00f
I'll push last changes, tonight

@b00f
Copy link
Collaborator

b00f commented Sep 11, 2023

@amirvalhalla
We can merge it now. No worries. I will fix it

@amirvalhalla
Copy link
Member Author

@amirvalhalla We can merge it now. No worries. I will fix it

@b00f
please let met push changes, everything is ok :)

@b00f
Copy link
Collaborator

b00f commented Sep 11, 2023

OK. Waiting for our last changes. Thanks

@b00f b00f merged commit 8151298 into pactus-project:main Sep 12, 2023
12 checks passed
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.

Write logs to file
2 participants