Skip to content

Minor Improvements: Log Level and Metrics Documentation#84

Merged
piceri merged 5 commits intomainfrom
piceri/minor-improvements
Apr 28, 2026
Merged

Minor Improvements: Log Level and Metrics Documentation#84
piceri merged 5 commits intomainfrom
piceri/minor-improvements

Conversation

@piceri
Copy link
Copy Markdown
Contributor

@piceri piceri commented Apr 27, 2026

This change contains the following minor changes:

  • Enable controlling log level via env var
  • Renamed deptracker_post_record_no_attestation to deptracker_post_record_unknown_artifact
  • Add documentation for deptracker_post_record_unknown_artifact_cache_hit to README

piceri added 5 commits April 10, 2026 16:50
Signed-off-by: Eric Pickard <piceri@github.com>
…d_unknown_artifact

Signed-off-by: Eric Pickard <piceri@github.com>
Signed-off-by: Eric Pickard <piceri@github.com>
Signed-off-by: Eric Pickard <piceri@github.com>
@piceri piceri marked this pull request as ready for review April 27, 2026 20:18
@piceri piceri requested a review from a team as a code owner April 27, 2026 20:18
Copilot AI review requested due to automatic review settings April 27, 2026 20:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes small operational and observability updates to deployment-tracker by adding env-configurable logging and adjusting Prometheus metric naming/documentation around unknown artifacts and caching.

Changes:

  • Add LOG_LEVEL environment variable support for controlling slog handler level.
  • Rename the “no attestation” post counter metric to “unknown artifact” and update code/tests to use the new name.
  • Document the unknown-artifact cache hit metric in the README.
Show a summary per file
File Description
pkg/dtmetrics/prom.go Renames the exported counter and Prometheus metric name for unknown-artifact post outcomes.
pkg/deploymentrecord/client.go Updates the 404-path metric increment to use the renamed counter.
pkg/deploymentrecord/client_test.go Updates metric assertions and test struct fields to match the renamed counter.
cmd/deployment-tracker/main.go Adds LOG_LEVEL parsing and configures slog handler level from env var.
README.md Documents LOG_LEVEL and updates/extends metrics documentation for unknown-artifact behavior and cache hits.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

pkg/deploymentrecord/client.go:276

  • The 404 branch currently treats any 404 as a NoArtifactError and increments the ...unknown_artifact metric. That contradicts the NoArtifactError type comment/tests implying it’s specifically for 404s whose body indicates "no artifacts found", and it risks caching/suppressing posts for other 404 causes (bad org/base URL, endpoint changes, etc.). Consider validating the response body/message before returning NoArtifactError (and only then incrementing the unknown-artifact metric); otherwise treat other 404s as client errors.
		case resp.StatusCode == 404:
			// No artifact found - do not retry
			dtmetrics.PostDeploymentRecordUnknownArtifact.Inc()
			slog.Debug("no artifact attestation found, no record created",
				"attempt", attempt,
				"status_code", resp.StatusCode,
				"container_name", record.Name,
				"resp_msg", string(respBody),
				"digest", record.Digest,
			)
			return &NoArtifactError{err: fmt.Errorf("no attestation found for %s", record.Digest)}
  • Files reviewed: 5/5 changed files
  • Comments generated: 4

Comment thread pkg/dtmetrics/prom.go
Comment thread cmd/deployment-tracker/main.go
Comment thread cmd/deployment-tracker/main.go
Comment thread README.md
@piceri piceri merged commit d5ad67e into main Apr 28, 2026
11 checks passed
@piceri piceri deleted the piceri/minor-improvements branch April 28, 2026 15:05
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.

3 participants