Skip to content

Conversation

@nikhilsinhaparseable
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable commented Dec 29, 2025

Fix OTEL endpoints to use PayloadConfig instead of JsonConfig so large OTEL payloads are handled correctly

replace const MAX_EVENT_PAYLOAD_SIZE with configurable value
add validation to keep this size > 100 bytes and less than 100 MBs

Summary by CodeRabbit

  • New Features

    • Maximum event payload size is now configurable via --max-event-payload-size CLI flag or P_MAX_EVENT_PAYLOAD_SIZE environment variable
    • Default limit remains 10 MB, with validation enforcing bounds between 100 bytes and 100 MB
  • Refactor

    • Payload size limit is now dynamically retrieved from configuration across all ingest endpoints instead of using a hardcoded value

✏️ Tip: You can customize this high-level summary in your review settings.

Fix OTEL endpoints to use PayloadConfig instead of JsonConfig
so large OTEL payloads are handled correctly

replace const MAX_EVENT_PAYLOAD_SIZE with configurable value
add validation to keep this size > 100 bytes and less than 100 MBs
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

Walkthrough

The changes convert a hardcoded maximum event payload size constant into a runtime-configurable option. A new CLI flag --max-event-payload-size with environment variable P_MAX_EVENT_PAYLOAD_SIZE is introduced. The constant is replaced with a dynamic function that retrieves values from runtime configuration, with validation ensuring sizes between 100 bytes and 100 MB.

Changes

Cohort / File(s) Summary
Configuration Option Addition
src/cli.rs, src/option.rs
Added new public field max_event_payload_size to Options struct with clap configuration (long flag, env var, default 10485760). Added validator function validate_payload_size() that enforces min 100 bytes and max 100 MB bounds with specific error messages.
HTTP Handler Constant Replacement
src/handlers/http/mod.rs
Replaced hardcoded constant MAX_EVENT_PAYLOAD_SIZE = 10485760 with public function max_event_payload_size() that returns the runtime configuration value.
HTTP Route Configuration Updates
src/handlers/http/modal/query_server.rs, src/handlers/http/modal/server.rs
Updated imports and replaced all usages of MAX_EVENT_PAYLOAD_SIZE constant with max_event_payload_size() function calls. Includes logstream, ingest, and OTEL endpoint route configurations (logs, metrics, traces).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested labels

for next release, docs needed

Poem

A rabbit hops through configs with glee, 🐰
Constants transform to functions so free,
Bounds check from bytes to one-hundred MB,
Command-line magic, dynamic and spree! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is missing the required template structure with sections like 'Fixes #XXXX', complete 'Description' section, and the testing/documentation checklist. Restructure the description to follow the template: add issue reference (Fixes #XXXX), expand description section with rationale, and include the testing/documentation checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: making payload size configurable and switching OTEL endpoints to PayloadConfig.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e67fc6 and a9b7e04.

📒 Files selected for processing (5)
  • src/cli.rs
  • src/handlers/http/mod.rs
  • src/handlers/http/modal/query_server.rs
  • src/handlers/http/modal/server.rs
  • src/option.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-18T18:01:22.834Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1405
File: src/handlers/http/modal/utils/ingest_utils.rs:271-292
Timestamp: 2025-08-18T18:01:22.834Z
Learning: In Parseable's ingestion validation, validate_stream_for_ingestion is designed to prevent regular log ingestion endpoints (ingest() and post_event()) from ingesting into streams that exclusively contain OTEL traces or metrics. The function allows mixed streams (regular logs + OTEL) but blocks ingestion into OTEL-only streams, maintaining architectural separation between regular log and OTEL ingestion pathways.

Applied to files:

  • src/handlers/http/modal/server.rs
📚 Learning: 2025-08-25T01:31:41.786Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/metadata.rs:63-68
Timestamp: 2025-08-25T01:31:41.786Z
Learning: The TOTAL_EVENTS_INGESTED_DATE, TOTAL_EVENTS_INGESTED_SIZE_DATE, and TOTAL_EVENTS_STORAGE_SIZE_DATE metrics in src/metadata.rs and src/storage/object_storage.rs are designed to track total events across all streams, not per-stream. They use labels [origin, parsed_date] to aggregate by format and date, while per-stream metrics use [stream_name, origin, parsed_date] labels.

Applied to files:

  • src/handlers/http/modal/server.rs
🧬 Code graph analysis (4)
src/cli.rs (1)
src/option.rs (1)
  • validate_payload_size (213-234)
src/handlers/http/modal/query_server.rs (1)
src/handlers/http/mod.rs (1)
  • max_event_payload_size (140-142)
src/option.rs (4)
src/event/format/json.rs (1)
  • s (316-316)
src/handlers/http/alerts.rs (4)
  • s (161-161)
  • s (167-167)
  • s (173-173)
  • s (179-179)
src/validator.rs (1)
  • size (138-138)
src/cli.rs (1)
  • format (653-654)
src/handlers/http/modal/server.rs (1)
src/handlers/http/mod.rs (1)
  • max_event_payload_size (140-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Quest Smoke and Load Tests for Distributed deployments
  • GitHub Check: Quest Smoke and Load Tests for Standalone deployments
  • GitHub Check: Build Default aarch64-unknown-linux-gnu
  • GitHub Check: coverage
  • GitHub Check: Build Default x86_64-unknown-linux-gnu
  • GitHub Check: Build Kafka x86_64-unknown-linux-gnu
  • GitHub Check: Build Default x86_64-pc-windows-msvc
  • GitHub Check: Build Default aarch64-apple-darwin
  • GitHub Check: Build Kafka aarch64-apple-darwin
🔇 Additional comments (9)
src/handlers/http/mod.rs (1)

140-142: LGTM! Clean conversion from constant to dynamic accessor.

The function correctly retrieves the configured payload size from runtime options, enabling the configurable behavior described in the PR objectives.

src/option.rs (1)

213-234: LGTM! Validation properly enforces payload size constraints.

The function correctly validates the payload size between 100 bytes and 100 MB with clear error messages. The logic is consistent with other validators in the codebase.

src/cli.rs (1)

436-443: LGTM! CLI configuration properly exposes the payload size setting.

The field is correctly configured with appropriate default value (10 MB), validation, and clear help text.

src/handlers/http/modal/query_server.rs (2)

24-25: LGTM! Imports correctly updated.

The imports now reference the new function-based accessor instead of the removed constant.


291-291: LGTM! Payload limit now uses dynamic configuration.

The JsonConfig correctly uses max_event_payload_size() to retrieve the configurable payload limit for the logstream endpoint.

src/handlers/http/modal/server.rs (4)

28-28: LGTM! Import correctly added.

The import enables usage of the new dynamic payload size accessor throughout this module.


466-466: LGTM! Regular ingest endpoints correctly use JsonConfig.

Both the logstream POST route and /ingest endpoint appropriately use JsonConfig with the dynamic payload limit, as they expect JSON payloads.

Also applies to: 537-537


550-550: LGTM! Critical fix for OTEL endpoints.

The switch from JsonConfig to PayloadConfig for OTEL endpoints (/v1/logs, /v1/metrics, /v1/traces) is correct and addresses the PR objective. This change is important because:

  • OTEL endpoints may use binary formats (protobuf) rather than JSON
  • PayloadConfig limits payload size without imposing JSON parsing
  • This enables proper handling of large OTEL payloads

Also applies to: 559-559, 568-568


28-28: Migration to max_event_payload_size() function is complete. No remaining references to the MAX_EVENT_PAYLOAD_SIZE constant were found in the codebase.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nitisht nitisht requested a review from parmesant December 29, 2025 08:36
@nitisht nitisht merged commit 1f2d3b4 into parseablehq:main Dec 29, 2025
11 of 12 checks passed
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