Repository navigation
Tidy up logging output - #17
Conversation
This allows us to redirect to a different output writer by modifying the default logger.
This fixes the following warning on minispire start up: % go run ./cmd/main.go# github.com/shoenig/go-m1cpu ../../go/pkg/mod/github.com/shoenig/go-m1cpu@v0.1.6/cpu.go:75:17: warning: variable length array folded to constant array as an extension [-Wgnu-folding-constant]
Summary of ChangesHello @sd109, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the application's logging mechanism by standardizing output through the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request updates the gopsutil dependency to v4 and replaces fmt.Printf calls with log.Printf for consistent logging, addressing a warning related to go-m1cpu and improving logging practices. However, the new logging statements introduce a security vulnerability by logging sensitive SVID tokens. These tokens are credentials and should not be written to logs, as this could lead to their exposure and compromise.
| } | ||
|
|
||
| fmt.Printf("JWT SVID issued: %s\n", token) | ||
| log.Printf("JWT SVID issued: %s\n", token) |
There was a problem hiding this comment.
While changing fmt.Printf to log.Printf improves consistent logging, logging a JWT SVID token, which is a sensitive credential, introduces a security vulnerability. Logging credentials can lead to their exposure and allow an attacker to impersonate a workload. It is recommended to remove this log statement or log a confirmation message without the token itself.
| } | ||
|
|
||
| fmt.Printf("WIT-SVID issued: %s\n", token) | ||
| log.Printf("WIT-SVID issued: %s\n", token) |
There was a problem hiding this comment.
While using the standard log package instead of fmt.Printf tidies up logging, logging a WIT-SVID token, which is a sensitive credential, introduces a security vulnerability. Logging credentials can lead to their exposure and allow an attacker to impersonate a workload. It is recommended to remove this log statement or log a confirmation message without the token itself.
| github.com/cofide/cofide-sdk-go v0.4.2 | ||
| github.com/go-jose/go-jose/v4 v4.1.3 | ||
| github.com/shirou/gopsutil/v3 v3.24.5 | ||
| github.com/shirou/gopsutil/v4 v4.25.8 |
There was a problem hiding this comment.
Updating gopsutil from v3 to v4 is a significant version bump. While the change is necessary to resolve the warning, it's good practice to ensure that there are no breaking API changes that might affect other parts of the codebase. A quick check of the gopsutil v4 migration guide would be beneficial if this hasn't been done already.
| ) | ||
|
|
||
| require ( | ||
| github.com/ebitengine/purego v0.8.4 // indirect |
There was a problem hiding this comment.
The addition of github.com/ebitengine/purego v0.8.4 // indirect seems to be a new indirect dependency. It's important to understand why this dependency was introduced with the gopsutil update, as it might indicate a new underlying requirement or a change in how gopsutil interacts with system libraries.
| github.com/gobwas/glob v0.2.3 // indirect | ||
| github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect | ||
| github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect | ||
| github.com/shoenig/go-m1cpu v0.1.6 // indirect |
There was a problem hiding this comment.
The removal of github.com/shoenig/go-m1cpu v0.1.6 // indirect is directly related to the warning mentioned in the PR description. This is a good cleanup, as it removes an unused dependency that was causing compilation warnings.
2 participants
Uses log.Printf instead of fmt.Printf and fixes the following warning on minispire start up: