Surfaced by the independent audit of #528 (finding M1).
Problem
The sink framework's byte-ceiling is advertised but enforced nowhere:
SinkCapabilities.max_batch_bytes is set by sinks but read by no code. SinkPipeline batches purely by record count (max_batch, default 100) + flush interval.- The docs contradict each other on who enforces it:
capabilities.rs (max_batch_bytes): "The pipeline never hands the sink a batch larger than this."pipeline.rs module doc: "the sink owns chunking a batch down to its own per-request byte limit inside append_batch."- Reality: neither happens.
AliyunSlsSink::append_batch encodes all records into a single LogGroup → one PutLogs. SLS caps the request body (single LogGroup, low single-digit MB). On today's metadata-only path this is fine (≈100 records × ~0.5–2 KB ≪ limit). But the next milestone — full prompt/response content capture (SinkContent) — makes a 100-record batch easily exceed the limit, and SLS then returns 400 PostBodyInvalid, which classifies as SinkError::Permanent → the whole batch is dropped, never retried. A log sink silently dropping under load is exactly the failure the sink's error-classification is meant to avoid.
Fix (do this before content capture ships)
- Implement byte-aware chunking in
AliyunSlsSink::append_batch (per the pipeline.rs design — the sink self-chunks, since only the sink knows the encoded size): pack records into multiple PutLogs requests, each kept under a conservative raw-size ceiling (~3 MB), always sending at least one record per request. Fast-path the common case (whole batch fits → one encode + one POST); only split when oversize.
- Reconcile the contradictory capability docs (
capabilities.rs vs pipeline.rs) to a single source of truth: the sink self-limits.
- Add a unit test that drives a synthetic >ceiling batch (e.g. one large
SinkContent) and asserts it fans out across multiple signed PutLogs requests.
- Restore the SLS capability to
batch_unit: Both + max_batch_bytes: Some(...) once (1) lands.
Interim (in #528)
The SLS capability was downgraded to the honest current state — batch_unit: Records, max_batch_bytes: None (parity with OtlpSink) — so it no longer advertises an unenforced ceiling. This issue blocks the content-capture milestone.
Refs: #528, api7/AISIX-Cloud#687
Surfaced by the independent audit of #528 (finding M1).
Problem
The sink framework's byte-ceiling is advertised but enforced nowhere:
SinkCapabilities.max_batch_bytesis set by sinks but read by no code.SinkPipelinebatches purely by record count (max_batch, default 100) + flush interval.capabilities.rs(max_batch_bytes): "The pipeline never hands the sink a batch larger than this."pipeline.rsmodule doc: "the sink owns chunking a batch down to its own per-request byte limit insideappend_batch."AliyunSlsSink::append_batchencodes all records into a singleLogGroup→ one PutLogs. SLS caps the request body (single LogGroup, low single-digit MB). On today's metadata-only path this is fine (≈100 records × ~0.5–2 KB ≪ limit). But the next milestone — full prompt/response content capture (SinkContent) — makes a 100-record batch easily exceed the limit, and SLS then returns400 PostBodyInvalid, which classifies asSinkError::Permanent→ the whole batch is dropped, never retried. A log sink silently dropping under load is exactly the failure the sink's error-classification is meant to avoid.Fix (do this before content capture ships)
AliyunSlsSink::append_batch(per thepipeline.rsdesign — the sink self-chunks, since only the sink knows the encoded size): pack records into multiple PutLogs requests, each kept under a conservative raw-size ceiling (~3 MB), always sending at least one record per request. Fast-path the common case (whole batch fits → one encode + one POST); only split when oversize.capabilities.rsvspipeline.rs) to a single source of truth: the sink self-limits.SinkContent) and asserts it fans out across multiple signed PutLogs requests.batch_unit: Both+max_batch_bytes: Some(...)once (1) lands.Interim (in #528)
The SLS capability was downgraded to the honest current state —
batch_unit: Records,max_batch_bytes: None(parity withOtlpSink) — so it no longer advertises an unenforced ceiling. This issue blocks the content-capture milestone.Refs: #528, api7/AISIX-Cloud#687