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

Set custom log level, and added formatting fields #340

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nebula-aac
Copy link
Contributor

@nebula-aac nebula-aac commented Aug 5, 2023

Description

This PR adds

  • formatted fields
  • a custom log level to call
  • revised Format to account for Warn and Error

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

@leecalcote leecalcote requested a review from Revolyssup August 9, 2023 22:04
@leecalcote leecalcote added the kind/enhancement Improvement in current feature label Aug 9, 2023
@leecalcote leecalcote removed the request for review from Revolyssup August 18, 2023 19:05
@leecalcote
Copy link
Member

Updating reviewers...

@leecalcote leecalcote requested a review from suhail34 August 18, 2023 19:05
@suhail34
Copy link
Contributor

How can i test this changes in my local

@Revolyssup
Copy link
Contributor

@suhail34 You can checkout this branch locally and then in meshery/meshery, point your go.mod to your local meshkit using replace. Then you can start Meshery and see the logs. Since there are new functions that are added, you can try replacing current .Info, .Error functions with these new functions to see how the logs come out.

@nebula-aac It would be nicer if could have some unit tests here.

@Revolyssup
Copy link
Contributor

@nebula-aac Do you have some screenshot examples of how the error logs will look like running it locally?

@suhail34
Copy link
Contributor

Hey @Revolyssup I tested this is the ss
image

@suhail34
Copy link
Contributor

I was thinking like if we can do the same as syslogformat and add in setformatter disableTimestamp: true something like this

TerminalLogFormat:
		log.SetFormatter(
&logrus.TextFormatter{
			DisableTimestamp: true
		}
)}

@Revolyssup
Copy link
Contributor

@suhail34 Also test it with Meshery server. I hope the logs don't get too big with 5-6 lines per error/warn log.

@nebula-aac
Copy link
Contributor Author

I'll have to make some updates on this PR. I'll provide some screenshots

@nebula-aac
Copy link
Contributor Author

If we want to keep it to the minimum on the logs for mesheryctl, I would like to know so that I'm not formatting the wrong fields.

@stale
Copy link

stale bot commented Oct 5, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the issue/stale Issue has not had any activity for an extended period of time label Oct 5, 2023
@nebula-aac nebula-aac added issue/willfix This issue will be worked on and removed issue/stale Issue has not had any activity for an extended period of time labels Oct 6, 2023
@nebula-aac nebula-aac added this to the v0.7.0 milestone Oct 6, 2023
@MUzairS15
Copy link

@nebula-aac Any updates ?

@suhail34
Copy link
Contributor

I think we could define the init funciton and disable the timestamp so that globally all of the logrus has timestamp disabled

@leecalcote leecalcote modified the milestones: v0.7.0, v0.8.0 Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/willfix This issue will be worked on kind/enhancement Improvement in current feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants