feat: register explicit shared-lN-domain fields for CPU cache breakouts - #208
Conversation
Registers shared-l1-domain through shared-l4-domain as explicit keyword fields in metric_desc.base, for tool-sysstat's upcoming per-cache-level NUMA-style breakout (mpstat cpu utilization grouped by shared cache domain). A dynamic_templates wildcard was tried first but verified live against OpenSearch to not work -- "dynamic": "strict" rejects any field matching a dynamic_templates glob exactly the same as an unregistered field; there is no bypass. Explicit fields match this schema's existing convention and fail loudly (one rejected document, not silently lost data) if a future host reports an L5+ cache level.
PR Review: CommonDataModel#208 — feat: register explicit shared-lN-domain fields for CPU cache breakoutsSummary: Registers Issues
File Coverage
Missing from diff
diff --git a/queries/cdmq/cdm.js b/queries/cdmq/cdm.js
index cdf59e8..b9708ab 100644
--- a/queries/cdmq/cdm.js
+++ b/queries/cdmq/cdm.js
@@ -278,6 +278,10 @@ indexDefs['v8dev']['metric_desc']['mappings']['properties']['metric_desc'] = {
mode: { type: 'keyword' },
socket: { type: 'keyword' },
domain: { type: 'keyword' },
+ 'shared-l1-domain': { type: 'keyword' },
+ 'shared-l2-domain': { type: 'keyword' },
+ 'shared-l3-domain': { type: 'keyword' },
+ 'shared-l4-domain': { type: 'keyword' },
cluster: { type: 'keyword' },
container: { type: 'keyword' },
cgroup: { type: 'keyword' },Limitations
VerdictRequest changes — The templates are correct and compile perfectly. However, the programmatic schemas defined in |
queries/cdmq/cdm.js maintains its own copy of the metric_desc.names schema (indexDefs['v8dev']['metric_desc'], deep-cloned into v9dev and v10dev), separate from templates/metric_desc.base. It's load-bearing in two places: checkCreateIndex()/updateIndexMappings() PUT it directly to OpenSearch as an alternate index-creation path, and add-run.js -- the actual production indexing script -- validates every document's fields against it, rejecting anything not listed. Without this fix, add-run.js would reject tool-sysstat's cache-domain data before it ever reached OpenSearch, even with the template already updated. Found via PR review; verified by tracing add-run.js's field validation and confirming v10dev inherits the fix through the deep-clone chain.
|
Good catch — verified and fixed in cec90a0.
Added the same four fields at the matching location in |
atheurer
left a comment
There was a problem hiding this comment.
LGTM! Approved after compiling templates and validating the JSON schemas.
Summary
shared-l1-domainthroughshared-l4-domainas explicitkeywordfields inmetric_desc.base, to support tool-sysstat's upcoming per-cache-level breakout (issue mpstat: add 'node' (NUMA node) and 'cpu' (alias for num) as CDM breakout dimensions tool-sysstat#68).CLAUDE.md: adynamic_templateswildcard approach was tried first but verified live against OpenSearch to provide no exception under"dynamic": "strict"— a field matching adynamic_templatesglob is rejected exactly like any other unregistered field. Explicit fields are the only working option and match this schema's existing convention.Test plan
metric_desc.jsonfrom the.basefragments viabuild.sh/Makefile, confirmed valid JSON with the new fields present and nodynamic_templateskeyshared-l3-domainis accepted and a document with an unregisteredshared-l5-domainis rejected withstrict_dynamic_mapping_exception