cmcfarlen opened a new pull request, #13505: URL: https://github.com/apache/trafficserver/pull/13505
Adds two facilities to `ts::Metrics`, plus a `traffic_ctl` option to inspect the first, and fixes three pre-existing defects found along the way. A follow-up PR uses these for per-upstream-server connection metrics; this PR is independently useful and stands on its own. ### Hidden metrics A second `Storage` instance, reached through `Metrics::hidden_instance()`, for high-cardinality intermediate values that are worth recording but not worth publishing. `Gauge::createHiddenPtr` / `Counter::createHiddenPtr` return the same correctly typed pointer as `createPtr`, so a hidden metric is read and written with the ordinary typed mutators with no cast at the call site. A separate store rather than a per-metric "hidden" flag is deliberate: it makes hidden metrics *structurally* unreachable from the published store, so no consumer can expose one by forgetting to check a flag. Since that also makes them hard to debug, `traffic_ctl metric match --include-hidden` lists them. The rec type bit for this (`RECT_HIDDEN_METRIC = 0x40`) sits deliberately outside `RECT_ALL` (`0x3F`), so hidden metrics are returned only when explicitly asked for and never as a side effect of a broad query. ### Derived metric aggregation - `MAX` and `MIN` in addition to `SUM`. The accumulator is seeded from the first source rather than from zero, since a zero seed is only correct for `SUM` and would clamp `MIN` to `<= 0`. - `Derived::add_source()`, for aggregates whose sources are discovered while the process runs rather than known at startup. Repeatedly calling `derive()` for one derived name does not work for this: it appends a separate entry per call, all targeting the same metric, so each update overwrites the others with its own subset and the last writer silently wins. `add_source()` accumulates into a single entry, and re-registering an existing source is a no-op. ### Pre-existing defects fixed - **`Storage::create()` had no exhaustion check.** Filling the last blob let the following bookkeeping call `addBlob()` and write one past the end of `_blobs`. Verified by temporarily shrinking `MAX_BLOBS`: the current code segfaults, and the `debug_assert` in `addBlob()` does not catch it because it is off by one against the access it guards (`_blobs[++_cur_blob]`) — and being a `debug_assert`, it is compiled out of release builds entirely. Now a `release_assert` against `MAX_BLOBS - 1`, with `create()` refusing the final slot and returning the reserved `bad_id`. - **Unresolvable derived sources were not skipped.** A source given by name or id that does not resolve was still passed to `lookup()`, which masks the unresolved id down to the reserved `bad_id` slot, so the aggregate silently included that slot's value. Observable under `MAX`/`MIN`, where the `bad_id` value can become the winning one; under `SUM` it was hidden by `bad_id` holding zero. - **The metrics unit tests were order dependent.** They asserted absolute metric ids and iterator positions, which only hold when the case runs first against an otherwise empty store. Any other case that creates a published metric made them fail. This lands first so the fragility is never introduced. ### Testing - Unit tests grow from 487 assertions / 32 cases to 6208 / 36, and pass under `--order rand` across many seeds. Each fix was checked against the unfixed code first to confirm the new assertions actually discriminate. - `tests/gold_tests/jsonrpc/metric_match_include_hidden.test.py` covers the `--include-hidden` RPC round trip end to end. This is load-bearing: the rec type also has to be accepted by the JSONRPC request decoder, which validates each requested type against a whitelist and rejects the entire request otherwise. That was invisible to every build-level check and only showed up end to end. - Verified against a running `traffic_server` with two temporary hidden metrics registered: absent from `metric match`, present with `--include-hidden`, and absent from a broad `proxy.process` query of ~900 published metrics. - Docs build clean under `-W` with `nitpicky = True`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
