BED-4602: Reduce Verbose Log Output #161
Conversation
WalkthroughRefactors logging across many Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cmd/list-group-owners.go (1)
109-109: Consider logging owner identifier for better debugging context.The log currently emits only
groupId, which identifies the group but provides no information about which specific owner was found. For consistency with other files in this PR (e.g.,list-function-app-role-assignments.gologs id, roleDefinitionId, principalId) and to aid debugging, consider also logging an owner-specific identifier.🔎 Suggested enhancement
-log.V(2).Info("found group owner", "groupId", groupOwner.GroupId) +log.V(2).Info("found group owner", "groupId", groupOwner.GroupId, "ownerId", groupOwner.Owner.Id)cmd/list-group-members.go (1)
119-119: Consider logging member identifier for better debugging context.Similar to the group owners case, this log emits only
groupIdwithout any member-specific identifier. To improve debugging capability and maintain consistency with the logging pattern in other files (which typically log multiple identifying fields), consider adding a member identifier.🔎 Suggested enhancement
-log.V(2).Info("found group member", "groupId", groupMember.GroupId) +log.V(2).Info("found group member", "groupId", groupMember.GroupId, "memberId", groupMember.Member.Id)cmd/list-storage-accounts.go (1)
105-105: Logging change achieves the PR objective.The change successfully reduces log verbosity by logging only
idandnameinstead of the entire storage account object.Optional: Consider consistent field access pattern
The code mixes explicit field access (
storageAccount.StorageAccount.Id) with promoted field access (storageAccount.Name). For consistency, consider using either both promoted or both explicit:-log.V(2).Info("found storage account", "id", storageAccount.StorageAccount.Id, "name", storageAccount.Name) +log.V(2).Info("found storage account", "id", storageAccount.Id, "name", storageAccount.Name)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.gitignorecmd/list-app-owners.gocmd/list-app-role-assignments.gocmd/list-apps.gocmd/list-automation-account-role-assignments.gocmd/list-automation-accounts.gocmd/list-container-registries.gocmd/list-container-registry-role-assignments.gocmd/list-device-owners.gocmd/list-devices.gocmd/list-function-app-role-assignments.gocmd/list-function-apps.gocmd/list-group-members.gocmd/list-group-owners.gocmd/list-groups.gocmd/list-key-vault-role-assignments.gocmd/list-key-vaults.gocmd/list-logic-app-role-assignments.gocmd/list-logic-apps.gocmd/list-managed-cluster-role-assignments.gocmd/list-managed-clusters.gocmd/list-management-group-role-assignments.gocmd/list-management-groups.gocmd/list-resource-group-role-assignments.gocmd/list-resource-groups.gocmd/list-role-assignments.gocmd/list-role-eligibility-schedule-instance.gocmd/list-roles.gocmd/list-service-principal-owners.gocmd/list-service-principals.gocmd/list-storage-account-role-assignments.gocmd/list-storage-accounts.gocmd/list-storage-containers.gocmd/list-subscription-owners.gocmd/list-subscription-role-assignments.gocmd/list-subscription-user-access-admins.gocmd/list-subscriptions.gocmd/list-tenants.gocmd/list-users.gocmd/list-virtual-machine-role-assignments.gocmd/list-virtual-machines.gocmd/list-vm-scale-set-role-assignments.gocmd/list-vm-scale-sets.gocmd/list-web-app-role-assignments.gocmd/list-web-apps.go
🧰 Additional context used
🧬 Code graph analysis (8)
cmd/list-tenants.go (2)
constants/misc.go (1)
DisplayName(29-29)client/query/params.go (1)
TenantId(41-41)
cmd/list-resource-group-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-subscription-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-service-principals.go (1)
constants/misc.go (1)
DisplayName(29-29)
cmd/list-key-vault-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-virtual-machine-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-management-group-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-storage-accounts.go (2)
models/azure/storage_account.go (1)
StorageAccount(22-34)models/storage-account.go (1)
StorageAccount(22-28)
🔇 Additional comments (43)
.gitignore (1)
133-134: Appropriate addition to gitignore.The new pattern for
.idea/**/go.imports.xmlis a valid gitignore entry that correctly targets JetBrains IDE-generated Go imports metadata. The placement within the "Generated files" subsection aligns well with similar patterns in the file.cmd/list-management-groups.go (1)
74-74: LGTM! Logging improvement reduces verbosity effectively.The change successfully reduces log output by logging only the management group's ID and name instead of the entire object, which aligns perfectly with the PR objective to minimize verbose log size while maintaining useful diagnostic information.
cmd/list-roles.go (1)
73-73: Effective reduction of verbose log output.The selective field logging (id, displayName, isEnabled) successfully addresses the PR objective while retaining the most relevant information for debugging and traceability. The field selection is appropriate for role entities and follows the structured logging pattern consistently applied across other list commands in this PR.
cmd/list-subscription-user-access-admins.go (1)
91-91: LGTM! Effective reduction in log verbosity.The change successfully replaces full-object logging with targeted field logging. The selected fields (id, principalId, scope, roleId) provide sufficient identifiers for debugging while significantly reducing log size, which aligns perfectly with the PR objective.
cmd/list-storage-containers.go (1)
113-113: LGTM! Effective log verbosity reduction.The change from logging the entire
storageContainerobject to logging only the key fields (id,name,tenant) successfully achieves the PR objective of reducing verbose log output while retaining sufficient information for debugging and tracing.cmd/list-devices.go (1)
73-73: LGTM - Log verbosity reduction implemented correctly.The change successfully reduces verbose log output by logging only the
deviceIdfield instead of the entire device object, which aligns with the PR objectives. TheDeviceIdfield exists on the Device type and is appropriately used as a unique identifier for logging.cmd/list-role-assignments.go (1)
109-109: The concern about nil pointer dereference is not applicable here.
RoleDefinitionis a value type (UnifiedRoleDefinition), not a pointer. In Go, value types always have a zero-value when unmarshaled from JSON withomitempty, and cannot be nil. Accessing.DisplayNameon an emptyUnifiedRoleDefinitionstruct is safe and returns an empty string rather than causing a panic. No defensive check is needed.cmd/list-web-apps.go (1)
111-111: LGTM! Logging change reduces verbosity effectively.The change from logging the entire
webAppobject to logging justidandnamefields aligns perfectly with the PR objective. These two fields provide sufficient context for debugging while significantly reducing log size.cmd/list-key-vault-role-assignments.go (1)
109-109: LGTM! Well-structured logging change.The log statement emits both the role assignment ID and the key vault ID, providing clear context for debugging while reducing log verbosity. The structured field approach is clean and effective.
cmd/list-users.go (1)
87-87: LGTM! Comprehensive and well-balanced logging.The log statement captures three key fields—
id,type, andisEnabled—which provide excellent debugging context while achieving the PR's goal of reducing log size. The field selection is thoughtful and practical.cmd/list-function-app-role-assignments.go (1)
113-113: LGTM! Typo fix and excellent logging enhancement.This change accomplishes two improvements:
- Fixes the typo: "asignment" → "assignment"
- Adds comprehensive structured logging with four key fields (id, roleDefinitionId, principalId, scope)
The detailed field selection provides excellent debugging context while still achieving significant log size reduction compared to logging the entire object.
cmd/list-key-vaults.go (1)
108-108: LGTM! Comprehensive logging with good field selection.The log statement captures three key fields—
id,name, andtenantId—which provide strong debugging context across different dimensions (technical identifier, human-readable name, and organizational scope). Well-balanced approach to verbosity reduction.cmd/list-groups.go (1)
73-73: LGTM! Clean and effective logging change.The log statement emits both
idandname(DisplayName), providing a good balance between a technical identifier and a human-readable label. This is consistent with similar changes across the codebase and achieves the PR's verbosity reduction goal.cmd/list-subscription-owners.go (1)
91-91: LGTM! Logging refinement reduces verbosity effectively.The change from logging the entire
subscriptionOwnerobject to logging specific key fields (principalId,role,subscriptionId) achieves the PR's objective of reducing verbose log output while maintaining essential debugging information.cmd/list-subscriptions.go (1)
95-95: LGTM! Concise logging for subscription identification.Logging only the
SubscriptionIdinstead of the entire object significantly reduces log verbosity while providing sufficient information for tracking subscription enumeration.cmd/list-logic-apps.go (1)
114-114: LGTM! Appropriate field selection for Logic App logging.The change to log
idandnamefields instead of the entirelogicappobject provides sufficient debugging information while significantly reducing log size.cmd/list-app-role-assignments.go (1)
105-105: LGTM! Comprehensive logging for role assignments.The structured logging of
id,appRoleId, andnamefields provides excellent context for debugging app role assignments while avoiding the verbosity of logging the entire object.cmd/list-automation-account-role-assignments.go (1)
113-113: LGTM! Essential fields logged for automation account role assignments.Logging
idandroleDefinitionIdcaptures the most important identifiers for automation account role assignments while reducing log verbosity.cmd/list-tenants.go (1)
84-84: LGTM! Thorough tenant identification in logs.Logging
id,name, andtenantIdprovides complete context for tenant enumeration while maintaining concise log output.cmd/list-managed-cluster-role-assignments.go (1)
118-118: LGTM! Comprehensive logging for managed cluster role assignments.The structured logging of
id,roleDefinitionId, andprincipalIdprovides excellent debugging context for managed cluster role assignments while significantly reducing log verbosity.cmd/list-function-apps.go (1)
106-106: LGTM! Concise logging for Function Apps.Logging
idandnamefields provides sufficient information for tracking Function App enumeration while reducing log size, consistent with the pattern used across other list commands.cmd/list-vm-scale-set-role-assignments.go (1)
118-118: LGTM! Logging improvement reduces verbosity.The change from logging the entire
vmScaleSetRoleAssignmentobject to logging only the key fields (idandroleId) effectively reduces log volume while preserving essential debugging context.cmd/list-app-owners.go (1)
92-92: LGTM! Cleaner log output.Logging only the
appIdinstead of the entireappOwnerobject streamlines the verbose output while maintaining the essential identifier for debugging.cmd/list-resource-groups.go (1)
105-105: LGTM! Concise and informative logging.The change to log only
idandnamefields provides clear resource identification while significantly reducing log size compared to the full object dump.cmd/list-logic-app-role-assignments.go (1)
118-118: LGTM! Improved log clarity.Logging the specific
idandroleDefinitionIdfields instead of the entire role assignment object maintains debugging capability while reducing verbose output.cmd/list-managed-clusters.go (1)
109-109: LGTM! Streamlined logging.The change to log only
idandnameeffectively reduces log verbosity while retaining the essential identifiers for managed cluster debugging.cmd/list-virtual-machine-role-assignments.go (1)
109-109: LGTM! Targeted logging improvement.Logging only
roleIdandvirtualMachineIdprovides the necessary context for debugging virtual machine role assignments while significantly reducing log size.cmd/list-storage-account-role-assignments.go (1)
113-113: LGTM! Comprehensive yet concise logging.Logging
id,roleDefinitionId, andprincipalIdprovides sufficient context for storage account role assignment debugging while avoiding the overhead of logging the entire object.cmd/list-subscription-role-assignments.go (1)
109-109: LGTM! Efficient logging pattern.Logging only the nested
RoleAssignment.IdandRoleAssignment.Namefields effectively reduces log verbosity while preserving the key identifiers for subscription role assignment debugging.cmd/list-service-principal-owners.go (1)
109-109: LGTM! Logging refinement reduces verbosity effectively.The change from logging the entire
servicePrincipalOwnerobject to just theServicePrincipalIdfield reduces log size while preserving essential traceability for debugging.cmd/list-vm-scale-sets.go (1)
110-110: LGTM! Structured logging with key identifiers.Logging
idandnameinstead of the entire VM scale set object significantly reduces log volume while retaining the most relevant identifiers for troubleshooting.cmd/list-resource-group-role-assignments.go (1)
110-110: LGTM! Granular logging with essential role assignment details.The structured logging of
id,name, andresourceGroupIdprovides clear traceability for role assignments while dramatically reducing log size compared to serializing the entire object.cmd/list-service-principals.go (1)
73-73: LGTM! Logging refinement captures key service principal identifiers.Emitting
id,name(DisplayName), andappIdprovides the essential identifiers for service principals while avoiding the overhead of logging entire response objects.cmd/list-automation-accounts.go (1)
105-105: LGTM! Concise logging with key automation account identifiers.Logging
idandnameinstead of the full automation account object reduces log verbosity while maintaining adequate traceability for debugging.cmd/list-virtual-machines.go (1)
105-105: LGTM! Structured logging with multi-tenant context.Logging
id,name, andtenantIdprovides clear VM identification across tenants while significantly reducing log output compared to serializing the entire virtual machine object.cmd/list-container-registry-role-assignments.go (1)
118-118: LGTM! Comprehensive role assignment logging with key fields.The structured logging captures all essential role assignment attributes (
id,principalId,roleDefinitionId,scope) while avoiding the overhead of serializing entire objects—this provides excellent traceability for RBAC debugging.cmd/list-container-registries.go (1)
110-110: LGTM! Concise logging with key container registry identifiers.Logging
idandnameinstead of the full container registry object reduces log size while maintaining the essential identifiers needed for troubleshooting.cmd/list-web-app-role-assignments.go (1)
118-118: LGTM! Logging change reduces verbosity and fixes typo.The change from logging the entire
webAppRoleAssignmentstruct to just theroleDefinitionIdfield achieves the PR objective of reducing log output. Additionally, the typo fix ("asignment" → "assignment") improves code quality.cmd/list-apps.go (1)
73-73: LGTM! Logging change effectively reduces verbosity.The change from logging the entire application object to just the
idandnamefields aligns with the PR objective and significantly reduces log output while retaining essential diagnostic information.cmd/list-device-owners.go (1)
108-108: The current log statement is correct; no changes needed.The log change successfully reduces verbosity. The
Ownerfield inDeviceOwneris typed asjson.RawMessage(raw JSON data), not a structured object with accessible fields likeId. Logging only thedeviceIdis the appropriate approach here, as extracting owner information would require unmarshaling the JSON, which is not done at this point in the code.Likely an incorrect or invalid review comment.
cmd/list-role-eligibility-schedule-instance.go (2)
75-75: LGTM! Logging refactor aligns with PR objectives.The change successfully reduces log verbosity by logging only the essential identifying fields (id, roleDefinitionId, principalId) instead of the entire object. This will significantly reduce log size while maintaining useful debugging information.
The fields chosen are appropriate as they represent the primary identifiers for role eligibility schedule instances and match the fields used in the data structure below (lines 81-83).
22-24: No action needed—these imports are correct. The file is a new addition to the codebase, so all imports are necessarily new. Theos,os/signal, andtimeimports are all used appropriately in the code (signal handling and timing operations).cmd/list-management-group-role-assignments.go (1)
109-109: Log statement refactored to emit specific fields instead of full objects.The change replaces object-level logging with granular field-level logging (id, roleId, principalId, scope). All accessed fields are confirmed to exist:
RoleAssignment.Id,RoleAssignment.Properties.RoleDefinitionId,RoleAssignment.Properties.PrincipalId, andRoleAssignment.Properties.Scope. The verbosity level V(2) is appropriate for detailed logging.
StranDutton
force-pushed
the
BED-4602-modify-verbose-log-output
branch
from
86ea416 to
ef18db4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cmd/list-subscription-user-access-admins.go (1)
91-91: LGTM! Logging change successfully reduces verbosity.The modification correctly logs only key fields (id, principalId, scope, roleId) instead of the entire object, achieving the PR's objective. The field selection is appropriate for tracing subscription user access admins.
Optional: Consider line length for readability.
The log statement spans over 240 characters. If preferred, you could split this across multiple lines or extract fields into variables for improved readability, though this is purely a stylistic preference.
🔎 Optional refactor for readability
-log.V(2).Info("found subscription user access admin", "id", subscriptionUserAccessAdmin.UserAccessAdmin.Id, "principalId", subscriptionUserAccessAdmin.UserAccessAdmin.Properties.PrincipalId, "scope", subscriptionUserAccessAdmin.UserAccessAdmin.Properties.Scope, "roleId", subscriptionUserAccessAdmin.UserAccessAdmin.Properties.RoleDefinitionId) +admin := subscriptionUserAccessAdmin.UserAccessAdmin +log.V(2).Info("found subscription user access admin", + "id", admin.Id, + "principalId", admin.Properties.PrincipalId, + "scope", admin.Properties.Scope, + "roleId", admin.Properties.RoleDefinitionId)cmd/list-key-vault-role-assignments.go (1)
109-109: Consider adding principalId and roleDefinitionId to the log output.While the current fields identify the assignment and vault, including
principalId(who is assigned) androleDefinitionId(what role) would provide more actionable context when reviewing logs, especially since the PR description mentions principalId as an example field to include.🔎 Proposed enhancement
-log.V(2).Info("found key vault role assignment", "id", keyVaultRoleAssignment.RoleAssignment.Id, "keyVaultId", keyVaultRoleAssignment.KeyVaultId) +log.V(2).Info("found key vault role assignment", "id", keyVaultRoleAssignment.RoleAssignment.Id, "keyVaultId", keyVaultRoleAssignment.KeyVaultId, "principalId", keyVaultRoleAssignment.RoleAssignment.Properties.PrincipalId, "roleDefinitionId", keyVaultRoleAssignment.RoleAssignment.Properties.RoleDefinitionId)cmd/list-role-eligibility-schedule-instance.go (1)
75-75: LGTM! Logging change successfully reduces verbosity.The refactored log statement now emits only key identifiers (
id,roleDefinitionId,principalId) instead of the full object, which aligns perfectly with the PR objective to reduce log volume while maintaining traceability.Optionally, consider including
DirectoryScopeIdfor additional context, as it represents the scope of the role assignment and could be valuable for debugging permission-related issues:💡 Optional enhancement
- log.V(2).Info("found unified role eligibility instance schedule", "id", item.Ok.Id, "roleDefinitionId", item.Ok.RoleDefinitionId, "principalId", item.Ok.PrincipalId) + log.V(2).Info("found unified role eligibility instance schedule", "id", item.Ok.Id, "roleDefinitionId", item.Ok.RoleDefinitionId, "principalId", item.Ok.PrincipalId, "directoryScopeId", item.Ok.DirectoryScopeId)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.gitignorecmd/list-app-owners.gocmd/list-app-role-assignments.gocmd/list-apps.gocmd/list-automation-account-role-assignments.gocmd/list-automation-accounts.gocmd/list-container-registries.gocmd/list-container-registry-role-assignments.gocmd/list-device-owners.gocmd/list-devices.gocmd/list-function-app-role-assignments.gocmd/list-function-apps.gocmd/list-group-members.gocmd/list-group-owners.gocmd/list-groups.gocmd/list-key-vault-role-assignments.gocmd/list-key-vaults.gocmd/list-logic-app-role-assignments.gocmd/list-logic-apps.gocmd/list-managed-cluster-role-assignments.gocmd/list-managed-clusters.gocmd/list-management-group-role-assignments.gocmd/list-management-groups.gocmd/list-resource-group-role-assignments.gocmd/list-resource-groups.gocmd/list-role-assignments.gocmd/list-role-eligibility-schedule-instance.gocmd/list-roles.gocmd/list-service-principal-owners.gocmd/list-service-principals.gocmd/list-storage-account-role-assignments.gocmd/list-storage-accounts.gocmd/list-storage-containers.gocmd/list-subscription-owners.gocmd/list-subscription-role-assignments.gocmd/list-subscription-user-access-admins.gocmd/list-subscriptions.gocmd/list-tenants.gocmd/list-users.gocmd/list-virtual-machine-role-assignments.gocmd/list-virtual-machines.gocmd/list-vm-scale-set-role-assignments.gocmd/list-vm-scale-sets.gocmd/list-web-app-role-assignments.gocmd/list-web-apps.go
✅ Files skipped from review due to trivial changes (1)
- cmd/list-web-apps.go
🚧 Files skipped from review as they are similar to previous changes (27)
- cmd/list-devices.go
- cmd/list-app-owners.go
- cmd/list-function-apps.go
- cmd/list-subscription-role-assignments.go
- cmd/list-storage-account-role-assignments.go
- cmd/list-resource-group-role-assignments.go
- cmd/list-service-principals.go
- cmd/list-group-owners.go
- cmd/list-managed-cluster-role-assignments.go
- cmd/list-vm-scale-sets.go
- cmd/list-web-app-role-assignments.go
- cmd/list-tenants.go
- cmd/list-device-owners.go
- cmd/list-virtual-machine-role-assignments.go
- cmd/list-storage-accounts.go
- cmd/list-container-registries.go
- cmd/list-virtual-machines.go
- cmd/list-automation-accounts.go
- cmd/list-service-principal-owners.go
- cmd/list-managed-clusters.go
- cmd/list-group-members.go
- cmd/list-users.go
- cmd/list-function-app-role-assignments.go
- cmd/list-groups.go
- cmd/list-logic-app-role-assignments.go
- cmd/list-container-registry-role-assignments.go
- cmd/list-vm-scale-set-role-assignments.go
🧰 Additional context used
🧬 Code graph analysis (3)
cmd/list-key-vault-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-management-group-role-assignments.go (1)
models/azure/role_assignment.go (1)
RoleAssignment(31-43)
cmd/list-apps.go (1)
constants/misc.go (1)
DisplayName(29-29)
🔇 Additional comments (15)
.gitignore (1)
133-134: LGTM!The addition of
.idea/**/go.imports.xmlis appropriately placed in the JetBrains section and follows the existing glob pattern convention. This prevents JetBrains IDE plugin metadata from being tracked, reducing unnecessary artifacts in version control.cmd/list-resource-groups.go (1)
105-105: LGTM! Logging improvement aligns with PR objectives.The change from logging the entire
resourceGroupobject to logging only theIdandNamefields effectively reduces log verbosity while preserving essential identifying information. This is sufficient for debugging and tracing enumerated resource groups, and the full object still flows through the pipeline for data processing.cmd/list-management-groups.go (1)
74-74: LGTM! Logging refinement aligns with PR objective.The change from logging the entire
item.Okobject to logging onlyidandnamefields effectively reduces verbose log output while preserving key identifying information needed for debugging.cmd/list-subscriptions.go (1)
95-95: LGTM! Improved logging granularity.The change successfully reduces log verbosity by logging only the subscription ID instead of the entire subscription object, while maintaining the same information value for troubleshooting.
cmd/list-apps.go (1)
73-73: LGTM! Enhanced structured logging.The change provides better structured logging by emitting individual fields (id and name) instead of the entire application object, making logs more readable and significantly smaller.
cmd/list-automation-account-role-assignments.go (1)
113-113: LGTM! More concise role assignment logging.The structured logging approach captures the essential identifiers (id and roleDefinitionId) without dumping the entire role assignment object, reducing log size while maintaining debugging utility.
cmd/list-app-role-assignments.go (1)
105-105: LGTM! Well-structured app role assignment logging.The logging now includes three key fields (id, appRoleId, and name) that provide sufficient context for troubleshooting while dramatically reducing log size compared to logging the entire object.
cmd/list-logic-apps.go (1)
114-114: LGTM! Cleaner logic app logging.The change reduces log verbosity by emitting only the id and name fields instead of the entire logic app object, maintaining essential debugging information while significantly reducing log size.
cmd/list-roles.go (1)
73-73: LGTM! Enhanced role logging with structured fields.The structured logging now includes three relevant fields (id, displayName, isEnabled) that provide comprehensive context for roles, making logs more useful for troubleshooting while avoiding the overhead of logging entire role objects.
cmd/list-subscription-owners.go (1)
91-91: LGTM! Improved subscription owner logging.The change provides structured logging with three key fields (principalId, role, and subscriptionId) that capture the essential information about subscription owners, reducing log size while maintaining full debugging capability.
cmd/list-key-vaults.go (1)
108-108: LGTM!The selective field logging (id, name, tenantId) provides sufficient identifying information while reducing log verbosity as intended.
cmd/list-storage-containers.go (1)
113-113: LGTM! Logging change achieves PR objective.The change successfully reduces verbose log output by logging only the key identifying fields (
id,name,tenant) instead of the entirestorageContainerobject. This maintains sufficient context for debugging while significantly reducing log size.cmd/list-management-group-role-assignments.go (1)
109-109: LGTM - Effective reduction of log verbosity!The change successfully replaces full-object logging with targeted field-level logging, significantly reducing log size while preserving essential debugging information (id, role definition, principal, and scope). All field paths exist on the struct and the code is safe—the logging is only reached when
item.Erroris nil.cmd/list-role-eligibility-schedule-instance.go (1)
22-24: These import additions appear to be marked incorrectly in the diff.The imports
os,os/signal, andtimeat lines 22-24 are marked as additions, but they're already present in the current code and actively used in lines 47, 52, and 56 (viasignal.NotifyContext(),os.Interrupt,os.Kill,time.Now(), andtime.Since()). Git history shows these imports are not new to this commit. Either these imports were reorganized rather than newly added, or the diff is incorrectly marking them as additions.cmd/list-role-assignments.go (1)
109-109: The logging statement on line 109 is safe and does not require a nil check. TheRoleDefinitionfield is a value type (UnifiedRoleDefinition, not a pointer), so when the API response does not include the roleDefinition field, it is zero-initialized with an empty string forDisplayName. Accessingitem.Ok.RoleDefinition.DisplayNamewill return an empty string rather than panic.Likely an incorrect or invalid review comment.
Currently, when verbose logs are enabled, the output is extremely bloated. This was caused by the log statements adding the entire response returned from Azure/Entra instead of only what is necessary. Each action performed by the app has has its verbose log modified to only include a couple of key details
id,name,principalId, etc., rather than the entire object.To test: run AzureHound with Verbose logs enabled and compare the new log output to the previous log output - the new one should be significantly smaller in size.
Note
Demo TBA, will supply log file size comparison
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.