fix(sls-flusher): propagate send failures and classify by actionability

[Snssn]

Summary

SLS send-failure handling had three concrete defects, verified against 7 days of production alarm data. This PR fixes them without changing alarm_type (downstream filters/dashboards stay valid).

• Send failures were invisible in metrics. All three send paths (ak/apiKey/webtracking) recorded an alarm, persisted, then returned normally — so flush() counted failed batches as succeeded and out_failed_entries_total stayed 0. They now rethrow, and flush() splits succeeded vs failed counts (partial webtracking chunks included).
• Alarms were unclassified and noisy. Failures are now classified into transient / quota / config / payload, driving alarm_level and recovery. A new optional failure_class field is added to the alarm record; alarm_type remains FLUSH_SEND_ALARM.
  • transient (timeout / fetch failed) no longer alarms per-occurrence — that volume already lives in the out_failed metric. Only a sustained per-endpoint outage (N consecutive all-batch-failed cycles) escalates to a single cooldown-gated alarm.
  • config (project not-exist / forbidden / in-recycle-bin) is cooldown-gated and trips a per-endpoint circuit breaker with exponential backoff (≤10 min) and a half-open probe that auto-recovers once the config is fixed.
  • payload (413 / a single entry over the body cap) — the oversize entry's largest field is truncated to fit, or the entry is dropped and counted, instead of being sent as a request guaranteed to be rejected.

Also fixes a long-standing dead branch: the old 429 quota check used instanceof HttpError, which never matched on the ak SDK path. Classification now inspects both HttpError.status and the SDK error code/message.

Test plan

• tests/unit/flushers/sls-transport.test.ts — classifyFailure across ak/webtracking error shapes + level mapping
• tests/unit/flushers/sls-flusher.failure-handling.test.ts (new) — outFailed vs outEntries, partial-chunk split, transient escalation/reset, config cooldown, circuit breaker trip/skip/recover/isolation, payload truncate/drop
• tests/unit/metrics/alarm-manager.test.ts — failure_class field + aggregation-key separation + backward-compat omission
• tests/unit/flushers/sls-flusher.dual-write.test.ts — no regression
• tsc --noEmit clean

[linrunqi08]

🔍 Code Review Summary

Severity	Count
Critical	0
High	0
Medium	4
Low	9

Lifecycle Verdict

Check	Result
资源释放	PASS
死锁/卡死风险	PASS
状态恢复正确性	PASS
PowerShell CLM 合规	N/A

All lifecycle checks pass — flushing flag in try/finally guarantees reset, .catch() never rethrows so Promise.all always resolves, circuit breaker state is correctly memory-only.

Merge Gate（合入门禁）

⛔ BLOCK — 4 Medium issues require resolution before merge:

ID	Severity	File	Issue
F1	Medium	sls-transport.ts:84	Bare HTTP 404/403 classified as config — proxy/CDN 404 would falsely trip circuit breaker
F2	Medium	sls-flusher.ts:248	Circuit breaker operates silently — no log for trip, skip, or recovery
F3	Medium	sls-flusher.ts:594	Redundant JSON.stringify per entry in splitForWebtracking — byteSize already pre-computed
F4	Medium	docs/sls-output.md	English docs missing new failure classification section (zh-CN has it)

总体结论

PR is well-architected — the centralized failure handling pipeline, failure classification, circuit breaker, and oversize guard are all solid. The partial-failure accounting via FlushFailure and the dead-branch fix for 429 detection show thorough analysis. The four Medium items are: (1) a proxy-404 false-positive risk with real production impact, (2) an observability gap that would complicate incident triage, (3) a hot-path performance regression, and (4) a documentation parity gap. All are straightforward to fix.

Additionally, 9 Low-severity items were identified (test coverage gaps, code reuse opportunities, defensive coding improvements) — see the full report for details.

Highlights（正向实践）

• FlushFailure elegantly carries both succeeded/failed counts for accurate partial-failure accounting
• Defense-in-depth truncate-then-drop for oversize entries prevents guaranteed-to-fail 413 requests
• Comprehensive new test suite with clean fake-timer patterns for circuit breaker verification
• Backward-compatible alarm schema extension with optional failure_class field
• Dead-branch fix for instanceof HttpError that never matched on the ak SDK path

---

评审报告详见: code-review/pr-256/final-report.md
Generated by LoongSuite-Pilot Code Review Agent

[linrunqi08]

[Medium — F1] Bare HTTP 404/403 status codes are classified as config regardless of response body, meaning a proxy/CDN/WAF 404 (not from SLS) will falsely trip the circuit breaker with up to 10-minute exponential backoff.

影响: A transient proxy routing issue returning 404 trips the config circuit breaker, turning a brief blip into prolonged data loss.

建议: Narrow the bare-status check so 404/403 only classify as config when the body also contains a known SLS error code:

if (CONFIG_ERROR_CODES.some(c => msg.includes(c)) ||
    ((status === 404 || status === 403) && /errorCode|ErrorCode/.test(msg))) {
  return 'config';
}

Generated by LoongSuite-Pilot Code Review Agent

[linrunqi08]

[Medium — F2] Circuit breaker operates silently — when open, entries are dropped (counter incremented) with no log. tripCircuit() and onEndpointSuccess() also emit no log for state transitions.

影响: Operators see outFailed climbing but no log explains why. Every other failure path has diagnostic logging; this is the exception. During incidents, the circuit breaker's 1-hour cooldown + silent drops make timeline reconstruction very difficult.

建议: Add logging at three points:

// In circuit-open skip:
logger.debug('circuit open, skipping send', { endpoint: endpoint.name, dropped: logs.length });

// In tripCircuit when threshold reached:
logger.warn('circuit breaker tripped', { endpoint: name, backoffMs: c.backoffMs });

// In onEndpointSuccess when circuit was open:
if (this.circuits.has(name)) {
  logger.info('circuit breaker recovered', { endpoint: name });
}

Generated by LoongSuite-Pilot Code Review Agent

[linrunqi08]

[Medium — F3] splitForWebtracking calls Buffer.byteLength(JSON.stringify(log.content)) for every entry despite log.byteSize already holding the pre-computed value from enqueue().

影响: Each log entry is serialized to JSON twice per flush cycle. For typical batches of hundreds of entries (5–10 KB avg), this doubles the hot-path serialization cost for webtracking endpoints.

建议: Use log.byteSize for the initial size check, only re-serializing after truncation:

let logSize = raw.byteSize; // pre-computed in enqueue()
if (logSize > maxBytes) {
  // ...truncation logic, re-compute only after mutation...
}

Generated by LoongSuite-Pilot Code Review Agent

[linrunqi08]

[Medium — F4] docs/zh-CN/sls-output.md adds a new "发送失败的分类、冷却与熔断" section (lines 156–168) but the English counterpart docs/sls-output.md has no corresponding section. Both files link to each other claiming bilingual parity.

影响: Non-Chinese-reading operators won't discover the classification table, cooldown semantics, or circuit-breaker behavior from the documentation.

建议: Add a corresponding "Send Failure Classification, Cooldown & Circuit Breaker" section to docs/sls-output.md.

Generated by LoongSuite-Pilot Code Review Agent

[ralf0131]

Summary

Adds a comprehensive failure classification and resilience layer to the SLS flusher. Well-structured improvement that transforms silent failures into actionable, categorized alarms with proper recovery mechanisms.

Key Changes:

1. Failure Classification (classifyFailure in sls-transport.ts): Clean taxonomy — transient / quota / config / payload — each mapped to appropriate alarm levels. Handles both webtracking (HttpError with .status) and AK SDK (error objects with .code/.errorCode) patterns correctly. ✅

2. Circuit Breaker (CircuitState): Per-endpoint breaker trips after 3 consecutive config failures, with exponential backoff (2s → 10min cap). Half-open probes auto-recover on success. Properly isolated — one bad endpoint doesn't affect others. ✅

3. Transient Escalation: Only alarms after 3 consecutive all-batch-failed cycles, preventing noise from intermittent failures. Partial success resets the streak — correct behavior. ✅

4. Cooldown Gating: Config + escalated-transient alarms are rate-limited to 1 per hour per endpoint per class. Prevents alarm fatigue during sustained outages. ✅

5. Oversize Payload Guard: splitForWebtracking now truncates single oversized fields (UTF-8 safe) or drops entries that can't fit. FlushFailure carries succeeded/failed split for accurate counter updates. ✅

6. AlarmManager Enhancement: failure_class context field keeps distinct failure types separate in alarm tracking while remaining backward compatible. ✅

Code Quality:

• Constants are well-documented with clear rationale
• Error propagation is correct (send methods now throw instead of silently recording)
• Test coverage is comprehensive (310 lines covering all scenarios including circuit breaker isolation, transient streak reset, and oversize truncation)
• truncateUtf8Bytes properly strips trailing replacement characters

---

Automated review by github-manager-bot

[Snssn]

Thanks for the review. Addressed all four Medium items in 697f897:

• F1 (proxy-404 false-trip): classifyFailure now only maps 404/403 to config when the body carries an SLS error code (errorCode), or when a known code (ProjectNotExist/ProjectForbidden/ProjectInRecycleBin/LogStoreNotExist) is present. A bare proxy/CDN/WAF 404/403 falls through to transient, so it can't trip the breaker. Added a test asserting bare 404/403 → transient.
• F2 (silent breaker): added logger.warn on trip (with configFails/backoffMs), logger.debug on open-skip (with dropped count), and logger.info on recovery.
• F3 (double serialize): splitForWebtracking now reuses the byteSize precomputed in enqueue(); it only re-serializes after a truncation actually mutates the entry.
• F4 (EN docs parity): added the "Send Failure Classification, Cooldown & Circuit Breaker" section to docs/sls-output.md mirroring zh-CN.

The 9 Low items (test-coverage/reuse/defensive nits) are noted; happy to fold in any you consider blocking. tsc clean, 41 unit tests pass.

[ralf0131]

Summary

Solid fix addressing three concrete defects in SLS send-failure handling. The changes are well-scoped, production-validated against 7 days of alarm data, and properly propagate failures without breaking downstream metric contracts.

Highlights

• FlushFailure exception: Clean propagation of partial batch results — succeeded/failed entries are now credited accurately instead of charging the whole batch to one side
• Circuit breaker pattern: Proper exponential backoff for terminal config errors, preventing pointless retries
• Transient escalation threshold: Smart design — network failures don't alarm per-occurrence, only after consecutive all-batch-failed cycles
• Cooldown-gated alarms: 1h cooldown prevents alert storms from persistent faults
• Truncation handling: Proper truncation marker for oversized fields with byteSize recalculation

Code Quality

• Clean separation between classifyFailure (transport layer) and FlushFailure (flusher layer)
• Per-endpoint state tracking is well-isolated
• Test coverage is comprehensive — covers all three send paths (ak/apiKey/webtracking) and failure classes

LGTM — production-validated fix with proper error propagation and alarm management.

---

Automated review by github-manager-bot