RHINENG-16026: tweak Kessel logs#2100
RHINENG-16026: tweak Kessel logs#2100Dugowitch wants to merge 1 commit intoRedHatInsights:masterfrom
Conversation
Updated the duration from ns to ms for readability, update the label to 'durationMs', omit unnecessary list of workspaces, and omit unnecessary final OK log.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts Kessel middleware logging for streamed workspace listing by switching duration logging from nanoseconds to milliseconds, simplifying logged fields, and removing an unnecessary success log message. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- For consistency and clarity, consider using
time.Since(start).Milliseconds()instead oftime.Since(start).Nanoseconds()/1e6when computingdurationMs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For consistency and clarity, consider using `time.Since(start).Milliseconds()` instead of `time.Since(start).Nanoseconds()/1e6` when computing `durationMs`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
SC Environment Impact AssessmentOverall Impact: ⚪ NONE No SC Environment-specific impacts detected in this PR. What was checkedThis PR was automatically scanned for:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2100 +/- ##
==========================================
- Coverage 59.40% 59.39% -0.01%
==========================================
Files 134 134
Lines 8707 8706 -1
==========================================
- Hits 5172 5171 -1
Misses 2989 2989
Partials 546 546
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| utils.LogError( | ||
| "err", err.Error(), "receivingDuration", time.Since(start), "permission", permission, "received_count", | ||
| len(workspaces), "failed to useStreamedListObjects", | ||
| "err", err.Error(), "durationMs", time.Since(start).Nanoseconds()/1e6, "permission", permission, |
There was a problem hiding this comment.
As suggested by sourcery: time.Since(start).Milliseconds() would be better / more readable :) .
Updated the duration from ns to ms for readability,
update the label to 'durationMs',
omit unnecessary list of workspaces,
and omit unnecessary final OK log.
Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Adjust Kessel middleware logging to improve readability and reduce noise.
Enhancements: