Skip to content

Fix endpoint and OTEL-017 false positives - #67

Open
starkross wants to merge 1 commit into
mainfrom
fix/rule-false-positives
Open

Fix endpoint and OTEL-017 false positives#67
starkross wants to merge 1 commit into
mainfrom
fix/rule-false-positives

Conversation

@starkross

Copy link
Copy Markdown
Owner

Context

Ran augur across a corpus of ~66 real and synthetic collector configs (OTel Helm charts, AWS ADOT, the OTel demo, collector-contrib testdata, plus hand-authored deployments). That surfaced ~56 false positives, concentrated in three rules. This PR fixes them.

Fixes

1. is_env_var missed ${env:VAR}:port (54 false OTEL-033, plus OTEL-018/046)

The helper only matched a value that was wholly an env reference (it required endswith("}")), so ${env:MY_POD_IP}:4317 β€” the Helm charts' standard bind β€” slipped past the guard and tripped OTEL-033/OTEL-018.

Now it matches any value that contains ${env: / ${ENV:, which also covers things like Bearer ${env:TOKEN}.

endpoint: ${env:OTLP_ENDPOINT}      # already skipped
endpoint: ${env:MY_POD_IP}:4317     # now skipped too (was flagged)

2. OTEL-017 fired on loadbalancing despite configured resilience

loadbalancing nests retry_on_failure / sending_queue under protocol.otlp, not at the top level. Taught _exporter_has_alt_retry to look there.

3. OTEL-017 fired on the nop exporter

nop discards data by design β€” retry/queue is meaningless. Added it to the pull-based exclusion set (and to the matching sets for OTEL-048/052 to prevent the same latent FP).

Deliberately left unchanged

OTEL-017 still warns on a bare otlp exporter and on awsxray/awsemf without max_retries. These look like false positives (those exporters enable retry/queue by default), but they are deliberate, test-locked behavior (test_017_warn_otlp_no_retry, test_017_warn_aws*_without_max_retries) β€” a "be explicit about resilience" nudge, not a logic bug. Reversing them is a product decision, so I left them and am flagging it here instead.

Impact on the corpus

Total findings 351 β†’ 295 (βˆ’56). OTEL-033 91 β†’ 37 (every env-var hit gone; the 37 left are real literal-IP / 0.0.0.0 binds). OTEL-017 14 β†’ 12 (loadbalancing + nop removed; the 12 left are the intentional otlp/aws cases).

Tests

Added env-var-with-port pass cases for OTEL-033/018, and nop + nested-loadbalancing pass cases for OTEL-017. regal clean, conftest verify 95/95, go test ./... green.

πŸ€– Generated with Claude Code

Running augur over a corpus of real configs (OTel Helm charts, AWS, demo)
surfaced false positives in three rules.

is_env_var only matched a value that was *wholly* an env reference (it
required endswith "}"), so "${env:MY_POD_IP}:4317" β€” the Helm charts'
standard bind β€” slipped past the guard and tripped OTEL-033/OTEL-018.
Match any value that *contains* ${env:/${ENV:, which also covers headers
like "Bearer ${env:TOKEN}". This removed 54 false OTEL-033 findings on the
helm fixtures with no loss of real coverage.

OTEL-017 (no retry/sending_queue) fired on two exporters that are not at
risk:
- loadbalancing nests retry_on_failure/sending_queue under protocol.otlp,
  not at the top level β€” teach _exporter_has_alt_retry about it.
- nop discards data by design, so retry/queue is meaningless β€” add it to
  the pull-based exclusion set (and to the matching sets for OTEL-048/052).

Left intentionally unchanged: OTEL-017 still warns on a bare otlp exporter
and on awsxray/awsemf without max_retries. Those are deliberate,
test-locked "be explicit about resilience" nudges, not logic bugs.

Tests: env-var-with-port pass cases for OTEL-033/018; nop and nested-
loadbalancing pass cases for OTEL-017. regal clean, conftest 95/95.

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