Skip to content

fix(prometheus): require word boundary when detecting HELP/TYPE comments - #304

Open
detail-app[bot] wants to merge 1 commit into
masterfrom
detail/bug-fix/fix-prometheus-require-word-boundary-when-detectin-8ce207
Open

detail-app[bot] wants to merge 1 commit into
masterfrom
detail/bug-fix/fix-prometheus-require-word-boundary-when-detectin-8ce207

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 16, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Fixes ENG-705

Bug

parseCommentLine in src/signals/prometheus/line_parser.zig dispatched on a bare std.mem.startsWith(rest, "HELP"/"TYPE") with no word-boundary check, so a #-comment whose first token merely began with those letters (e.g. # HELPFUL info, # TYPES: examples) was misclassified as HELP/TYPE metadata. The Prometheus exposition spec tokenizes HELP/TYPE on whitespace, so HELPFUL/TYPES: are single tokens distinct from HELP/TYPE and must be plain ignored comments.

On the production path this violated PolicyStreamingFilter's "comments preserved regardless of policy" contract: misclassified comments are stored as metadata rather than forwarded inline, so they were silently dropped from the proxy output when no later kept sample matched the bogus derived metric name (FUL/S:).

Fix

Require a word boundary — a space, tab, or end-of-comment — immediately after the HELP/TYPE token before dispatching to parseHelpLine/parseTypeLine. Lines such as # HELPFUL info now parse as .comment and forward unchanged; well-formed # HELP <name> … / # TYPE <name> <type> (space- or tab-separated) still parse as metadata.

Added two focused regression tests:

  • a parseLine unit test covering prefix-colliding tokens, tab-separated metadata, and the bare # HELP/# TYPE parse-error case
  • a PolicyStreamingFilter contract test asserting prefix-colliding comments survive a DROP policy, matching how the bug manifested in production

Testing

  • Unit tests, zig fmt --check, ziglint, zig build, and the full suite (zig build test --summary all and task do, including ReleaseSafe) all pass — 521/522 tests passed, 0 failures (1 skipped).
  • Verified end-to-end through the edge-prometheus binary: a local /metrics upstream served the trigger comments (# HELPFUL …, # TYPES: …) interleaved with metrics, proxied under both a no-policy and a DROP ^metric_ config. After the fix both comments are preserved in their original position, and well-formed # HELP/# TYPE metadata re-positions before its matching sample; under the DROP policy metric_* is dropped while the comments survive.
  • Confirmed the regression by temporarily reverting the fix and rebuilding the binary: with the buggy parser the two trigger comments are silently absent from proxied output; restoring the fix reproduces them. (These probe tests were removed before committing.)
  • Could not complete task signoff: task do and task build:safe pass, but task test:s3-e2e requires pulling the minio/minio image, which this sandbox's container registry denies — docker is available and hello-world pulls, but minio/minio is reproducibly denied. That step covers the unrelated S3-dump module and does not affect this fix, so gh signoff was not reached.

Automatic Fixes PRs can be configured here.

@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved at 5d84aff

Macroscope's review found this PR approvable — This is a focused Prometheus parsing fix that preserves valid metadata handling while ensuring ordinary comments beginning with HELP or TYPE remain in the output. The production change is minimal and supported by parser and filtering regression tests.

You can add or adjust custom eligibility rules. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant