Fix RequestCost instrumentation gaps and re-base the price table on resource-time
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 970
- Forks
- 486
- Avg merge
- 3d 33m
- Merged PRs (30d)
- 170
Description
Description
@RequestCost scores the "heaviness" of a request, token-style, so usage can be metered and rate-limited. An audit of where the annotation is actually placed against production behaviour — Glowroot main-thread profiles from six tenant instances, 7-day windows — found the instrumentation concentrated on the Contentlet API while the traffic was somewhere else entirely, and one class of cost being discarded outright.
Six instances, six different cost centres
| Shape | Where the time actually went | Metered before? |
|---|---|---|
| write-heavy API | addContentToIndex — ~10% of all samples, blocking |
no |
| GraphQL 60% / VTL 31% | prerender park 36%, NavTool.getNav 11% |
no |
| Velocity + remote API | CircuitBreakerUrl outbound HTTP ~50% |
yes, priced 4 |
| GraphQL 54% | GraphQL execute, transform, relationship N+1 | no |
| traditional Velocity | filter chain, render, binaries | partly |
A GraphQL-only customer could saturate a node and accrue almost nothing.
Background work was billing zero
incrementCost returned early when no HttpServletRequest was on the thread — before touching the totals that feed the collector. Site-search reindexing, scheduled publishing, remote/push publishing and content indexing already flow through priced methods (CONTENT_INDEX, CONTENT_CHECKIN, HTTP_FETCH); all of it was priced correctly and then thrown away at the last step.
Acceptance Criteria
- Every search result path is metered — GraphQL,
/api/content,/api/es/search, page render all bulk-load throughESContentFactoryImpl.findContentlets, which bypassedfind()and was uncosted - Cost scales with contentlets returned, and cached content is materially cheaper than content read from the database
- Content pricing does not vary with implementation details the customer cannot influence (batch size, statement count, query plan)
- Background work reaches the collector instead of being discarded, reported separately from request cost
-
/api/es/search,/api/vtl/*and#dotParseno longer bypass their existing charges - Price table reflects relative resource-time; image transforms and Tika metadata generation are not priced below a cache read
- Reported token values stay in their historical range and remain integral
- Rate-limit defaults scale with the price table
- No annotation double-charges via a ByteBuddy self-invocation chain
- Existing unit tests updated for the snapshot shape change
Additional Context
Pricing model. Two coherent options — meter primitives (accurate, but the bill moves with our cache hit rate and batch sizes) or meter customer-visible operations (stable, optimisable). Chosen: the latter, with the line drawn at what the customer can influence:
| Whose? | Priced? | |
|---|---|---|
| Did this need the database at all? | theirs — cacheable containers/pages, TTLs, query shape | yes |
| How many statements, what batch size, which plan? | ours | no |
Content is priced CONTENT_FROM_CACHE(1) per contentlet asked for, plus CONTENT_FROM_DB(10) as a surcharge on the ones read from Postgres — so 1 warm, 11 cold. The same cheap-hit/expensive-miss shape already existed for ES queries and file metadata and is now consistent across the model.
Collector change required. The pushed RequestCostSnapshot gains windowJobTokens and lifetimeJobTokens (doubles). They are reported separately — windowTokens stays request-only, total consumption is the sum. Unknown fields are dropped silently by the ingestor, so this is safe to deploy ahead of the ingestor change, but background cost is not billed until those two columns exist.
Expect a step change in reported tokens at deploy: prices were re-based and the denominator moved 1 → 10 to compensate. Roughly range-preserving for requests, but per-endpoint series shift by different amounts.
Not validated against a running instance. Prices are order-of-magnitude estimates from sampled time-share; placements were verified by reading call chains. The calibration loop — run a week, compare per-endpoint mean response time against mean cost, investigate outlier ratios — is what turns these into measured numbers.
Known gaps, deliberately out of scope:
- Embedding generation / vector DB indexing is not priced at all — no constant, nothing annotated. Likely the largest remaining gap given inference cost.
- Job cost is one undifferentiated bucket, not split by job type. Needs a job-name context set by background entry points.
- The telemetry publisher self-charges
HTTP_FETCHfor its own push (~10 tokens/window). - Hibernate is invisible to any DB-level metering —
PermissionBitAPIImpl.getPermissionsByRolegoes throughnet.sf.hibernate.loader.Loader, neverDotConnect.
Full analysis and evidence ship with the PR as docs/requestcost-session-summary.md and docs/requestcost-placement-analysis.md.
Related defects found during the audit: #36970, #36971.
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 the RequestCost instrumentation and the named entry points, including ESContentFactoryImpl.findContentlets, incrementCost, and the search, VTL, and #dotParse paths. Read the collector's RequestCostSnapshot shape and the requestcost placement analysis documents, then inspect the related unit tests. Done means all listed paths are charged without double-counting, background costs are reported separately, and the snapshot and price-table tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elasticsearch, graphql, java, postgresql
- Domain
- api, backend, databases, observability
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 32/100