Skip to content

chore: make reporting 413 handling unconditional - #7313

Open
rudder-devbox[bot] wants to merge 4 commits into
masterfrom
harness/PIPE-3324-fix-core-make-reporting-413-han-74f13d57
Open

chore: make reporting 413 handling unconditional#7313
rudder-devbox[bot] wants to merge 4 commits into
masterfrom
harness/PIPE-3324-fix-core-make-reporting-413-han-74f13d57

Conversation

@rudder-devbox

@rudder-devbox rudder-devbox Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

Removes the requestEntityTooLargeHandling feature flag from enterprise reporting so graceful HTTP 413 handling is always active for:

  • default reporting metrics (DefaultReporter.sendMetric)
  • flusher batches (Flusher.sendBatch)
  • error-detail reporting metrics (ErrorDetailReporter.sendEDMetric)

The constructors no longer load Reporting.requestEntityTooLargeHandling, Reporting.flusher.requestEntityTooLargeHandling, or Reporting.errorReporting.requestEntityTooLargeHandling, and tests no longer set the removed flag.

Validation:

  • Integration verification skipped: rudder-server is sha-pinned and not source-verifiable in mini for this reporting-only change; no user-visible runnable surface applies.
  • make -C /workspace/rudder-server fmt — passed
  • go test -count 1 ./enterprise/reporting ./enterprise/reporting/flusher — passed:
    • ok github.com/rudderlabs/rudder-server/enterprise/reporting 15.459s
    • ok github.com/rudderlabs/rudder-server/enterprise/reporting/flusher 25.352s
  • Repository-wide literal searches for requestEntityTooLargeHandling, Reporting.requestEntityTooLargeHandling, Reporting.flusher.requestEntityTooLargeHandling, and Reporting.errorReporting.requestEntityTooLargeHandling — no matches found
  • git diff --check — passed

Linear Ticket

PIPE-3324

⚠️ Known Issues

Integration mini verification was skipped because this rudder-server source change is not source-verifiable in the local mini stack; focused Go tests were used as the runnable validation surface.

Broader go test -count 1 ./enterprise/reporting/... failed in the unrelated enterprise/reporting/error_index package. Observed failures included TestErrorIndexReporter/sync_data timing out its condition and multiple TestWorkerWriter cases failing because DuckDB could not download the parquet extension due HTTP 403 from http://extensions.duckdb.org/v1.5.4/linux_amd64/parquet.duckdb_extension.gz. The directly touched packages passed.

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@rudder-devbox

rudder-devbox Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

👋 I'm rudder-devbox. I opened this PR from a Linear ticket.

Mention @devbox or @rudder-devbox and I'll act on it — on a top-level PR comment,
in a review summary, or on an inline comment on the diff. Without a mention I won't see it.

@itsmihir
itsmihir marked this pull request as ready for review August 27, 2026 06:56
@itsmihir
itsmihir requested review from atzoum and ktgowtham August 27, 2026 06:57
@itsmihir itsmihir changed the title fix(core): make reporting 413 handling unconditional chore: make reporting 413 handling unconditional Aug 27, 2026
@rudder-devbox

rudder-devbox Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

I checked the failed CI logs and did not find a source failure caused by the reporting change.

Root cause from the logs:

  • Warehouse Integration (deltalake) fails before exercising code under test because STS returns InvalidClientTokenId: The security token included in the request is invalid during TestIntegration/Event_flow.
  • Warehouse Integration (redshift) and Warehouse Integration (snowflake) repeatedly fail staging uploads with S3 PutObject returning 403 InvalidAccessKeyId: The AWS Access Key Id you provided does not exist in our records.
  • Package Unit (warehouse) fails TestGlueSchemaRepositoryRoundTrip/s3_datalake_with_glue at glue_test.go:127 with Glue CreateDatabase returning UnrecognizedClientException: The security token included in the request is invalid.
  • Package Unit (integration_test/partitionmigration) fails an unrelated integration assertion in TestPartitionMigrationGatewayProcessorMode/legacy_no_jobsdb_fanout: Condition never satisfied, message all sent events should be received by the webhook, with the log stuck at total Sent: 11530, total received: 10530.
  • The aggregate job reports package-unit -> failure, consistent with the package-unit jobs above.

I synced the PR branch and confirmed it is already at the pushed head commit 57fdbd11af148ef5aa6735366ed105aa9db48ff6. The working tree is clean, and a repo-wide search still shows no remaining requestEntityTooLargeHandling references. I did not push a new commit because the actionable failures are invalid CI/cloud credentials plus an unrelated partition-migration integration timeout, not a code/test/lint/build failure in the changed reporting files.

@rudder-devbox

rudder-devbox Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

I found and fixed a package-unit test isolation issue.

Root cause from the failed log: warehouse/integrations/bigquery/middleware TestQueryWrapper/slow_query called the live BigQuery Jobs API during a package-unit run and failed at middleware_test.go:96 with googleapi: got HTTP response code 403; the response said the client did not have permission for /bigquery/v2/projects/big-query-integration-poc/jobs. Because that real call failed, the expected slow-query Infow("executing query", ...) call was also missing.

Fix: middleware.Client now keeps production defaults that call query.Run and query.Read, but those call sites are injectable. The middleware unit test now stubs Run and Read locally, so it still verifies slow-query logging without requiring live BigQuery credentials or network permissions.

Validation:

  • go test -count 1 ./warehouse/integrations/bigquery/middleware passed
  • go test -count 1 -race -vet=all ./warehouse/integrations/bigquery/middleware passed
  • git diff --check passed

Pushed commit: f4ca434

@rudder-devbox

rudder-devbox Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Knowledge distillation completed

@rudder-devbox

rudder-devbox Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

I fixed the verify lint failure.

Root cause from the lint log: golangci-lint v2.9.0 reported two nilnil violations in warehouse/integrations/bigquery/middleware/middleware_test.go at lines 62 and 65: both BigQuery test stubs returned nil, nil for pointer-plus-error results.

Fix: updated those stubs to return empty non-nil *bigquery.Job and *bigquery.RowIterator values with nil errors, preserving the test behavior while satisfying nilnil.

Validation:

  • go test -count 1 ./warehouse/integrations/bigquery/middleware passed
  • go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.9.0 run -v ./warehouse/integrations/bigquery/middleware passed with 0 issues
  • git diff --check passed

Pushed commit: 5243027

@rudder-devbox

rudder-devbox Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Knowledge distillation completed

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.79%. Comparing base (267b973) to head (5243027).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7313      +/-   ##
==========================================
+ Coverage   79.34%   79.79%   +0.45%     
==========================================
  Files         609      609              
  Lines       67963    67978      +15     
==========================================
+ Hits        53922    54243     +321     
+ Misses      10876    10544     -332     
- Partials     3165     3191      +26     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants