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

unify logging #487

Merged
merged 3 commits into from
Jul 17, 2024
Merged

unify logging #487

merged 3 commits into from
Jul 17, 2024

Conversation

DavidKorczynski
Copy link
Collaborator

We use logging in multiple formats:

  • logging.info/debug/..
  • print(
  • write to files

It's fairly inconsistent. So am unifying it in a more Pythonic way. The format also makes it easier to track performance of the tool as we can find bottlenecks by looking at the logs.

We use logging in multiple formats:

- logging.info/debug/..
- print(
- write to files

It's fairly inconsistent. So am unifying it in a more Pythonic way. The
format also makes it easier to track performance of the tool as we can
find bottlenecks by looking at the logs.

Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
Signed-off-by: David Korczynski <david@adalogics.com>
@DavidKorczynski
Copy link
Collaborator Author

/gcbrun exp -n dk-test-infra-99x3 -m vertex_ai_gemini-1-5 -b comparison

@oliverchang oliverchang requested a review from DonggeLiu July 16, 2024 03:54
@DonggeLiu
Copy link
Collaborator

Thanks for unifying the logs, @DavidKorczynski. In addition to the benefit of the consistent format (as you mentioned), this will keep the logs in the correct order, which will help us debug.

However, since you happen to work on this, could you please also help us switch to google-cloud-logging?
The current code prints all logs in stderr, which shows all logs are errors:
image

Using google-cloud-logging will fix this, here is how it is used in FuzzBench.

@DavidKorczynski
Copy link
Collaborator Author

However, since you happen to work on this, could you please also help us switch to google-cloud-logging?
The current code prints all logs in stderr, which shows all logs are errors:

Am happy to do this, can we do this in follow-up PRs?

Copy link
Collaborator

@DonggeLiu DonggeLiu left a comment

Choose a reason for hiding this comment

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

Yep sure, the sooner the better as we won't be able to filter logs by severity once we merge this.

@DavidKorczynski DavidKorczynski merged commit 787c97a into main Jul 17, 2024
7 checks passed
@DavidKorczynski DavidKorczynski deleted the fix-logging-unify branch July 17, 2024 09:01
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.

2 participants