obs sink: byte-aware chunking for SLS PutLogs (blocks content capture)
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 157
- Forks
- 32
- Avg merge
- 1h 25m
- Merged PRs (30d)
- 145
Description
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.- 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.rsmodule doc: "the sink owns chunking a batch down to its own per-request byte limit insideappend_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 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. - Reconcile the contradictory capability docs (
capabilities.rsvspipeline.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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with AliyunSlsSink::append_batch and the SinkPipeline, then read capabilities.rs and the pipeline.rs module documentation to resolve the chunking contract. Add a synthetic oversized SinkContent unit test that verifies multiple signed PutLogs requests, and confirm the SLS capability advertises batch_unit: Both with a byte ceiling once self-chunking is implemented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100