Skip to content

fix(logging): downgrade not-in-task-store resync warning to V(3) - #19

Merged
bborbe merged 1 commit into
masterfrom
fix/v3-log
Aug 17, 2026
Merged

fix(logging): downgrade not-in-task-store resync warning to V(3)#19
bborbe merged 1 commit into
masterfrom
fix/v3-log

Conversation

@bborbe

Copy link
Copy Markdown
Owner

The Job informer re-delivers an already-handled terminal Job every ~5 minutes; each redelivery re-enters the missing-task branch of logMissingTask and the old Warning read as a failure. The synthetic failure was already published on the first (real) observation, so the resync is expected noise, not an error.

Found during the 2026-08-10 silent job-failure wedge investigation — the repeating not in task store warnings were a red herring that cost hours of misdiagnosis (the task's own write-up: "a warning is a hypothesis, the artifact it claims to describe is the evidence").

make precommit green (0 lint issues).

The Job informer re-delivers an already-handled terminal Job every ~5
minutes; each redelivery re-enters the missing-task branch of
logMissingTask and the old Warning read as a failure. The synthetic
failure was already published on the first (real) observation, so the
resync is expected noise, not an error. Found during the 2026-08-10
silent job-failure wedge investigation.

@ben-s-pull-request-reviewerben-s-pull-request-reviewerBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Now I have everything I need. Let me perform the Step 4c-sel CLASSIFY and Step 4d-sel ADJUDICATE in-session.

Step 4c-sel CLASSIFY

Changed files: CHANGELOG.md, pkg/job_watcher.go

Applying the Step 4b-i glob filter (judgment rules with triggers matching changed files):

  • changelog/conventional-prefix-required — trigger is CHANGELOG.mdapplicable (diff touches CHANGELOG.md)
  • changelog/unreleased-entry-required — trigger is CHANGELOG.mdapplicable
  • go-cli/slog-not-glog-in-new-projects — mechanical finding already captured it (existing project, exempt)
  • go-context/cancel-check-in-loop — mechanical findings already captured it (SHOULD-level)
  • go-architecture/counterfeiter-directive-on-interface — mechanical findings already captured it
  • go-architecture/new-prefix-constructor-naming — mechanical findings already captured it
  • go-testing/counterfeiter-mocks-required — mechanical findings already captured it

All other judgment rules have triggers that don't match the changed files.

Classify result:

{
"applicable": ["changelog/conventional-prefix-required", "changelog/unreleased-entry-required"],
"skipped": {}
}

Step 4d-sel ADJUDICATE

For each applicable rule, I read the doc and check the diff:

  • changelog/conventional-prefix-required: The CHANGELOG.md bullet is fix(logging): downgrade... — starts with fix:
  • changelog/unreleased-entry-required: The CHANGELOG.md has a ## Unreleased section with a - bullet for this PR ✓

No violations on either applicable rule.

Mechanical findings (already confirmed MUST-tier from pre-computed funnel):

  • go-architecture/counterfeiter-directive-on-interface on JobWatcher at line 29 — interface lacks //counterfeiter:generate
  • go-architecture/new-prefix-constructor-naming on JobFailureReason at line 256 — exported func returning exported type not prefixed New
  • go-composition/no-package-function-calls-in-business-logic — many glog, errors.Wrapf, ctx.Done(), cache.WaitForCacheSync calls in job_watcher.go (informers/watcher infrastructure — low-practical-concern as existing patterns in this codebase)
  • go-cli/slog-not-glog-in-new-projects — glog imported in job_watcher.go (existing project, exempt per rule's own exemption clause)
  • go-testing/counterfeiter-mocks-required — same as counterfeiter-directive finding above
  • go-context/cancel-check-in-loop (SHOULD) — loops over job conditions without ctx.Done() check

Verdict reasoning: The MUST-tier findings are all pre-existing patterns in the informer/watcher infrastructure — they existed before this PR and this PR's change (glog.Warningf → glog.V(3).Infof + explanatory comment) does not introduce any new violations. The SHOULD-tier cancel-check-in-loop findings are pre-existing. This PR is a targeted one-line log-level change with a correct explanatory comment. Approve.


{
"verdict": "approve",
"summary": "PR makes a targeted, correct log-level change: `glog.Warningf` → `glog.V(3).Infof` with an explanatory comment clarifying that the informer re-delivers terminal jobs every ~5 min and the Warning was misread as a failure. CHANGELOG entry follows conventions. All mechanical findings are pre-existing patterns in the informer/watcher layer, not introduced by this change.",
"comments": [
{
"file": "pkg/job_watcher.go",
"line": 29,
"severity": "major",
"message": "JobWatcher interface lacks a preceding //counterfeiter:generate directive, so go generate will not regenerate the fake when the interface drifts. *(rule: go-architecture/counterfeiter-directive-on-interface)*"
},
{
"file": "pkg/job_watcher.go",
"line": 256,
"severity": "major",
"message": "JobFailureReason is an exported function returning an exported type but does not start with 'New'. *(rule: go-architecture/new-prefix-constructor-naming)*"
},
{
"file": "pkg/job_watcher.go",
"line": 14,
"severity": "minor",
"message": "github.com/golang/glog is imported; per the rule's exemption clause, existing projects mid-migration are exempt — no action required. *(rule: go-cli/slog-not-glog-in-new-projects)*"
},
{
"file": "pkg/job_watcher.go",
"line": 230,
"severity": "nit",
"message": "Loop over job.Status.Conditions does not check ctx.Done() each iteration. *(rule: go-context/cancel-check-in-loop)*"
},
{
"file": "pkg/job_watcher.go",
"line": 240,
"severity": "nit",
"message": "Loop over job.Status.Conditions does not check ctx.Done() each iteration. *(rule: go-context/cancel-check-in-loop)*"
},
{
"file": "pkg/job_watcher.go",
"line": 257,
"severity": "nit",
"message": "Loop over job.Status.Conditions does not check ctx.Done() each iteration. *(rule: go-context/cancel-check-in-loop)*"
},
{
"file": "pkg/job_watcher.go",
"line": 327,
"severity": "nit",
"message": "Loop over pod.Status.ContainerStatuses does not check ctx.Done() each iteration. *(rule: go-context/cancel-check-in-loop)*"
},
{
"file": "pkg/job_watcher.go",
"line": 344,
"severity": "nit",
"message": "Loop over pod.Status.ContainerStatuses does not check ctx.Done() each iteration. *(rule: go-context/cancel-check-in-loop)*"
},
{
"file": "pkg/job_watcher.go",
"line": 356,
"severity": "nit",
"message": "Loop over pod.OwnerReferences does not check ctx.Done() each iteration. *(rule: go-context/cancel-check-in-loop)*"
}
],
"concerns_addressed": [
"correctness: log level downgrade from Warning to V(3) with explanatory comment — no new issues introduced by this change"
]
}

@bborbe
bborbe merged commit c8a9a6e into masterAug 17, 2026
1 check passed
@bborbe
bborbe deleted the fix/v3-log branch August 17, 2026 20:28
Sign up for freeto 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

@bborbe