Skip to content

obs: port worker observability + buildinfo from api repo (B1) - #8

Merged
mastermanas805 merged 2 commits into
masterfrom
obs/worker-obs-relocate-2026-05-12
May 12, 2026
Merged

mastermanas805 merged 2 commits into
masterfrom
obs/worker-obs-relocate-2026-05-12

Conversation

@mastermanas805

Copy link
Copy Markdown
Member

Summary

Relocates the worker observability code that was merged into the api repo
under api/worker/ (PR #37 there) into its real home in this repo. Switches
from the temporary _obs_stubs/{buildinfo,logctx} packages to the canonical
instant.dev/common/{buildinfo,logctx} packages.

Why

The api repo PR #37 shipped the observability scaffolding for the worker
but in the wrong git history because the orchestrator created the worktree
against InstaNode-dev/api rather than InstaNode-dev/worker. Production
instant-worker k8s Deployment runs an image built from THIS repo, so the
obs code never reached the running worker.

Today's PR #40 on the api repo also did the _obs_stubs -> common switch.
This PR mirrors that fix for the worker.

What ships

  • internal/jobs/middleware.go -- WithObservability[T] generic River-Worker
    wrapper. Stamps tid / trace_id on ctx via logctx, opens optional New
    Relic transaction, logs duration on completion. Fail-open on nil nrApp.
  • internal/jobs/middleware_test.go -- 7 tests + 1 subtest (8 total):
    tid stamping, trace_id missing/preserved, error propagation, nil-NR safe
    (success + failure), delegation of NextRetry/Timeout, int64 formatter.
  • internal/obs/nr.go -- InitNewRelic returning (nil, nil) on missing
    NEW_RELIC_LICENSE_KEY. Never crashes.
  • internal/obs/nr_test.go -- 2 tests (fail-open, nil-safe).
  • main.go -- slog wrapped in logctx.NewHandler("worker", base); NR
    init + Shutdown defer; /healthz now returns commit_id / build_time
    / version from buildinfo; StartWorkers receives nrApp.
  • internal/jobs/workers.go -- StartWorkers signature gains
    nrApp *newrelic.Application. Every river.AddWorker(...) wraps the
    worker via WithObservability(...).
  • Dockerfile -- bumped to golang:1.25-alpine to match go.mod.

Imports refactor (stubs -> common)

instant.dev/worker/internal/_obs_stubs/buildinfo -> instant.dev/common/buildinfo
instant.dev/worker/internal/_obs_stubs/logctx    -> instant.dev/common/logctx

The replace instant.dev/common => ../common directive already in go.mod
makes the canonical imports resolve to the sibling checkout.

Note: instant.dev/common/logctx.NewHandler takes (service, base) not
(service, commit_id, base). The canonical handler reads commit_id from
COMMIT_ID env (fallback "dev"); buildinfo.GitSHA is read separately for
the /healthz payload. To make the log fields agree with /healthz, the k8s
Deployment should set COMMIT_ID=$GIT_SHA env var (not done in this PR).

Tests

$ go test ./internal/jobs/ ./internal/obs/ -count=1 -v
...
--- PASS: TestWithObservability_StampsTIDOnContext (0.00s)
--- PASS: TestWithObservability_SetsTraceIDWhenMissing (0.00s)
--- PASS: TestWithObservability_PreservesExistingTraceID (0.00s)
--- PASS: TestWithObservability_PropagatesError (0.00s)
--- PASS: TestWithObservability_NilNRAppIsSafe (0.00s)
    --- PASS: TestWithObservability_NilNRAppIsSafe/success
    --- PASS: TestWithObservability_NilNRAppIsSafe/failure
--- PASS: TestWithObservability_DelegatesNextRetryAndTimeout (0.00s)
--- PASS: TestJobIDString (0.00s)
... (24 pre-existing tests) ...
ok      instant.dev/worker/internal/jobs        0.768s
--- PASS: TestInitNewRelic_FailOpenOnMissingLicenseKey (0.00s)
--- PASS: TestWaitForConnection_NilSafe (0.00s)
ok      instant.dev/worker/internal/obs         0.364s

34 PASS, 0 FAIL.

Live deploy verification

Image: ghcr.io/mastermanas805/instant-worker:v1.5.0-obs-20ee825 (linux/amd64, 36.9 MB).

$ kubectl rollout status deployment/instant-worker -n instant-infra --timeout=180s
deployment.apps/instant-worker image updated
Waiting for deployment "instant-worker" rollout to finish: 1 out of 2 new replicas...
...
deployment "instant-worker" successfully rolled out

$ kubectl port-forward -n instant-infra pod/instant-worker-f76647fc4-nctcj 18182:8091 &
$ curl http://localhost:18182/healthz
{"ok":true,"service":"instant-worker","commit_id":"20ee825","build_time":"2026-05-12T17:45:54Z","version":"v1.5.0-obs"}

commit_id=20ee825 matches the head SHA -- ldflag injection works end-to-end.

Container name in Deployment spec: worker.

Test plan

  • go test ./... passes
  • go build ./... clean
  • docker buildx build linux/amd64 succeeds
  • image pushed to ghcr.io
  • kubectl rollout completes
  • /healthz returns ldflag-injected commit_id
  • log lines include service + commit_id + trace_id + tid + team_id

Pushback

  1. Cluster had a transient DNS flake during the first rollout attempt
    (postgres-platform.instant.svc.cluster.local failed to resolve, then
    resolved on retry). Not caused by this PR -- pre-existing condition.

  2. instant.dev/common/logctx.NewHandler ignores buildinfo.GitSHA and
    reads commit_id from the COMMIT_ID env var. Until the k8s manifest
    sets COMMIT_ID=$GIT_SHA per deploy, log lines show commit_id=dev
    while /healthz shows the real SHA. Fix: either update the worker k8s
    Deployment env, or change common/logctx to fall back to
    buildinfo.GitSHA -- the latter is the cleaner long-term fix and
    belongs in a common-repo PR.

  3. go mod tidy against the local Go 1.26 toolchain bumped go 1.24.0
    to go 1.25. Bumped Dockerfile to golang:1.25-alpine to match;
    verified the build still produces a working binary.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

(Generated with Claude Code)

Claude (instanode) and others added 2 commits May 12, 2026 23:14
Relocates the observability code that was merged into InstaNode-dev/api
under api/worker/ (PR #37 there) into its real home in this repo.

What ships:
- internal/jobs/middleware.go — WithObservability[T] generic River-Worker
  wrapper that stamps tid/trace_id on ctx and (optionally) opens a New
  Relic transaction per job. Fail-open on nil nrApp.
- internal/jobs/middleware_test.go — 7 tests covering tid stamping,
  trace_id missing/present, error propagation, nil-NR safety, delegation
  of NextRetry/Timeout, plus the int64 formatter.
- internal/obs/nr.go — InitNewRelic + WaitForConnection helpers.
- internal/obs/nr_test.go — 2 tests asserting fail-open contract.
- main.go — slog wrapped in logctx.NewHandler, NR init, /healthz now
  emits commit_id/build_time/version, workers receive nrApp.
- internal/jobs/workers.go — StartWorkers gains nrApp parameter; every
  river.AddWorker call wraps the worker via WithObservability(...).

Critical detail: the api/worker/ PR shipped against TEMPORARY stubs at
instant.dev/worker/internal/_obs_stubs/{buildinfo,logctx}. This relocate
switches both imports to the canonical common packages:
- instant.dev/worker/internal/_obs_stubs/buildinfo -> instant.dev/common/buildinfo
- instant.dev/worker/internal/_obs_stubs/logctx    -> instant.dev/common/logctx

That mirrors today's PR #40 fix on the api repo, where the same stub->common
substitution was applied. The worker module's existing
`replace instant.dev/common => ../common` directive in go.mod makes the
canonical import resolve to the sibling checkout.

go.mod gains:
- github.com/newrelic/go-agent/v3 (direct)

Tests: go test ./... -count=1 — all green, 34 PASS in internal/jobs +
internal/obs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
go.mod ended up at go 1.25 after go mod tidy resolved newrelic / k8s
client-go transitive bounds. Matches the api repo Dockerfile.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mastermanas805
mastermanas805 merged commit f0ad101 into master May 12, 2026
mastermanas805 added a commit that referenced this pull request Jun 2, 2026
- orphan_sweep PASS 4: young-namespace (within grace) + age-lookup-error are
  NOT reaped (#8 grace branches).
- emitDeployFailedAudit: dedup-hit skips the INSERT (#15 idempotency branch).
- dbGracePeriodOpener.TerminateActiveGracePeriod: UPDATE success + error-wrap.
- billing terminal downgrade still succeeds when the grace-close errors (#5
  fail-open warn branch).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805 added a commit that referenced this pull request Jun 2, 2026
…dup, cursor (#78)

* fix(worker): bug-bash batch 2 — grace close, namespace reaper grace, autopsy dedup, cursor

Four confirmed bugs from the 2026-06-02 platform bug bash:

- #5 (P1) billing_reconciler: a terminal Razorpay status downgraded the team
  but left the active payment_grace_periods row open, so payment_grace_reminder
  emitted dunning emails forever and the terminator later re-acted on an
  already-cancelled subscription. Add TerminateActiveGracePeriod to the
  gracePeriodOpener interface (status→'terminated', terminated_at=now()) and
  call it in the terminal-downgrade branch (fail-open).

- #8 (P1) orphan_sweep PASS 4: the customer-namespace reaper excluded 'pending'
  resources from the live-token set AND had no creation-grace, so a sweep
  during two-phase provisioning could DELETE a live, mid-provision namespace.
  Add 'pending' to fetchLiveResourceTokens and a namespace-age grace check
  (skip if younger than orphanNoDBRowGrace) mirroring PASS 3.

- #15 (P2) deploy_failure_autopsy: emitDeployFailedAudit inserted a new
  deploy.failed audit row (new id) on every reconciler retry, and the forwarder
  dedups by audit_id (not deployment) → duplicate failure emails. Make it
  idempotent: skip the INSERT when a deploy.failed row already exists for the
  deployment (metadata->>'deploy_id'). Fail-open on probe error.

- #18 (P2) billing_reconciler scanChargeUndeliverable: jumping the cursor to
  now() on an empty window skipped rows that became visible a moment later
  (clock skew / late commit). Leave the cursor unchanged on count==0 — the 1h
  look-back re-applies and re-scanning the small indexed window is cheap.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(worker): cover bug-bash batch-2 changed lines (100% patch gate)

- orphan_sweep PASS 4: young-namespace (within grace) + age-lookup-error are
  NOT reaped (#8 grace branches).
- emitDeployFailedAudit: dedup-hit skips the INSERT (#15 idempotency branch).
- dbGracePeriodOpener.TerminateActiveGracePeriod: UPDATE success + error-wrap.
- billing terminal downgrade still succeeds when the grace-close errors (#5
  fail-open warn branch).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant