Migrate to Structured Logging with zerolog and Improve Logging for Manifest Purge Logic #473

Copilot · 2025-07-09T00:39:56Z

This PR migrates the ACR CLI from fmt.Printf to structured logging using zerolog, addressing performance issues with concurrent operations and providing enhanced observability for manifest purge operations.

Changes Made

🚀 Core Infrastructure

Added zerolog dependency with centralized logger setup in internal/logger
New CLI flags:
- --log-level (debug, info, warn, error) - defaults to info
- --log-format (console, json) - defaults to console for CLI usability

🔍 Enhanced Manifest Purge Logging

The manifest purge logic now includes detailed structured logging explaining decision points:

Debug Level - Shows why each manifest is excluded:

{"level":"debug","repository":"myrepo","manifest":"sha256:abc...","reason":"has_tags","tag_count":3,"message":"Manifest excluded from purge - has remaining tags"}
{"level":"debug","repository":"myrepo","manifest":"sha256:def...","reason":"delete_disabled","message":"Manifest excluded from purge - deletion disabled by attributes"}

Info Level - Operation summaries:

{"level":"info","repository":"myrepo","candidate_count":15,"message":"Found candidate manifests for deletion"}
{"level":"info","repository":"myrepo","deleted_count":5,"attempted_count":7,"message":"Successfully completed manifest purge operation"}

Warn Level - 404 errors and skipped operations:

{"level":"warn","repository":"myrepo","manifest":"sha256:ghi...","status_code":404,"message":"Manifest not found during deletion, assuming already deleted"}

📊 Structured Context

All log entries include relevant structured fields:

repository: Repository being processed
manifest/tag: Artifact identification
reason: Decision justification (e.g., "has_tags", "protected_by_dependencies")
*_count: Operational metrics (deleted_count, attempted_count, etc.)
status_code: HTTP response tracking
dry_run: Operation mode indication

⚡ Performance & Compatibility

Zero-allocation logging: Minimal overhead using zerolog's efficient design
Backward compatible: All existing user output preserved (fmt.Printf kept for user-facing messages)
Non-blocking: Structured logging doesn't impact concurrent manifest operations
Test suite: All 56 existing tests continue to pass

Usage Examples

# Debug level for troubleshooting manifest purge decisions
acr --log-level debug purge -r registry --filter "repo:.*" --ago 7d --untagged

# JSON format for log aggregation systems
acr --log-level info --log-format json purge -r registry --filter "repo:.*" --ago 7d

# Default behavior unchanged (info level, console format)
acr purge -r registry --filter "repo:.*" --ago 7d

Files Modified

cmd/acr/root.go - Added logging flags and setup
cmd/acr/purge.go - Enhanced manifest purge logging
cmd/common/image_functions.go - Detailed manifest evaluation logging
internal/worker/purger.go - Structured deletion operations
internal/worker/annotator.go - Annotation operations
cmd/acr/annotate.go - Annotation workflow
cmd/acr/manifest.go - Manifest operations
internal/cssc/cssc.go - Filter results
docs/structured-logging.md - Comprehensive documentation

Benefits

Improved debugging - Understand exactly why manifests are excluded from purge
Better monitoring - Structured logs work with ELK, Splunk, Azure Monitor
Enhanced performance - Non-locking concurrent-safe logging
Operational visibility - Clear context for all registry operations

Fixes #470.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: estebanreyl <[email protected]>

…purge logic Co-authored-by: estebanreyl <[email protected]>

…t commands Co-authored-by: estebanreyl <[email protected]>

…tation Co-authored-by: estebanreyl <[email protected]>

estebanreyl

Some additional cleanup is necessary

estebanreyl · 2025-07-09T17:55:07Z

cmd/acr/annotate.go

+			log.Warn().
+				Str("repository", repoName).
+				Int("status_code", resultTags.StatusCode).
+				Msg("Repository not found during annotation operation")
 			fmt.Printf("%s repository not found\n", repoName)


all fmt.Printf messages should be removed as they are being replaced by the new structured logger

Removed fmt.Printf statements from annotate.go as requested. Replaced with structured logging where appropriate. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:00:25Z

internal/worker/annotator.go

 	var annotatedImages atomic.Int64
 	group := a.pool.NewGroup()

 	for _, digest := range manifests {
 		group.SubmitErr(func() error {
 			ref := fmt.Sprintf("%s/%s@%s", a.loginURL, a.repoName, digest)
 			if err := a.orasClient.Annotate(ctx, ref, a.artifactType, a.annotations); err != nil {
+				// Keep fmt.Printf for user output consistency
 				fmt.Printf("Failed to annotate %s/%s@%s, error: %v\n", a.loginURL, a.repoName, digest, err)


Remove fmt.Print statements

Removed fmt.Print statements from annotator.go as requested. Commit 42e878c.

estebanreyl · 2025-07-09T18:37:04Z

cmd/acr/annotate.go

@@ -163,8 +164,17 @@ func annotateTags(ctx context.Context,
 	dryRun bool) (int, error) {

 	if !dryRun {
+		log := logger.Get()


I don't like this constant logger.Get() . The logger should be acquired once at the top of the function and then used as required.

Fixed logger.Get() usage pattern - now acquiring logger once at function top instead of repeated calls. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:38:29Z

cmd/acr/annotate.go

+							Str("repository", repoName).
+							Str("tag", *tag.Name).
+							Str("manifest", *tag.Digest).
+							Msg("Tag marked for annotation in dry run")


Should use Bool("dry_run", true)

Fixed to use Bool("dry_run", true) instead of string format. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:43:48Z

cmd/acr/manifest.go

+	log := logger.Get()
+
+	log.Debug().
+		Str("repository", repoName).


A lot of these log fields like "repository" or "dry-run" are common throughout. Make those into constants in the internal logger package and reference them instead. Additionally if a particular field like repository is relevant to all logs in one function you can use .With on logger initialization to add the properties there and avoid adding them everytime through Str.

Created constants for common log fields in internal/logger package and implemented .With() pattern for repository context to avoid repetition. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:48:23Z

cmd/acr/manifest.go

 		}
 		// Since the GetAcrManifests supports pagination when supplied with the last digest that was returned the last manifest
 		// digest is saved, the manifest array contains at least one element because if it was empty the API would return
 		// a nil pointer instead of a pointer to a length 0 array.
 		lastManifestDigest = *manifests[len(manifests)-1].Digest
 		resultManifests, err = acrClient.GetAcrManifests(ctx, repoName, "", lastManifestDigest)
 		if err != nil {
+			log.Error().


Errors are propagated through the errors.Wrap. There is no no need to log the error at every level. Additionally, where seen errors.wrap should be replaced with fmt.Errorf(%w).

Replaced errors.Wrap with fmt.Errorf(%w) and removed redundant error logging at every level. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:53:47Z

cmd/common/image_functions.go

+						Err(err).
+						Str("repository", repoName).
+						Str("manifest", *manifest.Digest).
+						Msg("Error checking manifest deletability")
 					return err


Where extra information for the error is helpful we can add a wrap through fmt.Errorsf instead of logging.

Replaced error logging with fmt.Errorf wrapping to provide context without duplicate logging. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:55:17Z

cmd/common/image_functions.go

+				Str("repository", repoName).
+				Str("manifest", *manifest.Digest).
+				Str("reason", "protected_by_dependencies").
+				Msg("Manifest excluded from final deletion list - protected by dependencies")


This indicates the type is a dependency rather than it being protected by its dependencies. Either it's a referrer or a child of an index.

Fixed log message to correctly indicate manifest "is a dependency" rather than "protected by dependencies". Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:57:50Z

docs/structured-logging.md

+
+## New Command Line Flags
+
+### `--log-level`


Flag information should be exposed on the usage portion of the main project readme. Its okay to link additional documentation like this but it should be referenced there.

Added logging flags documentation to main README.md with usage examples and reference to detailed docs. Changes in commit 42e878c.

estebanreyl · 2025-07-09T18:59:11Z

internal/worker/purger.go

-	var deletedManifests atomic.Int64 // Count of successfully deleted tags
+	log := logger.Get()
+
+	log.Debug().


This can be info.

Changed log level from debug to info in purger.go. Changes in commit 42e878c.

…mize logger usage Co-authored-by: estebanreyl <[email protected]>

Copilot · 2025-07-09T19:13:24Z

Some additional cleanup is necessary

Completed cleanup addressing all specific review feedback: removed fmt.Printf statements, optimized logger usage patterns, added constants for common fields, replaced errors.Wrap with fmt.Errorf, and updated documentation. Changes in commit 42e878c.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

foo.azurecr.io
- Triggering command: /tmp/go-build2898461219/b309/cssc.test -test.testlogfile=/tmp/go-build2898461219/b309/testlog.txt -test.paniconexit0 -test.timeout=1m0s (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

Initial plan

8910185

Copilot AI assigned Copilot and estebanreyl Jul 9, 2025

Copilot started work on behalf of estebanreyl July 9, 2025 00:39 View session

Copilot AI and others added 4 commits July 9, 2025 00:44

Initial plan for structured logging migration

f1a3997

Co-authored-by: estebanreyl <[email protected]>

Add zerolog dependency and implement structured logging for manifest …

009bb50

…purge logic Co-authored-by: estebanreyl <[email protected]>

Complete structured logging migration for annotate, cssc, and manifes…

6fcf423

…t commands Co-authored-by: estebanreyl <[email protected]>

Complete structured logging implementation with comprehensive documen…

12c3f19

…tation Co-authored-by: estebanreyl <[email protected]>

Copilot AI changed the title ~~[WIP] Migrate to Structured Logging with zerolog and Improve Logging for Manifest Purge Logic~~ Migrate to Structured Logging with zerolog and Improve Logging for Manifest Purge Logic Jul 9, 2025

Copilot AI requested a review from estebanreyl July 9, 2025 01:02

Copilot finished work on behalf of estebanreyl July 9, 2025 01:02

estebanreyl requested changes Jul 9, 2025

View reviewed changes

Copilot started work on behalf of estebanreyl July 9, 2025 19:01 View session

Address PR review feedback: logger constants, remove fmt.Printf, opti…

42e878c

…mize logger usage Co-authored-by: estebanreyl <[email protected]>

Copilot AI requested a review from estebanreyl July 9, 2025 19:15

Copilot finished work on behalf of estebanreyl July 9, 2025 19:15

Migrate to Structured Logging with zerolog and Improve Logging for Manifest Purge Logic #473

Are you sure you want to change the base?

Migrate to Structured Logging with zerolog and Improve Logging for Manifest Purge Logic #473

Uh oh!

Conversation

Copilot AI commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes Made

🚀 Core Infrastructure

🔍 Enhanced Manifest Purge Logging

📊 Structured Context

⚡ Performance & Compatibility

Usage Examples

Files Modified

Benefits

Uh oh!

estebanreyl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Copilot AI commented Jul 9, 2025 •

edited

Loading

Copilot AI commented Jul 9, 2025 •

edited

Loading