feat(request-cost): publish aggregate token telemetry to external endpoint

[wezell]

Fixes #35750

Summary

• Adds scheduled publishing of the per-window + lifetime request-cost (a.k.a. token) counters to an external REST collector.
• Reuses the existing RequestCostApiImpl monitor scheduler tick (default 60s, controlled by REQUEST_COST_TIME_WINDOW_SECONDS) — no new threads, no new schedulers.
• HTTP POST runs on DotConcurrentFactory.getInstance().getSubmitter() so a slow or dead collector never blocks the monitor scheduler.
• Off by default. Activates implicitly when both REQUEST_COST_PUSH_URL and REQUEST_COST_PUSH_TOKEN are set.

Why

RequestCostApiImpl already aggregates request counts and token totals (window + lifetime). Today those numbers are only emitted to the local log — they aren't queryable across a cluster. This wires the same aggregates into an external time-series sink so per-cluster, per-environment token consumption can be tracked centrally.

Wire format

One snapshot per tick (10 fields, exact):

{
  "clusterId": "...",
  "serverId": "...",
  "timestamp": "2026-05-19T18:48:00Z"  // ISO-8601, truncated to seconds,
  "windowSeconds": 60,
  "windowRequests": 1234,
  "windowTokens": 5678.5,
  "windowAvgTokensPerRequest": 4.6,
  "lifetimeRequests": 999999,
  "lifetimeTokens": 12345678.25,
  "lifetimeAvgTokensPerRequest": 12.35
}

• clusterId ← ClusterFactory.getClusterId()
• serverId ← ConfigUtils.getServerId()
• Both wrapped in Try with "unknown" fallback so a transient lookup failure doesn't drop the snapshot.
• timestamp is Instant.now().toString() (ISO-8601, UTC).
• Token values are pre-divided by REQUEST_COST_DENOMINATOR (consistent with what gets logged).

Config keys

Key	Default	Notes
REQUEST_COST_PUSH_URL	—	Presence + token activates the publisher
REQUEST_COST_PUSH_TOKEN	—	Bearer token; presence + url activates the publisher
REQUEST_COST_PUSH_TIMEOUT_MS	5000	Per-request timeout

Operability: CircuitBreakerUrl blocks private-subnet URLs unless ALLOW_ACCESS_TO_PRIVATE_SUBNETS=true is set on the same JVM. Most internal collectors live on a private network, so set that flag too if your collector hostname resolves to a private address. The flag is captured at first access via a static Lazy<Boolean>, so set it before traffic warms up.

Existing knobs (unchanged):

• REQUEST_COST_ACCOUNTING_ENABLED — overall request-cost gating
• REQUEST_COST_TIME_WINDOW_SECONDS — scheduler cadence (drives publish cadence too)
• REQUEST_COST_DENOMINATOR — scaling factor

Behavior

• Publisher emits every tick while enabled, even when the window had zero requests — keeps the time series continuous so idle vs. dead is distinguishable.
• Log line is suppressed on consecutive idle windows (unchanged from prior behavior).

Failure handling

• Transport errors and non-2xx responses are logged via Logger.warnEvery(..., 10 min) and the snapshot is dropped. This is observational telemetry, not durable accounting — buffering on disk would be over-engineered.
• publish() returns immediately; the actual POST happens on the shared submitter.

Test plan

• RequestCostSnapshotTest (3 tests) — locks JSON shape: every expected field present, all values round-trip correctly, exactly 10 fields on the wire (guards against accidental field leakage when someone adds a private helper later).
• RequestCostPublisherTest (9 tests) — enable gate: disabled by default, URL-alone is not enough, token-alone is not enough, both-set activates, publish() is a no-op when disabled.
• ./mvnw compile -pl :dotcms-core -DskipTests — BUILD SUCCESS.
• ./mvnw test -pl :dotcms-core -Dtest='RequestCostSnapshotTest,RequestCostPublisherTest' — 12/12 passing.
• Manual smoke: point REQUEST_COST_PUSH_URL at a local request bin, verify a snapshot lands within the window.

Out of scope

• A localhost-HttpServer integration test of the actual POST. CircuitBreakerUrl blocks private subnets via a static Lazy<Boolean> that captures ALLOW_ACCESS_TO_PRIVATE_SUBNETS on first access — that makes the test fragile to JVM/test ordering. The post path is mostly orchestration; serialization is fully covered by the snapshot test.
• Per-Price breakdown in the payload (e.g. {DB_QUERY: N, ES_QUERY: M}). Would require additional counters in incrementCost; can land as a follow-up if the collector needs it.
• Naming. The package and existing classes still say RequestCost; the new field names use "tokens" since that's the better term, but a package rename is a separate, larger refactor (24+ callers).

Files

• dotCMS/src/main/java/com/dotcms/cost/RequestCostSnapshot.java (new)
• dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java (new)
• dotCMS/src/main/java/com/dotcms/cost/RequestCostApiImpl.java (modified — adds snapshot construction + publisher call in logRequestCost)
• dotCMS/src/test/java/com/dotcms/cost/RequestCostSnapshotTest.java (new)
• dotCMS/src/test/java/com/dotcms/cost/RequestCostPublisherTest.java (new)

🤖 Generated with Claude Code

[claude]

Claude finished @wezell's task in 2m 49s —— View job

---

PR Review

Read all five files plus CircuitBreakerUrl, DotConcurrentFactory.getSubmitter().submit(...), and Logger.warnEvery*. Notes below — most prior review feedback (Copilot, fabrizzio, my earlier pass) still applies and a couple of points are worth re-emphasizing.

---

[🟠 High] CircuitBreakerUrl#doString() emits its own un-rate-limited WARN on any non-2xx — dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:93
At CircuitBreakerUrl.java:208-217, doString() calls Logger.warn(this, "Invalid response detected when consuming [%s] with http status [%d] and response:%s") directly whenever isError() is true (any 4xx/5xx, or unprocessed). That fires before your Logger.warnEvery(REQUEST_COST_PUSH_FAIL_HTTP, …) check at line 102-106, so a misconfigured collector still spams the log every tick — the rate limit you added is bypassed. Copilot flagged the same thing; I'm reinforcing it because the suggested fix (use doOut(OutputStream.nullOutputStream()) or a no-warning path) is a real, mechanical change you can land here, and without it the warnEvery machinery in this file is largely cosmetic for the common failure mode (a permanently broken collector URL).

---

[🟡 Medium] Submitter rejection isn't handled in publish() — dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:65
DotConcurrentFactory.getInstance().getSubmitter().submit(...) re-throws as DotConcurrentException (a RuntimeException) when the underlying executor's queue is saturated (DotConcurrentFactory.java:1043-1044). It is caught one frame up by logRequestCost()'s outer catch (Exception), so the scheduler survives — but the message that comes out is "Error logging request tokens: ...", which doesn't tell an operator that telemetry was dropped because the system pool was over capacity. If you don't want a try/catch here, at minimum mention in a comment that publish() can throw and rely on the upstream catch — currently the contract reads "fire-and-forget" but isn't.

---

[🟡 Medium] httpSchemeWarned is process-wide and never resets — dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:124-139
Restating from my prior pass since it didn't move: an operator who flips https:// → http:// → https:// → http:// only sees the first warning, then telemetry POSTs the bearer token in cleartext forever with no log trail. fabrizzio also pointed at this region. Suggest one of:

1. Refuse to post when scheme is http AND host is non-loopback (127.0.0.0/8, ::1, localhost), and warnEvery on the drop.
2. Or, keep the warn-once but tie the latch to the current URL so a flip back to http:// re-fires.

---

[🟡 Medium] sanitizeUrlForLog keeps the query string and fragment — dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:164-167
Restating: operators sometimes shove auth into the URL (?token=…). Userinfo gets stripped here, but query/fragment don't, so a misconfigured push URL still leaks the secret into every transport-error log. One-line fix:

final URI safe = new URI(uri.getScheme(), null, uri.getHost(), uri.getPort(),
                          uri.getPath(), null, null);

---

[🟡 Medium] RequestCostPublisherTest doesn't establish a clean starting state — dotCMS/src/test/java/com/dotcms/cost/RequestCostPublisherTest.java:24-29
@After clears, @Before does not. If anything in the JVM (a prior test class, a system property, an earlier setup hook) has already set REQUEST_COST_PUSH_URL or REQUEST_COST_PUSH_TOKEN, the very first test method in this class can see leaked state and test_isEnabled_returnsFalse_whenUrlAndTokenAreMissing flips false-positive. Copilot's point — make the clear method run before each test too (or rename to @Before and drop @After).

---

[🟢 Low / Nit] A few smaller items

• dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:35 — FAIL_LOG_INTERVAL_MS is hard-coded; fabrizzio suggested TimeUnit.MINUTES.toMillis(10) and/or making it configurable. Either is cheap.
• dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:81-82 — Map.of("Authorization", "Bearer " + token) is the idiomatic shape (fabrizzio's nit) — headers is never mutated after construction.
• dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:109-115 — Logger.warnEveryAndDebug(cl, message, throwable, intervalMs) exists (Logger.java:209) and bundles the warnEvery + debug-with-throwable pair you wrote into one call.
• dotCMS/src/main/java/com/dotcms/cost/RequestCostSnapshot.java:14 — fabrizzio asked about Immutables. The class is already public final with public final fields, an all-args ctor, and locked Jackson visibility; converting to a @Value.Immutable interface mostly trades hand-written for generated. Not worth the churn unless you want the builder.
• dotCMS/src/main/java/com/dotcms/cost/RequestCostApiImpl.java:68-69 — Math.max(1.0d, NaN) returns NaN. Config.getFloatProperty won't normally produce NaN, but if someone literally configures REQUEST_COST_DENOMINATOR=NaN, this clamp doesn't save you and the snapshot still ships JSON-invalid NaN literals. A Double.isFinite() guard would close that.
• dotCMS/src/main/java/com/dotcms/cost/RequestCostApiImpl.java:131 — the isEnabled() check here and the one inside publish() are redundant; fine, just noting.

---

Verdict
Design is sound — reusing the existing tick, off-by-default gate, no new threads, snapshot shape locked by tests, no caller-blocking POST. The 🟠 above is the one I'd most want addressed before merge: a misconfigured collector currently makes the publisher's own rate limiter pointless because CircuitBreakerUrl warns out from under it. Everything else is incremental.
• branch: feat/request-cost-publisher

[claude]

🔍 dotCMS Backend Review

[🟡 Medium] dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:121-136

Plain-HTTP downgrade is warned-once-then-allowed. An operator who pastes http:// (or whose config is tampered with) silently ships the bearer token in cleartext to any host — including private subnets when ALLOW_ACCESS_TO_PRIVATE_SUBNETS=true. Because httpSchemeWarned is a process-lifetime AtomicBoolean, a runtime flip https→http→https→http is silently re-enabled with no log line, and after the warning rotates out of retained logs the cleartext POSTs continue with no visible trail.

if (scheme != null && scheme.equalsIgnoreCase("http")
        && httpSchemeWarned.compareAndSet(false, true)) {
    Logger.warn(this.getClass(), "...bearer token will be sent in cleartext...");
}

💡 Refuse to post when scheme is http AND host is non-loopback. 127.0.0.0/8 / ::1 / localhost are reasonable carve-outs for dev/sidecar collectors; everything else should drop the snapshot and warnEvery so the failure is recurringly visible.

---

[🟡 Medium] dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:140-145

sanitizeHeaderValue strips only \r and \n, but RFC 7230 §3.2.4 prohibits all CTLs in header field-values (NUL \0, vertical tab �, form feed \f, the rest of �–�, and �). Apache HttpClient typically rejects bare LF/CR, but a NUL byte can survive on some HTTP stacks and corrupt the wire format. More importantly: the gate in isEnabled() reuses this sanitizer for the empty-check — a token consisting solely of non-CRLF CTLs would currently bypass the isSet gate and activate the publisher with effectively-empty auth.

static String sanitizeHeaderValue(final String value) {
    return value == null ? null : value.replace("\r", "").replace("\n", "").trim();
}

💡 Strip the whole CTL range:

return value == null ? null : value.replaceAll("[\\x00-\\x1F\\x7F]", "").trim();

---

[🟡 Medium] dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java:148-168

sanitizeUrlForLog strips userinfo but preserves the query string and fragment. If an operator misconfigures the URL with the bearer token (or any other secret) as a query parameter — https://host/cost?token=secret — the secret lands in every transport/HTTP/exception failure log line. The query string is not needed to disambiguate the target in operator logs.

final URI safe = new URI(
        uri.getScheme(), null, uri.getHost(), uri.getPort(),
        uri.getPath(), uri.getQuery(), uri.getFragment());
return safe.toString();

💡 Drop the query and fragment too — only scheme/host/port/path are needed for log disambiguation:

final URI safe = new URI(uri.getScheme(), null, uri.getHost(), uri.getPort(),
                          uri.getPath(), null, null);

---

Next steps

• 🔴 / 🟠 Fix locally and push — these need your judgment
• 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in <File.java>
• Every new push triggers a fresh review automatically

[Copilot]

Pull request overview

Adds opt-in, scheduled publishing of aggregated request-cost (“token”) telemetry to an external HTTP collector, reusing the existing RequestCostApiImpl monitor tick so cluster-wide usage can be tracked outside local logs.

Changes:

• Introduces RequestCostSnapshot as a fixed-shape JSON payload for per-window and lifetime token aggregates.
• Adds RequestCostPublisher to POST snapshots asynchronously via DotConcurrentFactory and CircuitBreakerUrl, gated by REQUEST_COST_PUSH_URL + REQUEST_COST_PUSH_TOKEN.
• Extends RequestCostApiImpl#logRequestCost() to build/publish snapshots each tick while keeping console log throttling for consecutive idle windows; adds unit tests locking payload shape and enablement behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
dotCMS/src/main/java/com/dotcms/cost/RequestCostSnapshot.java	Defines the on-the-wire snapshot payload and locks JSON field visibility.
dotCMS/src/main/java/com/dotcms/cost/RequestCostPublisher.java	Implements async POST publishing with config gating and basic hardening/sanitization.
dotCMS/src/main/java/com/dotcms/cost/RequestCostApiImpl.java	Builds snapshot from existing counters and triggers publisher from the existing scheduler tick.
dotCMS/src/test/java/com/dotcms/cost/RequestCostSnapshotTest.java	Verifies snapshot JSON contains the expected fields/values and exactly 10 fields.
dotCMS/src/test/java/com/dotcms/cost/RequestCostPublisherTest.java	Verifies enablement gate, disabled no-op, and sanitization helpers.