fix(transform): return an error on non-2xx in send_http_post

[Steven Patterson (stevepatterson5)]

Description

Two changes:

1. send_http_post now returns an error on any non-2xx response. It previously discarded the response and never inspected the status code. A bounded 512-byte prefix of the body is retained so the failure can explain itself; the limit keeps server-echoed payload data out of logs and errors. The existing debug log is unchanged and still emitted before the return.
2. CODEOWNERS now points at @LiveRamp/seceng instead of @brexhq/substation, a team that does not exist in this org.

Motivation and Context

The HTTP client is retryablehttp.NewClient() with stock defaults. Its baseRetryPolicy retries transport errors, 429, and 5xx (except 501) — everything else returns false, nil. So every 4xx except 429 came back with err == nil, and because the status code was never checked, the batch was silently discarded: no error, no 5xx metric, no dead-letter message.

This is not hypothetical. In the seceng LogScale pipelines it silently dropped 429,030 events over 7 days across 10 sources, vault-audit alone accounting for ~306k (~994 MB). Every one was an HTTP 400. The pipelines reported perfect health throughout.

That figure is a floor, not a total — the only trace was a Debug-level log line, so sources running with substation_debug = false (including the largest, at 733 TB) produced no evidence at all.

The cause of the upstream 400s is still unknown, precisely because the response body was thrown away. It is not payload size: identical batch sizes both succeed and fail, and 253 of 400 sampled successful sends were >10 MB. The 400s return in ~80 ms versus ~17 s for a successful ingest, which points at request-level validation rather than capacity. This change is what will surface the reason.

On CODEOWNERS: GitHub silently ignores unresolvable owner entries, so the require_code_owner_reviews branch protection setting had no owner to enforce and the rule was inert. @LiveRamp/seceng has since been granted access to this repository, so the entry now resolves.

How Has This Been Tested?

• gofmt, go build ./..., go vet ./transform/ all clean.
• Full suite green: TZ=UTC go test -timeout 30s ./... — 12 packages. Without TZ=UTC, TestTimeToString and TestTransform/time_to_string fail on any host not in UTC. Pre-existing and unrelated to this change; CI and the container build both run UTC.
• Added transform/send_http_post_test.go covering 200, 201, 400, 401, 404. Verified it actually catches the bug: reverting only the source change while keeping the test yields expected an error for status 400, got nil for all three error cases.
• 429 and 5xx are deliberately excluded from the test — exercising them would incur the client's full retry backoff and exceed the 30s timeout used by CI and by seceng-containers/containers/substation/Dockerfile.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

Deployment note

Merging this does not change production. The image is built by LiveRamp/seceng-containers, which pins ARG SUBSTATION_COMMIT (currently d1a0320); that needs bumping to this commit to trigger a Jenkins build, then local.substation_sha bumped in the infrastructure repo.

When it does reach production, the silent drops become loud failures — 500 → Pub/Sub retry → DLQ. That is the intent, but it will generate DLQ traffic on vault-audit immediately, so it should land when someone can watch it.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Enhanced HTTP transform failure messages by including the HTTP status code and a limited excerpt of the server response body, making errors easier to diagnose.
  • Successful 2xx requests continue to be processed as expected.
• Tests
  • Added automated test coverage for HTTP transform behavior across multiple status codes, verifying both success handling and that error messages include response content for non-2xx cases.

[Snyk Service Account (ops-github-snyk)]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ff9739d-3895-4148-9425-d7ebb4371f45

📥 Commits

Reviewing files that changed from the base of the PR and between e63013d and 2a7602e.

📒 Files selected for processing (1)

• cmd/gcp/function/substation/pubsub.go

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• liveramp/big_data_core (manual)

🚧 Files skipped from review as they are similar to previous changes (1)

• cmd/gcp/function/substation/pubsub.go

---

Walkthrough

Changes

The HTTP transformer now reports non-2xx responses with bounded response-body context and includes status tests. Repository ownership and lint directives are also updated.

HTTP response handling

Layer / File(s)	Summary
Bounded HTTP error handling transform/send_http_post.go, transform/send_http_post_test.go	sendHTTPPost retains a bounded response-body prefix, returns errors for non-2xx statuses, and tests successful and error responses.

Repository maintenance

Layer / File(s)	Summary
CODEOWNERS attribution CODEOWNERS	The listed ownership entry changes from brexhq/substation to LiveRamp/seceng.
Lint directive update cmd/gcp/function/substation/pubsub.go	The pubSubHandler directive is updated to ignore cyclomatic and cognitive complexity.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: returning an error for non-2xx responses in send_http_post.
Description check	✅ Passed	The description follows the template and includes summary, motivation, testing, change type, and checklist details.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@transform/send_http_post.go`:
- Around line 217-219: Update the non-2xx error construction in the HTTP POST
flow to avoid interpolating the sensitive url value into the returned error.
Preserve the transform ID, status code, and trimmed response body, while
omitting the URL or using an established redaction helper if one exists.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f1530483-eb67-4c6c-aae4-7dcdf0cde5ae

📥 Commits

Reviewing files that changed from the base of the PR and between 6bf64cb and b965b30.

📒 Files selected for processing (3)

• CODEOWNERS
• transform/send_http_post.go
• transform/send_http_post_test.go

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• liveramp/big_data_core (manual)