fix(telemetry): reject non-positive maxExportBatchSize at plugin load

Review finding, pinned red-first: the SDK accepts processor.maxExportBatchSize <= 0 (or fractional), but its shutdown drain then splices empty batches without consuming the queue — disposing telemetry hangs forever whenever records are queued. The constructor now rejects a non-positive-integer batch size before building the SDK processor, per the misconfiguration-fails-loud rule; everything else in the processor block remains the SDK's verbatim passthrough.
2026-07-27 20:11:30 +08:00
parent a72436105b
commit bf76c52d76
5 changed files with 16 additions and 4 deletions
@@ -2,5 +2,5 @@
 # side as of the last confirmed-consistent state. Both languages carry equal authority;
 # after editing either side, bring the other along and re-record with:
 #   pnpm run verify-translation-pairing --write
-README.md: 6ead0ec7b7053551676b482dd4c905c8cd7cc1fe
-README.zh.md: 852242bfc4ac719660f31935167d2f4fa0f58f2e
+README.md: e345449033b066103fb14e546e8d9909d68fe696
+README.zh.md: 6cad58ebc1ad9bab6dc722fcf0d702f500891d32
@@ -17,7 +17,7 @@ The OpenTelemetry backend for [the telemetry seam](../session-telemetry/) — th
    processor: {}            # optional; passed verbatim to BatchLogRecordProcessor
 ```

-`exporter.url` is the one field this package validates itself — required, no default, must parse as `http(s)` — so a missing endpoint fails at plugin load. Everything else is the SDK's option shape, owned and documented by the SDK, and both blocks pass through whole: every `OTLPExporterNodeConfigBase` field (`headers`, `timeoutMillis`, `compression`, `keepAlive`, …) reaches the exporter, and batching, export cadence (`scheduledDelayMillis`), retry, queue bounds, and loss policy under sustained failure are the SDK's documented behavior, tuned through the `processor` passthrough. The backend deliberately implements no `flush()`: the batch processor is the only flusher in the process, which is what makes `shutdown()`'s drain complete. Removing this block from `cordis.yml` is the opt-out: no residual state, no `enabled` flag.
+`exporter.url` is the one field this package validates itself — required, no default, must parse as `http(s)` — so a missing endpoint fails at plugin load (as does a non-positive-integer `processor.maxExportBatchSize`, which the SDK accepts but then hangs on at shutdown). Everything else is the SDK's option shape, owned and documented by the SDK, and both blocks pass through whole: every `OTLPExporterNodeConfigBase` field (`headers`, `timeoutMillis`, `compression`, `keepAlive`, …) reaches the exporter, and batching, export cadence (`scheduledDelayMillis`), retry, queue bounds, and loss policy under sustained failure are the SDK's documented behavior, tuned through the `processor` passthrough. The backend deliberately implements no `flush()`: the batch processor is the only flusher in the process, which is what makes `shutdown()`'s drain complete. Removing this block from `cordis.yml` is the opt-out: no residual state, no `enabled` flag.

 ## What leaves the machine

@@ -17,7 +17,7 @@
    processor: {}            # optional; passed verbatim to BatchLogRecordProcessor
 ```

-`exporter.url` 是本包唯一自行校验的字段：必填、无默认值、必须能解析为 `http(s)`，因此缺失端点会在插件加载时失败。其余全部是 SDK 自己的选项形态，由 SDK 拥有并在 SDK 文档中说明，两个配置块都整体透传（passthrough）：`OTLPExporterNodeConfigBase` 的每个字段（`headers`、`timeoutMillis`、`compression`、`keepAlive` 等）都会到达导出器；批处理、导出节奏（`scheduledDelayMillis`）、重试、队列上限，以及持续失败下的丢失策略，都是 SDK 的文档化行为，经 `processor` 透传调优。该后端刻意不实现 `flush()`：批处理器是进程内唯一执行 flush 的组件，`shutdown()` 的排空正因如此才是完整的。从 `cordis.yml` 中删除该配置块即为退出方式：无残留状态，也没有 `enabled` 开关。
+`exporter.url` 是本包唯一自行校验的字段：必填、无默认值、必须能解析为 `http(s)`，因此缺失端点会在插件加载时失败（`processor.maxExportBatchSize` 不是正整数时同样如此：SDK 会接受该值，随后却在关闭时因它挂起）。其余全部是 SDK 自己的选项形态，由 SDK 拥有并在 SDK 文档中说明，两个配置块都整体透传（passthrough）：`OTLPExporterNodeConfigBase` 的每个字段（`headers`、`timeoutMillis`、`compression`、`keepAlive` 等）都会到达导出器；批处理、导出节奏（`scheduledDelayMillis`）、重试、队列上限，以及持续失败下的丢失策略，都是 SDK 的文档化行为，经 `processor` 透传调优。该后端刻意不实现 `flush()`：批处理器是进程内唯一执行 flush 的组件，`shutdown()` 的排空正因如此才是完整的。从 `cordis.yml` 中删除该配置块即为退出方式：无残留状态，也没有 `enabled` 开关。

 ## 哪些数据会离开本机

@@ -106,6 +106,14 @@ export class TelemetryOtel extends Telemetry {
    if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
      throw new Error(`session-telemetry-otel: exporter.url must be http(s), got ${parsed.protocol}`)
    }
+    // The one processor field checked beyond the SDK's own validation: the
+    // SDK accepts a non-positive batch size, but its shutdown drain then
+    // splices empty batches without consuming the queue — dispose would hang
+    // forever with records queued. Misconfiguration fails at load instead.
+    const batchSize = config.processor?.maxExportBatchSize
+    if (batchSize !== undefined && (!Number.isInteger(batchSize) || batchSize < 1)) {
+      throw new Error(`session-telemetry-otel: processor.maxExportBatchSize must be a positive integer, got ${String(batchSize)}`)
+    }
    this.provider = new LoggerProvider({
      resource: resourceFromAttributes({
        'service.name': APP_IDENTITY.product,
@@ -202,6 +202,10 @@ describe('TelemetryOtel config fails loud', () => {
    [{ exporter: { url: '' } }, /exporter\.url is required/],
    [{ exporter: { url: 'not a url' } }, /not a valid URL/],
    [{ exporter: { url: 'ftp://collector' } }, /must be http\(s\)/],
+    // The SDK accepts a non-positive batch size but its shutdown drain then
+    // splices empty batches forever — dispose would hang, so reject at load.
+    [{ exporter: { url: 'http://c/v1/logs' }, processor: { maxExportBatchSize: 0 } }, /maxExportBatchSize/],
+    [{ exporter: { url: 'http://c/v1/logs' }, processor: { maxExportBatchSize: 0.5 } }, /maxExportBatchSize/],
  ])('rejects %j at plugin load', async (config, message) => {
    const ctx = new Context()
    await ctx.plugin(SessionStore)