Skip to content

feat: abstraction#22

Merged
mingrammer merged 14 commits into
daangn:mainfrom
mingrammer:feat/unified-metric-abstraction
Apr 21, 2026
Merged

feat: abstraction#22
mingrammer merged 14 commits into
daangn:mainfrom
mingrammer:feat/unified-metric-abstraction

Conversation

@mingrammer

Copy link
Copy Markdown
Contributor

No description provided.

BREAKING CHANGE: Moves module to v2 path (github.com/daangn/autopprof/v2).

## What's new

- `Metric` interface unifies CPU / Mem / Goroutine / custom metrics behind
  one contract (`Name`, `Threshold`, `Interval`, `Query`, `Collect`). Built-in
  watchers become pre-defined Metric implementations.
- Runtime `Register` / `Unregister` API for adding / removing user metrics
  after `Start` — useful when a metric lives inside a struct whose lifecycle
  begins after `autopprof.Start()` (connection pools, queue handlers, …).
- `Option.Metrics` for start-time registration of user metrics.
- `Option.App` becomes the single source for the `<app>` filename segment.
- `Reporter` interface collapses to a single `Report(ctx, r, ReportInfo)`
  method. `ReportInfo.MetricName` lets implementations route without parsing
  filenames.
- `CollectResult.Reader == nil` lets Metric.Collect opt out of the Reporter
  call for side-effect-only hooks; empty Filename / Comment fall back to
  autopprof-generated defaults.
- New convenience constructor `NewMetric` for ad-hoc metrics.

## Removed / breaking

- `Option.ReportBoth` (use `ReportAll`).
- `SlackReporterOption.App` (moved to `Option.App`).
- `SlackReporterOption.Channel` (Slack API dropped channel-name uploads).
- `Reporter` interface's `ReportCPUProfile` / `ReportHeapProfile` /
  `ReportGoroutineProfile` + `CPUInfo` / `MemInfo` / `GoroutineInfo`.
- `report.CPUProfileFilenameFmt` / `HeapProfileFilenameFmt` /
  `GoroutineProfileFilenameFmt` public constants (internalized).

## Bug fixes bundled

- `Option.ReportAll` and `Option.DisableGoroutineProf` were silently ignored
  in v1 (the values were never assigned to the internal autopprof struct at
  Start time). They now take effect correctly.

## Concurrency notes

- CPU snapshot queue (`cgroupV1/V2`) gains `qMu` for concurrent `CPUUsage`.
- `defaultProfiler` gains `cpuMu` around `pprof.StartCPUProfile` (process-wide
  singleton).
- `stop()` closes `ap.stopC` under `metricsMu` so a racing `Register` sees
  either (a) the open state and spawns its watcher, or (b) the closed state
  and returns `ErrNotStarted` — `wg.Add` can never interleave with `wg.Wait`.
Runs all built-in metrics (CPU / Mem / Goroutine) plus a runtime-
registered custom metric (e2e_counter via autopprof.Register) and
uploads each report to a real Slack channel.

Configure via env:
  SLACK_TOKEN       — Slack bot token with files:write.
  SLACK_CHANNEL_ID  — destination channel ID.
  SLACK_APP         — optional; "<app>" segment (default autopprof-e2e).
  E2E_DURATION      — optional; defaults to 180s so the 24×5s CPU
                      snapshot window has time to warm up.

Includes:
  - examples/e2e/main.go    — load generator + custom metric.
  - examples/e2e/run.sh     — volume-mount docker wrapper.
  - examples/e2e/Dockerfile — self-contained multi-stage image option.
  - examples/e2e/README.md  — expected Slack output + usage.
v2 depends on atomic.Int32/Int64 (introduced in Go 1.19) and go.mod
already declares `go 1.19`. Dropping Go 1.17/1.18 from the matrix and
adding 1.20-1.22 to track the current three-minor-version window.
os.Hostname is only called at threshold breach (not in any hot path),
so the sync.Once cache was over-engineering.
validate is only invoked by Start (linux-only), so keeping it in the
universal option.go made it dead code on non-linux builds. Move it to
option_linux.go; the Option struct itself stays universal so
autopprof_unsupported.go can still reference it.
- Move Option.validate() back into option.go; the unused-on-non-linux
  warning is harmless and the split added a file for no real benefit.
- Rename bootstrapMetrics → registerBuiltinMetrics to match what it
  actually does. Move Option.Metrics registration out to Start() so
  the function's name isn't misleading.
- Drop reservedMetricNames (cascadedRunners already owns built-in names;
  Option.Metrics name collisions are left to the caller).
- Drop customRunners map: Register now just spawns a watcher and
  returns error — the global ap.stopC is the only cancellation path,
  so per-runner stopC and cancel closures are no longer needed.
- Drop Unregister and the related errors
  (ErrMetricAlreadyRegistered / ErrMetricNotRegistered /
  ErrCannotUnregisterBuiltIn / ErrReservedMetricName).
- stop() no longer drains cascadedRunners — the map goes away with ap.
- Update example, e2e, unsupported stubs, and tests accordingly.
cascadedRunners is populated only during Start and read-only
thereafter, so no mutex is needed. While here:

- Flip the watchMetric bool to isBuiltin so the cascade call is
  readable at the site, and fix the swapped argument in
  registerBuiltIn / registerMetric (cascade was not firing).
- Refresh stale doc comments that still referenced Unregister, the
  per-runner stopC, and the reserved-name policy.
Drop comments that only narrate what the code or identifier already
says. Public API docs (Option field descriptions, Metric godoc) are
kept so go doc still reads well.
Separate cpuMetric, memMetric, and goroutineMetric into their own files
for easier navigation. Shared helpers (collectBuiltIn, defaultFilename,
defaultComment, hostnameSafe, reportTimeLayout, MetricName* constants)
remain in metric_builtin.go.
Move hostnameSafe, collectProfile (renamed from collectBuiltIn),
defaultFilename, and defaultComment from metric_builtin.go into
metric.go, and remove the now-empty metric_builtin.go.
@mingrammer mingrammer merged commit 5500f75 into daangn:main Apr 21, 2026
16 of 20 checks passed
@mingrammer mingrammer deleted the feat/unified-metric-abstraction branch April 22, 2026 11:01
mingrammer added a commit that referenced this pull request Apr 22, 2026
@mingrammer mingrammer changed the title feat: unified Metric abstraction with custom metric hooks feat: abstraction Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant