dotCMS / dotCMS/core

Fix RequestCost instrumentation gaps and re-base the price table on resource-time

Open
#36,977 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotCMS : Technical Debt Team : Platform Type : Task
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 through ESContentFactoryImpl.findContentlets, which bypassed find() 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 #dotParse no 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 separatelywindowTokens 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_FETCH for its own push (~10 tokens/window).
  • Hibernate is invisible to any DB-level metering — PermissionBitAPIImpl.getPermissionsByRole goes through net.sf.hibernate.loader.Loader, never DotConnect.

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.