Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/snap/cmd_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ var shortChangesHelp = i18n.G("List system changes")
var shortTasksHelp = i18n.G("List a change's tasks")
var longChangesHelp = i18n.G(`
The changes command displays a summary of system changes performed recently.

For more details, see /var/log/snapd/changes.log
`)
var longTasksHelp = i18n.G(`
The tasks command displays a summary of tasks associated with an individual
Expand Down
6 changes: 6 additions & 0 deletions dirs/dirs.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ var (

SnapdMaintenanceFile string

SnapdLogDir string
SnapdChangesLog string

SnapdStoreSSLCertsDir string
SnapdPKIV1Dir string
SystemCertsDir string
Expand Down Expand Up @@ -632,6 +635,9 @@ func SetRootDir(rootdir string) {
SnapIconsPoolDir = filepath.Join(SnapCacheDir, "icons-pool")
SnapIconsDir = filepath.Join(SnapCacheDir, "icons")

SnapdLogDir = filepath.Join(rootdir, "/var/log/snapd")
SnapdChangesLog = filepath.Join(SnapdLogDir, "changes.log")

SnapSeedDir = SnapSeedDirUnder(rootdir)
SnapDeviceDir = SnapDeviceDirUnder(rootdir)

Expand Down
190 changes: 190 additions & 0 deletions overlord/changeslogger/changeslogger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2026 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package changeslogger

import (
"fmt"
"os"
"path/filepath"
"sync"
"time"

"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/overlord/state"
)

// ChangeInfo stores immutable information about a change for logging
type ChangeInfo struct {
ID string
Kind string
Summary string
Status string
SpawnTime time.Time
ReadyTime time.Time
Error string
}

// ExtractChangeInfo extracts necessary information from a change (must be called with state locked)
func ExtractChangeInfo(chg *state.Change) ChangeInfo {
info := ChangeInfo{
ID: chg.ID(),
Kind: chg.Kind(),
Summary: chg.Summary(),
Status: chg.Status().String(),
SpawnTime: chg.SpawnTime(),
ReadyTime: chg.ReadyTime(),
}
if chg.Err() != nil {
info.Error = chg.Err().Error()
}
return info
}

// Manager logs changes to a file for audit/monitoring purposes
type Manager struct {
state *state.State
mu sync.Mutex
seenChanges map[string]ChangeInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
seenChanges map[string]ChangeInfo
seenChanges map[string]ChangeInfo // indexed by change ID

changeLogPath string
retryCount map[string]int
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The retryCount field is defined but never incremented. Task retry counts are tracked internally by the state machine, not by changes themselves. Since this field is unused, it should be removed along with its references at lines 153-156 in the logChange method.

Copilot uses AI. Check for mistakes.
}

// New creates a new ChangesLogger manager
func New(s *state.State) *Manager {
return &Manager{
state: s,
seenChanges: make(map[string]ChangeInfo),
changeLogPath: dirs.SnapdChangesLog,
retryCount: make(map[string]int),
}
}

// Ensure checks for any state changes and logs them
func (m *Manager) Ensure() error {
m.mu.Lock()
defer m.mu.Unlock()

m.state.Lock()
changes := m.state.Changes()

// Extract info for all changes while holding lock
changeInfos := make(map[string]ChangeInfo)
for _, chg := range changes {
changeInfos[chg.ID()] = ExtractChangeInfo(chg)
}
m.state.Unlock()

// Track which change IDs we've seen this pass
currentChangeIDs := make(map[string]bool)

for id, info := range changeInfos {
currentChangeIDs[id] = true

// Check if this is a new change or if it has changed status
if oldInfo, seen := m.seenChanges[id]; !seen || oldInfo.Status != info.Status {
// Log the change
if err := m.logChange(info); err != nil {
// Log the error but don't fail the ensure
logger.Noticef("Failed to log change %s: %v", id, err)
}
// Store the info to compare next time
m.seenChanges[id] = info
}
}

// Clean up old tracked changes that are no longer in state
for id := range m.seenChanges {
if !currentChangeIDs[id] {
delete(m.seenChanges, id)
delete(m.retryCount, id)
}
}

return nil
}

// logChange writes a change entry to the log file in APT history.log format
func (m *Manager) logChange(info ChangeInfo) error {
// Ensure the directory exists
logDir := filepath.Dir(m.changeLogPath)
if err := os.MkdirAll(logDir, 0755); err != nil {
return fmt.Errorf("cannot create log directory: %v", err)
}

// Open the log file for appending
f, err := os.OpenFile(m.changeLogPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644)
if err != nil {
return fmt.Errorf("cannot open log file: %v", err)
}
defer f.Close()

// Write the change entry as Key: Value pairs
lines := []string{
fmt.Sprintf("Timestamp: %s", time.Now().Format(time.RFC3339)),
fmt.Sprintf("ID: %s", info.ID),
fmt.Sprintf("Kind: %s", info.Kind),
fmt.Sprintf("Summary: %s", info.Summary),
fmt.Sprintf("Status: %s", info.Status),
fmt.Sprintf("SpawnTime: %s", info.SpawnTime.Format(time.RFC3339)),
}

if !info.ReadyTime.IsZero() {
lines = append(lines, fmt.Sprintf("ReadyTime: %s", info.ReadyTime.Format(time.RFC3339)))
}

retryCount := m.retryCount[info.ID]
if retryCount > 0 {
lines = append(lines, fmt.Sprintf("RetryCount: %d", retryCount))
}

if info.Error != "" {
lines = append(lines, fmt.Sprintf("Error: %s", info.Error))
}

// Write all lines followed by a blank line separator
for _, line := range lines {
if _, err := f.WriteString(line + "\n"); err != nil {
return fmt.Errorf("cannot write log entry: %v", err)
}
}

// Write blank line separator between entries
if _, err := f.WriteString("\n"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use the yaml --- record separator? We could then treat the whole file as a YAML and automatically be machine readable.

Copy link
Author

Choose a reason for hiding this comment

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

Seems reasonable to me. My goal was to keep the file as plain-texty as possible so it wouldn't require special tools to parse and would therefore be trivially scriptable with native tools that come bundled with most distributions.

Separating by --- seems reasonable to me but do you think it's fine to keep the extension as .log and the rest of the file simple (e.g., no nested mappings, sequences, or other YAML-isms)?

return fmt.Errorf("cannot write log separator: %v", err)
}

return nil
}

// StartUp performs any necessary initialization
func (m *Manager) StartUp() error {
// Ensure the log directory exists on startup
logDir := filepath.Dir(m.changeLogPath)
if err := os.MkdirAll(logDir, 0755); err != nil {
return fmt.Errorf("cannot create changes log directory: %v", err)
}

if stat, err := os.Stat(m.changeLogPath); err == nil {
logger.Debugf("Changes log file already exists at %q (size: %d)", m.changeLogPath, stat.Size())
}

Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

Log files can grow unbounded without rotation. The PR does not include logrotate configuration or any mechanism to prevent the changes.log file from growing indefinitely. Consider adding a logrotate configuration file in the data/ directory or documenting the need for one.

Suggested change
// Note: this manager does not implement log rotation for the changes log.
// The file at dirs.SnapdChangesLog is expected to be managed by an external
// log rotation mechanism (for example, a logrotate configuration in the
// data/ directory) to prevent unbounded growth over time.

Copilot uses AI. Check for mistakes.
return nil
}
Loading