From 1f22a113065d88b90c42ff72a082d10553e15b35 Mon Sep 17 00:00:00 2001 From: htjulia Date: Mon, 21 Sep 2026 15:58:53 +0900 Subject: [PATCH 1/2] fix(trace): say what the metrics flag actually means, everywhere Two claims about disabled metrics were false, and the console found both by connecting a real client to a real server. Neither was reachable by the test suite as it stood. **"Empty when disabled" was never true.** `TraceMetrics.snapshots()` iterates `byModule` with no reference to the flag; the flag gates `observe`, so turning it off stops new observations and keeps everything already aggregated. A reverse grep for that claim found it in four places, not one: the method's own javadoc, the REST controller's, and -- worse -- the published guide in both languages, where a consumer reads it. An empty result means "nothing was ever aggregated", never "aggregation is off", and all four now say so. **The `ready` ack disagreed with the MCP tool under the same field name.** `TraceMetrics.enabled()` is this platform's answer to "are metrics on" and it already folds in `trace.enabled`, because recording is gated before aggregation is reached. `ModuleMetricsTool` reports that value. The ack I wrote reached past it for the raw switch, so the same question had two answers depending on which surface asked it. The ack now reports the effective value, and `metricsEnabled` is defined in the contract as the conjunction rather than described by its value -- an implementation sending the raw switch is not conforming, which a wording that only said "reports whether metrics are on" would have left open. That fix removes one wrong answer and creates another, so the docs say that too: `{tracesEnabled: false, metricsEnabled: true}` can no longer be reported, but on `{false, false}` a client reading only `metricsEnabled` will offer to turn metrics on, and turning them on changes nothing while recording is off. The field answers "are metrics accruing", never "would enabling them help". The console raised this after the first correction and it is the sharper statement of the limit. Three tests, each failing on the specific regression rather than on shape: retention across a disable, no accrual while recording is off although the switch is on, and the ack carrying the effective value. The last one runs in the configuration where the two readings diverge, so a change back to the raw switch fails instead of shipping -- which matters because the test suite runs from exploded classes and cannot otherwise observe this frame's honesty. Verified: 599 tests, 0 failures, 0 skipped; PoC gate passes 6 combinations x 6 criteria with no SKIPPED and no unreported cell. Co-Authored-By: htjulia --- docs/guide/04-rest-api.ko.md | 11 +- docs/guide/04-rest-api.md | 11 +- .../htcom/protean/runtime/TraceMetrics.java | 8 +- .../org/htcom/protean/web/StreamReady.java | 8 +- .../protean/web/TraceAdminController.java | 6 +- .../htcom/protean/web/TraceStreamService.java | 8 +- .../TraceDisabledGatesMetricsTest.java | 133 ++++++++++++++++++ .../org/htcom/protean/TraceMetricsTest.java | 20 +++ 8 files changed, 194 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/htcom/protean/TraceDisabledGatesMetricsTest.java diff --git a/docs/guide/04-rest-api.ko.md b/docs/guide/04-rest-api.ko.md index 2b81361..bdb4c5b 100644 --- a/docs/guide/04-rest-api.ko.md +++ b/docs/guide/04-rest-api.ko.md @@ -367,7 +367,9 @@ POST /platform/modules/cp-mod/approve?approver=alice ### GET `/platform/traces/metrics` -모듈별 집계 메트릭(요청 수·오류율·지연 백분위)을 조회한다. **opt-in** — `protean.trace.metrics.enabled=true` 일 때만 집계되며, 꺼져 있으면 빈 목록을 반환한다. +모듈별 집계 메트릭(요청 수·오류율·지연 백분위)을 조회한다. **opt-in** — `protean.trace.metrics.enabled=true` 일 때만 집계된다. + +그 플래그를 끈다고 이 엔드포인트가 비지는 **않는다**. 플래그가 막는 것은 집계이지 노출이 아니므로, 끄기 전에 관찰된 행은 그대로 계속 나간다. 따라서 빈 목록은 *"집계된 적이 없다"* 는 뜻이지 *"집계가 꺼져 있다"* 가 아니다 — 둘을 구별하려면 SSE `ready` 프레임의 `metricsEnabled` 를 읽는다. - 쿼리 파라미터: - `moduleId`(선택) — 지정 시 그 모듈만(추적 안 된 모듈이면 빈 목록), 생략 시 추적 중인 전 모듈. @@ -399,7 +401,7 @@ POST /platform/modules/cp-mod/approve?approver=alice - named 이벤트: - `ready` — 연결 확인(ack). 언제나 첫 프레임이다(아래 참고) - `trace` — 새 `RequestTrace` 델타 - - `metrics` — `ModuleMetricsSnapshot[]` 갱신(모듈별 누적 집계, `protean.trace.metrics.enabled=true` 일 때만 채워짐) + - `metrics` — `ModuleMetricsSnapshot[]` 갱신(모듈별 누적 집계. 새 관찰은 `protean.trace.metrics.enabled=true` 일 때만 쌓이지만, 끄기 전에 집계된 행은 계속 나간다) - `modules` — 현재 `ModuleStatus[]` - `summary` — `TraceSummary`: 콘솔 헤더용 **윈도** 집계(아래 참고) - `/platform/traces` 와 마찬가지로 스트림 연결 자체는 trace 기록에서 제외된다(자기-관측 방지). @@ -416,7 +418,10 @@ data: {"platform":"protean","platformVersion":"0.1.0","tracesEnabled":true, - `platform` — 구현체 식별자. 언제나 `"protean"`. 이 계약을 여러 구현이 공유하므로, 버전 숫자만 받은 콘솔이 그것이 누구의 버전인지 추측하지 않도록 함께 싣는다. **표시·진단용이며 동작을 분기하는 데 쓰지 않는다.** - `platformVersion` — 이 라이브러리의 버전. jar manifest 의 `Implementation-Version` 에서 읽는다. manifest 가 없는 레이아웃(exploded classes — 이 라이브러리 자신의 테스트, 그리고 클래스 디렉터리로 잡힌 IDE 실행)에서는 **`null`** 이다. 그 상황에서는 버전을 정말로 모르며, 임의의 대체 문자열은 "지금 어느 버전과 이야기하고 있나" 에 대한 지어낸 답이 된다. 실제로는 보기 드물다 — 소비자 앱은 protean 을 jar 로 해석하고 `examples/quickstart` 데모도 그러므로, **콘솔이 실제로 붙는 경로는 전부 진짜 버전을 내려보낸다.** 필드 자체는 항상 있으므로 클라이언트가 "없음" 과 "모름" 을 구별할 일이 없다. -- `tracesEnabled` / `metricsEnabled` — `protean.trace.enabled` 와 `protean.trace.metrics.enabled`. **"기록이 꺼져 있다"** 와 **"켜져 있는데 아직 아무 일도 없었다"** 를 가른다. 빈 스트림만으로는 이 둘이 구별되지 않는다. +- `tracesEnabled` — `protean.trace.enabled`. **"기록이 꺼져 있다"** 와 **"켜져 있는데 아직 아무 일도 없었다"** 를 가른다 — 빈 스트림만으로는 구별되지 않는다. +- `metricsEnabled` — **`protean.trace.enabled` AND `protean.trace.metrics.enabled` 로 정의된다.** metrics 스위치 단독 값이 아니다. 기록은 집계보다 **앞서** 막히므로 `tracesEnabled` 가 false 인 동안 그 스위치는 아무 의미가 없다. MCP 메트릭 툴이 같은 이름으로 답하는 값과 같으며, 이 계약을 구현하면서 원시 스위치를 싣는 것은 계약 위반이다. + + ⚠️ **그래도 `tracesEnabled` 를 먼저 읽어야 한다.** 논리곱은 틀린 답 하나를 없애지만(`{"tracesEnabled": false, "metricsEnabled": true}` 는 이 프레임이 내보낼 수 없는 상태다) 다른 하나를 만든다. `{"tracesEnabled": false, "metricsEnabled": false}` 에서 `metricsEnabled` 만 보는 클라이언트는 *"메트릭이 꺼져 있다 — 켜면 행이 생긴다"* 고 말하는데, 기록이 꺼져 있는 한 켜도 아무 일도 일어나지 않는다. `metricsEnabled` 는 "메트릭이 쌓이고 있는가" 에 답할 뿐 **"켜면 도움이 되는가" 에는 답하지 않는다.** - `buffered` — 이 프레임 **다음에 오는 `trace` 프레임이 싣는 행 수**이며, 링이 보유한 행 수가 아니다. `protean.trace.capacity` 가 재생 상한 200 을 넘으면 둘이 달라진다. 숫자만 알리고 행은 주지 않는 것은 둘 다 안 하느니만 못하다. - `tickMs` — 푸시 주기(`1000`). 클라이언트의 침묵 감지 워치독이 가정 대신 계약을 기준으로 자기 값을 정할 수 있게 한다. - `capacity` — **이 연결 시점의** `protean.trace.capacity`. 클라이언트가 "이보다 과거는 서버에 없다" 를 말할 수 있다. LIVE 키이므로 이후 변경은 이미 연결된 클라이언트에 닿지 않으며, 재연결 때 새 값이 다시 나간다. diff --git a/docs/guide/04-rest-api.md b/docs/guide/04-rest-api.md index 24e80d7..151bdd8 100644 --- a/docs/guide/04-rest-api.md +++ b/docs/guide/04-rest-api.md @@ -367,7 +367,9 @@ Query recent request-execution traces, newest-first. If `protean.trace.enabled=f ### GET `/platform/traces/metrics` -Query per-module aggregate metrics (request count, error rate, latency percentiles). **opt-in** — aggregation happens only when `protean.trace.metrics.enabled=true`; when off, it returns an empty list. +Query per-module aggregate metrics (request count, error rate, latency percentiles). **opt-in** — aggregation happens only when `protean.trace.metrics.enabled=true`. + +Turning that flag off does **not** empty this endpoint: it gates aggregation, not exposure, so rows observed before it was turned off keep being returned. An empty list therefore means *"nothing was ever aggregated"*, never *"aggregation is off"* — to tell those apart, read `metricsEnabled` from the SSE `ready` frame. - Query parameters: - `moduleId` (optional) — when set, only that module (empty list if the module is not tracked); when omitted, all tracked modules. @@ -399,7 +401,7 @@ Live server-sent-events (SSE) push stream — the console uses it in place of 5- - Named events: - `ready` — connection acknowledgement, always the first frame (see below) - `trace` — new `RequestTrace` deltas - - `metrics` — a `ModuleMetricsSnapshot[]` update (per-module cumulative aggregate; populated only when `protean.trace.metrics.enabled=true`) + - `metrics` — a `ModuleMetricsSnapshot[]` update (per-module cumulative aggregate; new observations accrue only when `protean.trace.metrics.enabled=true`, but rows aggregated before it was turned off keep being sent) - `modules` — the current `ModuleStatus[]` - `summary` — a `TraceSummary`: a **windowed** aggregate for the console header (see below) - Like `/platform/traces`, the stream connection itself is excluded from trace recording (no self-observation). @@ -416,7 +418,10 @@ It carries only what a client must know **once, at connect time** and cannot der - `platform` — implementation identifier, always `"protean"`. Present so a console watching several implementations of this contract is not left reading a bare version number and guessing whose it is. **Display and diagnostics only — do not branch behaviour on it.** - `platformVersion` — this library's version, read from the jar manifest (`Implementation-Version`). **`null`** when running from a layout that has no manifest (exploded classes: the library's own test suite, and IDE runs configured against class directories) — the version is genuinely unknown there, and a placeholder would be a fabricated answer. In practice it is rarely seen: a consumer application resolves protean as a jar, and so does the `examples/quickstart` demo, so every path a console actually connects to reports a real version. The field is always present, so a client never has to distinguish "absent" from "unknown". -- `tracesEnabled` / `metricsEnabled` — `protean.trace.enabled` and `protean.trace.metrics.enabled`. They separate **"recording is off"** from **"recording is on but nothing has happened yet"**, which an empty stream cannot. +- `tracesEnabled` — `protean.trace.enabled`. Separates **"recording is off"** from **"recording is on but nothing has happened yet"**, which an empty stream cannot. +- `metricsEnabled` — **defined as `protean.trace.enabled` AND `protean.trace.metrics.enabled`**, not the metrics switch on its own. Recording is gated before aggregation is reached, so the switch means nothing while `tracesEnabled` is false. This is the same value the MCP metrics tool reports under the same name; an implementation of this contract that sends the raw switch is not conforming. + + ⚠️ **Read `tracesEnabled` first anyway.** The conjunction removes one wrong answer — `{"tracesEnabled": false, "metricsEnabled": true}` is not a state this frame can report — but it creates another. On `{"tracesEnabled": false, "metricsEnabled": false}` a client that looks only at `metricsEnabled` will say *"metrics are off; turn them on and rows appear"*, and turning them on changes nothing while recording is off. `metricsEnabled` answers "are metrics accruing", never "would enabling them help". - `buffered` — how many rows the `trace` frame that follows this one carries, **not** how many the ring holds. The two differ whenever `protean.trace.capacity` exceeds the 200-row replay cap; announcing a count while withholding the rows would be worse than announcing neither. - `tickMs` — the push period (`1000`), so a client's silence watchdog has a contract to size itself against instead of assuming one. - `capacity` — `protean.trace.capacity` **as of this connection**, letting a client say "nothing older than this is on the server". It is a live key, so a later change does not reach an already-connected client; the value is re-sent on reconnect. diff --git a/src/main/java/org/htcom/protean/runtime/TraceMetrics.java b/src/main/java/org/htcom/protean/runtime/TraceMetrics.java index 0e20c6b..9aa79a4 100644 --- a/src/main/java/org/htcom/protean/runtime/TraceMetrics.java +++ b/src/main/java/org/htcom/protean/runtime/TraceMetrics.java @@ -116,7 +116,13 @@ public Optional snapshot(String moduleId) { return m == null ? Optional.empty() : Optional.of(m.snapshot(key)); } - /** Snapshots of every tracked module (unordered). Empty when disabled or nothing observed yet. */ + /** + * Snapshots of every tracked module (unordered). Empty only when nothing has been observed yet — + * not when aggregation is disabled. {@code protean.trace.metrics.enabled} gates {@link #observe}, + * so turning it off stops new observations but retains what was already aggregated, and those rows keep + * being returned. Callers that need to say why a result is empty must read the flag separately rather + * than infer it from emptiness. + */ public List snapshots() { List out = new ArrayList<>(); for (Map.Entry e : byModule.entrySet()) { diff --git a/src/main/java/org/htcom/protean/web/StreamReady.java b/src/main/java/org/htcom/protean/web/StreamReady.java index a87fa8a..7445e62 100644 --- a/src/main/java/org/htcom/protean/web/StreamReady.java +++ b/src/main/java/org/htcom/protean/web/StreamReady.java @@ -26,7 +26,13 @@ * a fabricated answer to "which version am I talking to". * @param tracesEnabled {@code protean.trace.enabled}: distinguishes "recording is off" from "recording is on * but nothing has happened yet", which an empty stream cannot. - * @param metricsEnabled {@code protean.trace.metrics.enabled}: same distinction for the {@code metrics} frame. + * @param metricsEnabled {@code protean.trace.enabled} AND {@code protean.trace.metrics.enabled}, not the + * metrics switch alone: recording is gated before aggregation is reached, so the switch + * means nothing while {@code tracesEnabled} is false. This is the same value the MCP + * metrics tool reports under the same name. It answers "are metrics accruing" and + * not "would enabling them help" — on {@code (false, false)} a client reading + * only this field would offer to turn metrics on, which changes nothing while recording + * is off, so {@code tracesEnabled} still has to be read first. * @param buffered how many rows the {@code trace} frame that follows this one carries — not how * many the ring holds. The two differ whenever the ring is larger than the replay cap, and * announcing a count while withholding the rows is worse than announcing neither. diff --git a/src/main/java/org/htcom/protean/web/TraceAdminController.java b/src/main/java/org/htcom/protean/web/TraceAdminController.java index efb3dc2..05d02d9 100644 --- a/src/main/java/org/htcom/protean/web/TraceAdminController.java +++ b/src/main/java/org/htcom/protean/web/TraceAdminController.java @@ -65,7 +65,11 @@ public List recent(@RequestParam(defaultValue = "50") int limit, /** * Per-module aggregated metrics (opt-in; {@code protean.trace.metrics.enabled}). With {@code moduleId}, * returns that module's metrics (empty list if untracked); without it, every tracked module. - * Returns an empty list when metrics are disabled. + * + *

Disabling metrics does not empty this: the flag gates aggregation, not exposure, so rows + * observed before it was turned off keep being returned. An empty list therefore means "nothing was ever + * aggregated", never "aggregation is off" — a client that needs to tell those apart reads the flag, which + * the SSE {@code ready} frame carries as {@code metricsEnabled}. */ @GetMapping("/metrics") public List metrics(@RequestParam(required = false) String moduleId) { diff --git a/src/main/java/org/htcom/protean/web/TraceStreamService.java b/src/main/java/org/htcom/protean/web/TraceStreamService.java index a63fa53..9798a64 100644 --- a/src/main/java/org/htcom/protean/web/TraceStreamService.java +++ b/src/main/java/org/htcom/protean/web/TraceStreamService.java @@ -127,8 +127,12 @@ public SseEmitter open() { */ private StreamReady ready(int buffered) { ProteanProperties.Trace trace = props.getTrace(); - return StreamReady.of(trace.isEnabled(), trace.getMetrics().isEnabled(), buffered, TICK_MS, - trace.getCapacity()); + // metrics.enabled() is the platform's own answer to "are metrics on", and it already folds in + // trace.enabled -- recording is gated before aggregation is reached, so the metrics switch alone means + // nothing while tracing is off. Reading the raw switch here would make this frame disagree with the MCP + // tool, which reports the same name from the same accessor, and would let a client promise rows that can + // never arrive. + return StreamReady.of(trace.isEnabled(), metrics.enabled(), buffered, TICK_MS, trace.getCapacity()); } /** One push cycle: advance the cursor by the newest delta, then fan the deltas + fresh snapshots out. */ diff --git a/src/test/java/org/htcom/protean/TraceDisabledGatesMetricsTest.java b/src/test/java/org/htcom/protean/TraceDisabledGatesMetricsTest.java new file mode 100644 index 0000000..1ae3d07 --- /dev/null +++ b/src/test/java/org/htcom/protean/TraceDisabledGatesMetricsTest.java @@ -0,0 +1,133 @@ +/* + * Copyright (c) 2026 htjulia + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package org.htcom.protean; + +import org.htcom.protean.module.ModuleDescriptor; +import org.htcom.protean.module.ModulePlatform; +import org.htcom.protean.runtime.TraceMetrics; +import org.htcom.protean.runtime.TraceStore; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.test.web.servlet.MockMvc; + +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * {@code protean.trace.enabled} gates the whole recording path in {@code RequestTraceFilter}, ahead of + * per-module aggregation, so metrics accrue only while both it and {@code protean.trace.metrics.enabled} + * are true. {@link TraceMetrics#enabled()} already folds the two together, and the SSE {@code ready} ack must + * report that rather than the raw switch — otherwise the ack disagrees with the MCP metrics tool, which + * answers the same question from the same accessor, and a client acting on the ack alone would promise rows that + * can never arrive. + * + *

This runs in the configuration where the two readings diverge — the switch says on, the effective answer is + * off — so anything that goes back to reading the raw switch fails here rather than silently shipping. + */ +@SpringBootTest(properties = { + "protean.trace.enabled=false", // the outer gate + "protean.trace.metrics.enabled=true" // says "on", but nothing can reach it +}) +@AutoConfigureMockMvc +class TraceDisabledGatesMetricsTest { + + static final Path STORE_DIR = Path.of(System.getProperty("java.io.tmpdir"), "protean-trace-gate-test"); + + @DynamicPropertySource + static void props(DynamicPropertyRegistry registry) { + registry.add("protean.module-store.dir", STORE_DIR::toString); + } + + @Autowired MockMvc mockMvc; + @Autowired ModulePlatform platform; + @Autowired TraceMetrics metrics; + @Autowired TraceStore store; + @Autowired org.htcom.protean.autoconfigure.ProteanProperties props; + + static final String ID = "gate-mod"; + static final String FQCN = "runtime.gate.GateController"; + + @AfterEach + void cleanup() { + try { + platform.uninstall(ID); + } catch (RuntimeException ignored) { + } + } + + @Test + void metrics_report_off_and_never_accrue_while_trace_recording_is_off() throws Exception { + platform.install(descriptor()); + + // The switch is on, but the effective answer is off — and it is the effective answer the platform gives. + assertThat(props.getTrace().getMetrics().isEnabled()).isTrue(); + assertThat(metrics.enabled()).isFalse(); + + mockMvc.perform(get("/gate/ping")).andExpect(status().isOk()); + mockMvc.perform(get("/gate/ping")).andExpect(status().isOk()); + + // Requests are served, yet nothing is aggregated: the filter returns early on !trace.enabled, before the + // aggregation call is reached. Reporting the switch here would tell a client to expect these rows. + assertThat(metrics.snapshots()).isEmpty(); + assertThat(metrics.snapshot(ID)).isEmpty(); + // Same gate, so the ring is empty too — `buffered` in the ack is therefore 0, not a stale count. + assertThat(store.recent(200, null)).isEmpty(); + } + + @Test + void the_ready_ack_reports_the_effective_value_not_the_switch() throws Exception { + String body = mockMvc.perform(get("/platform/traces/stream") + .header("Accept", "text/event-stream")) + .andExpect(status().isOk()) + .andReturn().getResponse().getContentAsString(); + + String ready = body.lines().dropWhile(l -> !l.startsWith("event:ready")) + .filter(l -> l.startsWith("data:")).findFirst().orElseThrow(); + // Both false, although protean.trace.metrics.enabled is true: the ack answers "are metrics accruing", + // which is the same question the MCP tool answers, not "is the switch set". + assertThat(ready).contains("\"tracesEnabled\":false"); + assertThat(ready).contains("\"metricsEnabled\":false"); + } + + static ModuleDescriptor descriptor() { + String src = """ + package runtime.gate; + import org.springframework.web.bind.annotation.GetMapping; + import org.springframework.web.bind.annotation.RestController; + @RestController + public class GateController { + @GetMapping("/gate/ping") + public String ping() { return "pong"; } + } + """; + String test = """ + package runtime.gate; + import org.junit.jupiter.api.Test; + import static org.junit.jupiter.api.Assertions.assertEquals; + public class GateControllerTest { + @Test void ping() { assertEquals("pong", new GateController().ping()); } + } + """; + return ModuleDescriptor.builder() + .id(ID).version("1.0.0") + .controllerFqcn(FQCN).componentFqcns(List.of(FQCN)) + .sources(Map.of(FQCN, src)).tests(Map.of("runtime.gate.GateControllerTest", test)) + .build(); + } +} diff --git a/src/test/java/org/htcom/protean/TraceMetricsTest.java b/src/test/java/org/htcom/protean/TraceMetricsTest.java index 9afa80c..444bc47 100644 --- a/src/test/java/org/htcom/protean/TraceMetricsTest.java +++ b/src/test/java/org/htcom/protean/TraceMetricsTest.java @@ -40,6 +40,26 @@ void disabled_by_default_is_a_noop() { assertThat(m.snapshot("mod-a")).isEmpty(); } + @Test + void disabling_stops_new_observations_but_keeps_what_was_already_aggregated() { + ProteanProperties props = new ProteanProperties(); + props.getTrace().getMetrics().setEnabled(true); + TraceMetrics m = new TraceMetrics(props); + m.observe("mod-a", 200, 5, null, 1000); + + props.getTrace().getMetrics().setEnabled(false); // live key, flipped at runtime + + // The flag gates observation, not exposure. The row survives, so an empty result can never be read as + // "metrics are off" — the javadoc used to claim exactly that, and a caller believing it would report a + // disabled platform as an idle one. + assertThat(m.enabled()).isFalse(); + m.observe("mod-a", 500, 9, null, 1001); // dropped: this is what the flag actually stops + assertThat(m.snapshots()).hasSize(1); + ModuleMetricsSnapshot retained = m.snapshot("mod-a").orElseThrow(); + assertThat(retained.count()).isEqualTo(1); + assertThat(retained.errorCount()).isZero(); + } + @Test void counts_requests_and_errors() { TraceMetrics m = enabledMetrics(512); From d2c7a2aa99d7eabb0d9ffa92cce23b3725b2e527 Mon Sep 17 00:00:00 2001 From: htjulia Date: Mon, 21 Sep 2026 16:18:37 +0900 Subject: [PATCH 2/2] docs(stream): say that tickMs is a floor, not a guaranteed period The `ready` ack calls `tickMs` "the push period", and a client sizing a silence watchdog from it will read that as "a frame arrives every second". The ticker is scheduled with a fixed *delay*, so a cycle starts a second after the previous one ends -- and a cycle walks every module, recomputes the summary window over the whole ring, and serializes four frames to each connected client in turn. The effective interval stretches under exactly the load that makes a watchdog matter. So the number is a floor. A client that treats it as exact reports a healthy server as stalled, which is the failure the field was added to prevent. Found by the console session while consuming this contract: it sizes its stale timeout at six ticks and has room, but noticed that nothing in the contract said the value was a minimum. Co-Authored-By: htjulia --- docs/guide/04-rest-api.ko.md | 4 +++- docs/guide/04-rest-api.md | 4 +++- src/main/java/org/htcom/protean/web/StreamReady.java | 8 ++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/guide/04-rest-api.ko.md b/docs/guide/04-rest-api.ko.md index bdb4c5b..be9be98 100644 --- a/docs/guide/04-rest-api.ko.md +++ b/docs/guide/04-rest-api.ko.md @@ -423,7 +423,9 @@ data: {"platform":"protean","platformVersion":"0.1.0","tracesEnabled":true, ⚠️ **그래도 `tracesEnabled` 를 먼저 읽어야 한다.** 논리곱은 틀린 답 하나를 없애지만(`{"tracesEnabled": false, "metricsEnabled": true}` 는 이 프레임이 내보낼 수 없는 상태다) 다른 하나를 만든다. `{"tracesEnabled": false, "metricsEnabled": false}` 에서 `metricsEnabled` 만 보는 클라이언트는 *"메트릭이 꺼져 있다 — 켜면 행이 생긴다"* 고 말하는데, 기록이 꺼져 있는 한 켜도 아무 일도 일어나지 않는다. `metricsEnabled` 는 "메트릭이 쌓이고 있는가" 에 답할 뿐 **"켜면 도움이 되는가" 에는 답하지 않는다.** - `buffered` — 이 프레임 **다음에 오는 `trace` 프레임이 싣는 행 수**이며, 링이 보유한 행 수가 아니다. `protean.trace.capacity` 가 재생 상한 200 을 넘으면 둘이 달라진다. 숫자만 알리고 행은 주지 않는 것은 둘 다 안 하느니만 못하다. -- `tickMs` — 푸시 주기(`1000`). 클라이언트의 침묵 감지 워치독이 가정 대신 계약을 기준으로 자기 값을 정할 수 있게 한다. +- `tickMs` — 푸시 간격(`1000`). 클라이언트의 침묵 감지 워치독이 가정 대신 계약을 기준으로 자기 값을 정할 수 있게 한다. + + ⚠️ **이 값은 하한이지 보장이 아니다.** 티커는 *fixed delay* 로 걸려 있어 다음 주기는 이전 주기가 **끝난** 뒤 이만큼 지나 시작한다 — 그리고 한 주기는 모듈을 전수 조회하고, 링 전체로 윈도를 다시 계산하고, 연결된 클라이언트마다 차례로 네 프레임을 직렬화해 보낸다. 부하가 커지면 실효 간격이 늘어난다. 워치독은 `tickMs` 의 **넉넉한 배수**로 잡아야 한다 — 정확한 주기로 믿는 클라이언트는 멀쩡한 서버를 멈춘 것으로 보고하게 된다. - `capacity` — **이 연결 시점의** `protean.trace.capacity`. 클라이언트가 "이보다 과거는 서버에 없다" 를 말할 수 있다. LIVE 키이므로 이후 변경은 이미 연결된 클라이언트에 닿지 않으며, 재연결 때 새 값이 다시 나간다. 이후 버전에서 `ready` 에 필드가 더해질 수 있다 — 클라이언트는 모르는 필드를 무시해야 한다. diff --git a/docs/guide/04-rest-api.md b/docs/guide/04-rest-api.md index 151bdd8..6860a7d 100644 --- a/docs/guide/04-rest-api.md +++ b/docs/guide/04-rest-api.md @@ -423,7 +423,9 @@ It carries only what a client must know **once, at connect time** and cannot der ⚠️ **Read `tracesEnabled` first anyway.** The conjunction removes one wrong answer — `{"tracesEnabled": false, "metricsEnabled": true}` is not a state this frame can report — but it creates another. On `{"tracesEnabled": false, "metricsEnabled": false}` a client that looks only at `metricsEnabled` will say *"metrics are off; turn them on and rows appear"*, and turning them on changes nothing while recording is off. `metricsEnabled` answers "are metrics accruing", never "would enabling them help". - `buffered` — how many rows the `trace` frame that follows this one carries, **not** how many the ring holds. The two differ whenever `protean.trace.capacity` exceeds the 200-row replay cap; announcing a count while withholding the rows would be worse than announcing neither. -- `tickMs` — the push period (`1000`), so a client's silence watchdog has a contract to size itself against instead of assuming one. +- `tickMs` — the push interval (`1000`), so a client's silence watchdog has a contract to size itself against instead of assuming one. + + ⚠️ **It is a floor, not a guarantee.** The ticker is scheduled with a *fixed delay*, so the next cycle starts this long after the previous one **ends** — and a cycle walks every module, recomputes the window over the ring, and serializes four frames to every connected client in turn. Under load the effective interval stretches. Size a watchdog at a generous multiple of `tickMs`; a client that treats it as an exact period will report a healthy server as stalled. - `capacity` — `protean.trace.capacity` **as of this connection**, letting a client say "nothing older than this is on the server". It is a live key, so a later change does not reach an already-connected client; the value is re-sent on reconnect. Fields may be added to `ready` in later versions — a client must ignore the ones it does not recognize. diff --git a/src/main/java/org/htcom/protean/web/StreamReady.java b/src/main/java/org/htcom/protean/web/StreamReady.java index 7445e62..0c787e5 100644 --- a/src/main/java/org/htcom/protean/web/StreamReady.java +++ b/src/main/java/org/htcom/protean/web/StreamReady.java @@ -36,8 +36,12 @@ * @param buffered how many rows the {@code trace} frame that follows this one carries — not how * many the ring holds. The two differ whenever the ring is larger than the replay cap, and * announcing a count while withholding the rows is worse than announcing neither. - * @param tickMs the push period, so a client's silence watchdog has a contract to size itself against - * instead of assuming one. + * @param tickMs the push interval, so a client's silence watchdog has a contract to size itself against + * instead of assuming one. It is a floor, not a guarantee: the ticker runs on a + * fixed delay, so a cycle starts this long after the previous one ends, and a + * cycle walks every module, recomputes the window over the whole ring, and serializes four + * frames to each connected client in turn. A watchdog should allow a generous multiple of + * this rather than treat it as an exact period. * @param capacity {@code protean.trace.capacity} as of this connection, letting a client say * "nothing older than this is on the server". It is a live key, so a later change does not * reach an already-connected client; the value is re-sent on reconnect.