Add allowlisted metadata-only quota telemetry by arkmish · Pull Request #155 · linkedin/zookeeper

arkmish · 2026-09-28T06:58:38Z

Description

Expose recorded namespace quota usage and limits for monitoring without recursively scanning the live data tree or changing quota enforcement.

The new AdminServer command is:

/commands/quota_stats?path=/app

Requests require an exact match in zookeeper.quotaStats.allowedNamespaces, a strictly validated JSON array that defaults to [].

Validate the request path and the entire allowlist before accessing the tree. Root and reserved /zookeeper paths are rejected.
Read only the namespace's existence and its exact existing quota-stat/limit nodes. Do not substitute an ancestor quota or recount the subtree.
Return schema version, path, recorded count/byte usage and limits, plus explicit available and reason fields.
Distinguish missing, incomplete, malformed, or changed metadata. Unavailable samples have null numeric fields; native unset limits are null, while configured zero remains zero.
Use standard command errors for invalid requests/configuration and avoid echoing raw invalid metadata.

The allowlist limits which paths can be disclosed; it does not replace AdminServer access controls. Sampling remains non-atomic and may reflect accounting lag.

Tests

Validated CommandsTest, DataTreeTest, ServerMetricsTest, and ZooKeeperQuotaTest using Maven on JDK 11 with Java 8 API targeting:

Tests run: 76, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Coverage includes exact JSON/null semantics, strict path/allowlist validation, numeric bounds, missing and malformed quota records, no ancestor substitution, bounded read-only access, concurrent changes, namespace removal, and server restarts.

Local code review completed

Changes that Break Backward Compatibility (Optional)

No client protocol, ACL decision, persistence-format, quota-enforcement, or write-accounting change. The command is default-deny and supplies no production namespace approvals.

available=true means the required metadata was readable and valid for the sample. It does not establish freshness, atomic consistency, safe capacity, or an approved operational budget.

Documentation (Optional)

Updated zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md with configuration syntax, command examples, response/error schemas, and sampling limitations.

🤖 Generated with GitHub Copilot CLI

Expose quota_stats through AdminServer with a default-empty exact-path JSON allowlist. Sample existing quota records without subtree traversal or write-path changes, report unavailable data explicitly, and cover validation, concurrent changes and restart behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Retain upstream SPIFFE authorization and quota statistics as an independent feature branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bellatrix007 · 2026-10-19T10:36:24Z


+* *quota_stats* :
+    Read only the existing quota usage and limit metadata for one explicitly
+    allowlisted namespace. Requires a `path` parameter. See


What is namespace in this ?

bellatrix007 · 2026-10-19T10:50:42Z

+        if (getNode(path) != namespace || getNode(statPath) != statNode || getNode(limitPath) != limitNode) {
+            return new QuotaStats(null, null, "quota_changed");
+        }


is this to catch any race conditions?

bellatrix007 · 2026-10-19T11:16:32Z

+protected administrative access path; this command adds no authentication,
+ACL policy, transport listener or four-letter command.
+
+The JVM property `zookeeper.quotaStats.allowedNamespaces` must contain one


Operability: zkServer.sh start expands $JVMFLAGS unquoted, so whitespace splits the flag. ["/a", "/b"] becomes two args, the property is invalid JSON, and every request gets the config error (fails closed, but confusing). Please document the no-space form -Dzookeeper.quotaStats.allowedNamespaces=["/a","/b"] (it must be a single shell word). Optional: also accept a comma-separated list to avoid quoting.

bellatrix007 · 2026-10-19T11:16:34Z

+does not walk the live subtree, search ancestor quotas, create watches,
+repair accounting, alter enforcement or register per-path metrics.
+
+**This is not an atomic or historical snapshot.** Stat and limit records are


Nested quotas: ZooKeeper charges a write only to the deepest ancestor that has a quota (getMaxPrefixWithQuota -> PathTrie.findMaxPrefix). If /app/team1 has its own quota, its writes never reach the zookeeper_stats of /app, so /app undercounts the subtree while available=true. Please add this to the limitations.

bellatrix007 · 2026-10-19T11:16:36Z

+            final boolean allowed;
+            try {
+                allowed = isAllowed(path, System.getProperty(ALLOWED_NAMESPACES, "[]"));
+            } catch (IOException | IllegalArgumentException | SecurityException e) {


Non-blocking: the exception is swallowed, so a bad allowlist is visible only to the HTTP caller. Consider a one-time or rate-limited LOG.warn with the exception class (not the raw value) so operators can find it in the server log.

bellatrix007 · 2026-10-19T11:16:38Z

+allowlist entry and its own quota metadata. Encode the `path` query parameter
+normally when its characters require URL encoding.
+
+Invalid/missing paths, invalid configuration and disallowed paths return only


Worth stating explicitly: AdminServer returns HTTP 200 even when error is non-null (JettyAdminServer.CommandServlet always sets SC_OK). Clients must check error, not the HTTP status.

bellatrix007 · 2026-10-19T11:16:39Z

+    }
+
+    @Test
+    public void testQuotaStatsDeniedConfigurationAccess() {


System.setSecurityManager is deprecated for removal and throws UnsupportedOperationException on JDK 18+ (unless -Djava.security.manager=allow); it also mutates JVM-global state. It passes on the current JDK 8/11 matrix but will break on upgrade. Suggest a small package-private hook for the property read so the SecurityException branch can be tested directly, or dropping this test since the catch is purely defensive.

bellatrix007 · 2026-10-19T11:16:41Z

+    }
+
+    @Test
+    public void testQuotaStatsAfterServerRestart() throws Exception {


Two stop/start cycles with a single assertion at the end. If the intent is log replay followed by snapshot load, please assert after each restart and add a comment; otherwise one cycle is enough. As written, a failure cannot be attributed to a specific path.

bellatrix007 · 2026-10-19T11:16:43Z

+                assertTrue("Unexpected subtree lookup", "/quota-test".equals(path)
+                           || Quotas.statPath("/quota-test").equals(path)
+                           || Quotas.quotaPath("/quota-test").equals(path));
+                assertTrue("Quota telemetry must perform bounded lookups", ++lookups <= 6);


<= 6 hard-codes the current 3 lookups + 3 identity rechecks (same constant in the two tests asserting tree.lookups <= 6). The path whitelist above already guards against subtree walks. Consider a named constant with a comment so a legitimate extra lookup is not read as a regression.

bellatrix007 · 2026-10-19T11:16:45Z


    private static final Logger LOG = LoggerFactory.getLogger(DataTree.class);

+    private static final Pattern QUOTA_VALUE = Pattern.compile("count=-?[0-9]+,bytes=-?[0-9]+");


Forward-port note: this matches only the 3.6 two-field format. Upstream master StatsTrack is key/value based (countHardLimit/byteHardLimit, , or ; separators, trailing = sentinel such as bytes=5=), so on 3.7+ valid records would be reported as invalid_quota_stats/invalid_quota_limits. Fine for 3.6, but worth a comment here and a format-aware parse when this is ported.

bellatrix007 · 2026-10-19T11:17:07Z

The commit title ends with "(B1)", which suggests a multi-part series. Could you link the other parts or a tracking issue so reviewers can see the intended scope?

bellatrix007 · 2026-10-19T11:17:09Z

The branch contains a merge commit (Merge branch-3.6 into quota statistics). If the repo prefers linear history, please squash or rebase before merging.

This was referenced Sep 28, 2026

Add opt-in write-size and outcome audit metadata #152

Open

Backport the native snapshot size comparer #153

Merged

Add snapshot diagnostics and opt-in audit and quota telemetry #150

Closed

arkmish added quota telemetry Namespace quota usage, limits, and bounded telemetry collection monitoring Observability, collection coverage, dashboards, and alerting labels Sep 30, 2026

Merge branch-3.6 into quota statistics

b741e33

Retain upstream SPIFFE authorization and quota statistics as an independent feature branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

arkmish marked this pull request as ready for review October 8, 2026 05:28

bellatrix007 reviewed Oct 19, 2026

View reviewed changes

Navigation Menu

Repository navigation

Add allowlisted metadata-only quota telemetry - #155

Add allowlisted metadata-only quota telemetry#155
arkmish wants to merge 2 commits into
branch-3.6from
armishra/zk-quota-stats

arkmish commented Sep 28, 2026 •

edited

Loading

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026 •

edited

Loading

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 Oct 19, 2026 •

edited

Loading

Uh oh!

bellatrix007 Oct 19, 2026

Uh oh!

bellatrix007 commented Oct 19, 2026

Uh oh!

bellatrix007 commented Oct 19, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		private static final Logger LOG = LoggerFactory.getLogger(DataTree.class);

		private static final Pattern QUOTA_VALUE = Pattern.compile("count=-?[0-9]+,bytes=-?[0-9]+");

Conversation

arkmish commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Changes that Break Backward Compatibility (Optional)

Documentation (Optional)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bellatrix007 Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bellatrix007 Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bellatrix007 commented Oct 19, 2026

Uh oh!

bellatrix007 commented Oct 19, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arkmish commented Sep 28, 2026 •

edited

Loading

bellatrix007 Oct 19, 2026 •

edited

Loading

bellatrix007 Oct 19, 2026 •

edited

Loading