feat: validate incompatible metric aggregations

CLAUDE.md

@@ -35,7 +35,7 @@ Supporting document types: `param`, `tag`, `config_*`
 - Versions tracked as git branches and in `VERSION` file (currently `v10dev`)
 - `cdm.js` exports `supportedCdmVersions` array: `['v7dev', 'v8dev', 'v9dev', 'v10dev']`
 - Index naming pattern: `cdm{VERSION}-{DOCTYPE}*` (e.g., `cdmv10dev-metric_data*`)
-- v10dev's key addition is `default-aggregation` — a per-metric field on `metric_desc` (`sum`/`avg`/`max`/`min`) telling query-time aggregation how to combine values across breakout dimensions, instead of always duration-weighted summing
+- v10dev's key addition is `default-aggregation` — a per-metric field on `metric_desc` (`sum`/`avg`/`max`/`min`) telling query-time aggregation how to combine values across breakout dimensions, instead of always duration-weighted summing. Metric definitions may also provide `disallowed-aggregations` for combinations that are not meaningful for that specific metric.
 
 ## Templates (`templates/`)
 - Index mappings for every document type are defined in `queries/cdmq/cdm.js`'s `indexDefs` object (`v8dev` hand-written, `v9dev`/`v10dev` built forward via `deepClone`). This is the only live schema — `add-run.js`'s `checkCreateIndex()`/`updateIndexMappings()` use it to create/update OpenSearch indices, and also use it as a client-side validation gate, rejecting any document field not present in it before the document is ever sent to OpenSearch.

queries/cdmq/README.md

@@ -32,6 +32,14 @@ Many of the scripts refer to different terms we associate with either running a
 
 Below are documented most common scripts used for this project. All of these scripts can be run via `node ./script-name.js`, and some have wrapper scripts `script-name.sh` which provide the casual user a more convenient invocation. If you are using [crucible](https://github.com/perftool-incubator/crucible), it may provide an alternative way to use this script (documented in each script subsection below).
 
+### Aggregation overrides
+
+`--aggregation` overrides a metric's `default-aggregation`. Metric definitions
+may declare combinations that are not meaningful for that metric using
+`metric_desc.disallowed-aggregations`. Such a query returns an error by
+default. Use `--allow-incompatible-aggregation` when the combination is
+intentional and should still be evaluated.
+
 ### get-result-summary.js
 
 This script produces a summary of a single run., including tags, metrics present, as well as all the iterations and their samples. To run this script, you must specify a run-id: `node ./get-result-summary.js --run 0bda53c3-f0b2-416a-be54-cee738b75010`. If you are using the crucible project, you will likely be using the crucible command-line `crucible get result --run 0bda53c3-f0b2-416a-be54-cee738b75010`. In this example, the following output is produced:

queries/cdmq/cdm.js

@@ -382,6 +382,9 @@ indexDefs['v9dev']['metric_desc'] = deepClone(indexDefs['v8dev']['metric_desc'])
 indexDefs['v9dev']['metric_desc']['mappings']['properties']['metric_desc']['properties']['default-aggregation'] = {
   type: 'keyword'
 };
+indexDefs['v9dev']['metric_desc']['mappings']['properties']['metric_desc']['properties']['disallowed-aggregations'] = {
+  type: 'keyword'
+};
 
 // TODO: add new names for cdmv9
 
@@ -3490,6 +3493,40 @@ getDefaultAggregation = function (instance, run, source, type, yearDotMonth) {
   return 'sum';
 };
 
+// A metric class alone is not sufficient to determine whether an aggregation is
+// meaningful: for example, averaging boolean values can represent the true
+// fraction. Metric producers can explicitly declare only combinations that are
+// invalid for their metric, leaving unusual but intentional overrides to the
+// caller's discretion.
+getDisallowedAggregations = function (instance, run, source, type, yearDotMonth) {
+  var q = {
+    size: 10000,
+    _source: ['metric_desc.class', 'metric_desc.disallowed-aggregations'],
+    query: {
+      bool: {
+        filter: [
+          { term: { 'run.run-uuid': run } },
+          { term: { 'metric_desc.source': source } },
+          { term: { 'metric_desc.type': type } }
+        ]
+      }
+    }
+  };
+  var resp = esRequest(instance, 'metric_desc', '/_search', q, yearDotMonth);
+  var data = JSON.parse(resp.getBody());
+  var result = [];
+  if (data.hits && data.hits.hits) {
+    data.hits.hits.forEach((hit) => {
+      var desc = hit._source && hit._source.metric_desc;
+      if (desc && Array.isArray(desc['disallowed-aggregations'])) {
+        result.push({ class: desc.class, aggregations: desc['disallowed-aggregations'] });
+      }
+    });
+  }
+  return result;
+};
+exports.getDisallowedAggregations = getDisallowedAggregations;
+
 // --------------------------------------------------------------------------------------------------------------
 calcAvg = function (
   thisBegin,
@@ -3841,6 +3878,11 @@ getMetricDataSets = async function (instance, sets, yearDotMonth) {
   var retCode = 0;
   var retMsg = '';
   for (var i = 0; i < sets.length; i++) {
+    // Keep the public request spelling hyphenated while accepting the internal
+    // camelCase form used by the query library during validation.
+    if (isDefined(sets[i]['allow-incompatible-aggregation'])) {
+      sets[i].allowIncompatibleAggregation = sets[i]['allow-incompatible-aggregation'];
+    }
     // If a begin and end are not defined, get it from the period.begin & period.end.
     // If a begin and/or end are not defined, and the period is not defined, error out.
     // If a run is not defined, get it from the period.
@@ -3972,6 +4014,39 @@ getMetricDataSets = async function (instance, sets, yearDotMonth) {
   }
   var metricGroupIdsByLabelSets = resp['metric-id-sets'];
 
+  // Reject only combinations explicitly declared invalid by the metric
+  // definition. A caller can bypass this check when the unusual aggregation
+  // is intentional.
+  for (var idx = 0; idx < sets.length; idx++) {
+    if (sets[idx].aggregation && !sets[idx].allowIncompatibleAggregation) {
+      var disallowed = getDisallowedAggregations(
+        instance,
+        sets[idx].run,
+        sets[idx].source,
+        sets[idx].type,
+        yearDotMonth
+      );
+      var invalid = disallowed.filter((entry) => entry.aggregations.includes(sets[idx].aggregation));
+      if (invalid.length > 0) {
+        var classes = invalid
+          .map((entry) => entry.class)
+          .filter((value, position, values) => value && values.indexOf(value) === position)
+          .join(', ');
+        retMsg =
+          'ERROR: aggregation [' +
+          sets[idx].aggregation +
+          '] is disallowed for [' +
+          sets[idx].source +
+          '::' +
+          sets[idx].type +
+          ']';
+        if (classes) retMsg += ' (metric class: ' + classes + ')';
+        retMsg += '. Use allow-incompatible-aggregation to run this query when the combination is intentional.';
+        return { 'ret-code': 4, 'ret-msg': retMsg, code: 'INCOMPATIBLE_AGGREGATION' };
+      }
+    }
+  }
+
   // Check if any regex filters resulted in zero matches
   for (var idx = 0; idx < metricGroupIdsByLabelSets.length; idx++) {
     if (Object.keys(metricGroupIdsByLabelSets[idx]).length === 0) {

queries/cdmq/get-metric-data.js

@@ -163,6 +163,10 @@ async function main() {
       '[optional] Filter out (do not output) metrics which do not pass the conditional.  gt=greater-than, ge=greater-than-or-equal, lt=less-than, le=less-than-or-equal'
     )
     .option('--aggregation <sum|avg|max|min>', '[optional] Override the default aggregation method for this query')
+    .option(
+      '--allow-incompatible-aggregation',
+      '[optional] Allow an aggregation explicitly disallowed by the metric definition'
+    )
     .option('--output-format <json|table|csv>', 'table')
     .option(
       '--date-format <default|epoch_ms>',
@@ -208,6 +212,7 @@ async function main() {
     breakout: program.breakout, // Send as array to preserve complex breakout syntax
     filter: program.filter,
     aggregation: program.aggregation,
+    'allow-incompatible-aggregation': program.allowIncompatibleAggregation,
     instances: program.instances.length > 0 ? program.instances : undefined
   };
 

queries/cdmq/server.js

@@ -1659,6 +1659,7 @@ app.post('/api/v1/metric-data', async (req, res) => {
       'breakout',
       'filter',
       'aggregation',
+      'allow-incompatible-aggregation',
       'instances'
     ];
     var fieldErr = validateBodyFields(req.body, knownFields);
@@ -1676,6 +1677,7 @@ app.post('/api/v1/metric-data', async (req, res) => {
       breakout,
       filter,
       aggregation,
+      'allow-incompatible-aggregation': allowIncompatibleAggregation,
       instances: reqInstances
     } = req.body;
 
@@ -1781,12 +1783,13 @@ app.post('/api/v1/metric-data', async (req, res) => {
       resolution: resolution,
       breakout: breakout,
       filter: filter,
-      aggregation: aggregation
+      aggregation: aggregation,
+      allowIncompatibleAggregation: allowIncompatibleAggregation === true
     };
     var resp = await cdm.getMetricDataSets(instance, [set], yearDotMonth);
     if (resp['ret-code'] != 0) {
-      return res.status(500).json({
-        code: 'METRIC_QUERY_FAILED',
+      return res.status(resp.code === 'INCOMPATIBLE_AGGREGATION' ? 400 : 500).json({
+        code: resp.code || 'METRIC_QUERY_FAILED',
         error: resp['ret-msg']
       });
     }