From c8e9bc39fc3b14f94ca9eda1c6eead036ab7a914 Mon Sep 17 00:00:00 2001 From: Torstein Tauno Svendsen Date: Mon, 7 Sep 2026 09:43:11 +0200 Subject: [PATCH] The answer's metric names identify the document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--using` was the only way to get a unit when the window held no `!meta`, which meant typing the name of a document whose metrics are already listed in the answer. So `facts_for` looks it up instead: for each metric, which locally installed documents declare it. Three sources, in order of authority — the document `using` NAMED, then a `!meta` (the producer's own word about these numbers), then the local documents. `using` becomes the override. ⚠ Inference degrades to NO unit, never a wrong one. Two documents declaring one metric in different units are both named and the unit is left blank, because the unit decides which axis a series lands on: guessing between `ms` and `s` would not mislabel a plot, it would regroup it. Live today — `timberfs-apache-combined` and `-nginx-combined` share `http_requests` and `http_bytes` at identical units, while `http_latency` is nginx's alone at `s` and apache logs `%{ms}T` in milliseconds. Two things it now says that were silent: a `using` naming a document that declares none of the answer's metrics, and a `using` that disagrees with a `!meta`. And the test harness catches per test, as the timbersh one does. Unguarded it aborted the whole run on an exception, so every test after the raising one never ran — which is the failure mode a suite exists to prevent, and it hid this test's own first draft. --- packaging/timbergraph.1 | 23 ++++++--- tests/timbergraph/test-timbergraph | 83 +++++++++++++++++++++++++++++- tools/README.md | 15 ++++-- tools/timbergraph.py | 72 ++++++++++++++++++++++++-- tools/timbersh | 14 +++-- 5 files changed, 185 insertions(+), 22 deletions(-) diff --git a/packaging/timbergraph.1 b/packaging/timbergraph.1 index 586440e..620e09d 100644 --- a/packaging/timbergraph.1 +++ b/packaging/timbergraph.1 @@ -53,11 +53,21 @@ or is refused too. .PP ⚠ What a line cannot say is the UNIT, and which metrics exist but were -silent in this window. A +silent in this window. Three things can supply the unit, in order of +authority: the document +.B \-\-using +NAMED, then a .B !meta -marker supplies the unit where the window happens to hold one; otherwise -name the extractor document with -.BR \-\-using . +marker if the window holds one (the producer's own word about these +numbers), then the documents installed HERE \(em the answer's metric +names identify the document, so a metric only one of them declares needs +nothing typed. ⚠ Where two of them declare it in DIFFERENT units, both +are named and the unit is left blank: the unit decides which axis a +series lands on, so guessing between +.B ms +and +.B s +would not mislabel the plot, it would regroup it. .PP .B THE MARKERS ARE THE ERROR BARS .PP @@ -130,8 +140,9 @@ falls in \(em which is what additive data supports, and the reason a stored percentile was never allowed. .TP .BI \-\-using " PATH|NAME" -An extractor document, for the unit and the title \(em a path, or a NAME -looked up in +An extractor document, for the unit and the title \(em the OVERRIDE, since +the metric names in the answer already identify the document where that +is unambiguous. A path, or a NAME looked up in .BR /usr/lib/timberfs/tally.extractors.d , .BR /etc/timberfs/tally.extractors.d , then diff --git a/tests/timbergraph/test-timbergraph b/tests/timbergraph/test-timbergraph index d4d2336..ae0111e 100755 --- a/tests/timbergraph/test-timbergraph +++ b/tests/timbergraph/test-timbergraph @@ -233,6 +233,80 @@ def test_an_extractor_is_a_path_or_a_name(): sh.rmtree(home, ignore_errors=True) +def test_the_documents_installed_here_identify_the_metric(): + """A tally line cannot carry its unit and the store's manifest + deliberately records no extractor names, so the unit came from a + `!meta` in the window or from a document the reader NAMED. But the + answer's own metric names identify the document, so `using` can be + the override rather than the normal path — and where two documents + disagree, the answer is no unit rather than a guess, because the unit + decides which axis a series lands on.""" + import tempfile, shutil, json as j + d = tempfile.mkdtemp(prefix="timbergraph-test-") + + def doc(path, name, metrics): + with open(os.path.join(d, path), "w", encoding="utf-8") as fh: + j.dump({"v": "1.0-EXPERIMENTAL", "name": name, + "window": {"axis": "logline", "width_ms": 60000}, + "metrics": metrics}, fh) + + doc("one.json", "one", [ + {"name": "only_mine", "measure": [{"count": True, "unit": "widgets"}]}, + {"name": "shared_agree", "measure": [{"count": True, "unit": "B"}]}, + {"name": "shared_differ", "measure": [{"sum": "t", "unit": "ms"}]}]) + doc("two.json", "two", [ + {"name": "shared_agree", "measure": [{"count": True, "unit": "B"}]}, + {"name": "shared_differ", "measure": [{"sum": "t", "unit": "s"}]}]) + + was = g.EXTRACTOR_DIRS + g.EXTRACTOR_DIRS = (d,) + try: + # One document declares it: nothing has to be typed. + f, notes = g.facts_for(["only_mine"], []) + check("inferred", "widgets", f["only_mine"]["unit"]) + check("and said which", "one", f["only_mine"]["extractor"]) + check("silently", [], notes) + + # Several agree, so which one it was does not matter. + f, notes = g.facts_for(["shared_agree"], []) + check("agreeing", "B", f["shared_agree"]["unit"]) + check("also silently", [], notes) + + # ⚠ They disagree: no unit, and BOTH named. Guessing between ms + # and s would not mislabel the plot, it would regroup it. + f, notes = g.facts_for(["shared_differ"], []) + check("no guess", None, f["shared_differ"].get("unit")) + check("one note", 1, len(notes)) + for want in ("one=ms", "two=s", "using"): + if want not in notes[0]: + FAILED.append(f"the ambiguity note omits {want!r}: {notes[0]!r}") + + # A `!meta` is the producer's own word about these numbers and + # outranks the inference. + _, markers = g.read("2026-09-06T10:00:00.000Z 0s !meta " + "metric=only_mine unit=furlongs\n") + f, _ = g.facts_for(["only_mine"], markers) + check("the tape wins over a guess", "furlongs", f["only_mine"]["unit"]) + + # An explicit `using` is an instruction and outranks both — but a + # disagreement with the tape is reported rather than silent. + f, notes = g.facts_for(["only_mine"], markers, ["one"]) + check("using wins", "widgets", f["only_mine"]["unit"]) + check("and says so", 1, len(notes)) + if "furlongs" not in notes[0]: + FAILED.append(f"the conflict note omits the tape's unit: {notes[0]!r}") + + # Naming a document that declares none of them is worth saying: + # it is silently accepted otherwise. + f, notes = g.facts_for(["only_mine"], [], ["two"]) + check("still inferred", "widgets", f["only_mine"]["unit"]) + if not notes or "declares none" not in notes[0]: + FAILED.append(f"a `using` that matches nothing went unreported: {notes!r}") + finally: + g.EXTRACTOR_DIRS = was + shutil.rmtree(d, ignore_errors=True) + + def test_a_name_that_resolves_nowhere_says_where_it_looked(): """A directory is listed only if it exists, so where none does the list is empty — and the failure then named nowhere at all, which says @@ -267,7 +341,14 @@ def main(): tests = [(n, f) for n, f in sorted(globals().items()) if n.startswith("test_") and only in n] for name, fn in tests: - fn() + # ⚠ Caught per test, as the timbersh suite does. Unguarded, one + # test raising aborted the whole run and every test after it + # silently never ran — which is the failure mode a suite exists + # to prevent. + try: + fn() + except Exception as e: # noqa: BLE001 + FAILED.append(f"{name}: {type(e).__name__}: {e}") if FAILED: for f in FAILED: print(f"FAIL {f}", file=sys.stderr) diff --git a/tools/README.md b/tools/README.md index c9b11ab..82e0797 100644 --- a/tools/README.md +++ b/tools/README.md @@ -692,9 +692,18 @@ multiplies the count, so drawing one without `--quantile` or `--by le` is refused too. What a line cannot say is the **unit**, and which metrics exist but were -silent in the window. A `!meta` marker supplies the unit where the window -happens to hold one; otherwise name the extractor document with `--using`, -which takes a path **or a name**: +silent in the window. Three things can supply the unit, in order of authority: +the document `--using` **named**, then a `!meta` marker if the window holds one +(the producer's own word about these numbers), then the documents installed +**here** — the answer's own metric names identify the document, so a metric +only one of them declares needs nothing typed. + +⚠ Where two documents declare one metric in **different** units, both are named +and the unit is left blank rather than guessed. The unit decides which axis a +series lands on, so picking between `ms` and `s` would not mislabel the plot, it +would regroup it. + +So `--using` is the **override**, and it takes a path **or a name**: ```sh mkdir -p ~/.config/timberfs/tally.extractors.d diff --git a/tools/timbergraph.py b/tools/timbergraph.py index d163e4b..5fb1c46 100644 --- a/tools/timbergraph.py +++ b/tools/timbergraph.py @@ -601,6 +601,71 @@ def extractor_facts(paths): return facts +def declaring(metric, dirs=None): + """Which local documents declare `metric`, as `[(name, unit)]`. + + The answer's own metric names identify the document, which is why + `using` can be an override rather than the normal path: `http_latency` + is declared by one shipped document, so nothing has to be typed for a + plot of it to know its unit. + """ + out = [] + for doc in declared(dirs).values(): + for m in doc.get("metrics", []): + if m.get("name") != metric: + continue + unit = (m.get("histogram") or {}).get("unit") + for measure in m.get("measure", []): + unit = measure.get("unit", unit) + out.append((doc.get("name"), unit)) + return out + + +def facts_for(metrics, markers, using=(), dirs=None): + """The unit and description per metric, from the three sources that + can supply them, plus what a reader should be told. + + In order of authority: the documents `using` NAMED (an instruction), + then a `!meta` in the window (the producer's own word about these + numbers), then inference from the documents installed here (a guess, + and taken only where every document that declares the metric agrees). + + ⚠ Inference degrades to NO unit, never to a wrong one. Two documents + declaring one metric in different units is reported and left blank — + the unit decides which axis a series lands on, so guessing between + `ms` and `s` would not mislabel a plot, it would regroup it. + """ + facts, notes = extractor_facts(using), [] + tape = tape_units(markers) + named = {m for m in facts} + if using and not (named & set(metrics)): + notes.append( + f"⚠ {', '.join(using)} declares none of {', '.join(sorted(metrics))} " + f"— units come from elsewhere or nowhere") + for m in metrics: + f = facts.setdefault(m, {}) + if f.get("unit") and tape.get(m) and f["unit"] != tape[m]: + notes.append( + f"⚠ {m}: the document you named says {f['unit']}, the tape says " + f"{tape[m]} — using the document") + if f.get("unit"): + continue + if tape.get(m): + f["unit"] = tape[m] + continue + owners = declaring(m, dirs) + units = {u for _, u in owners if u} + if len(units) == 1: + f["unit"] = units.pop() + f.setdefault("extractor", owners[0][0]) + elif len(units) > 1: + notes.append( + f"⚠ {m}: declared here by " + + ", ".join(f"{n}={u}" for n, u in sorted(owners) if u) + + " — name one with `using` for an axis label") + return facts, notes + + #: Tic intervals worth landing on, in seconds. A reader looks for #: :00, :15, :30 — never :07. TICS = (60, 120, 300, 600, 900, 1800, 3600, 7200, 10800, 21600, 43200, @@ -893,12 +958,11 @@ def main(argv=None): k, _, v = kv.partition("=") where[k] = v - facts = extractor_facts(args.using) if args.against: metrics = metrics + [args.against] - for m in metrics: - facts.setdefault(m, {}) - facts[m].setdefault("unit", tape_units(markers).get(m)) + facts, notes = facts_for(metrics, markers, args.using) + for note in notes: + print(note, file=sys.stderr) if args.against: if len(metrics) != 2: diff --git a/tools/timbersh b/tools/timbersh index 23892bf..a7c6358 100755 --- a/tools/timbersh +++ b/tools/timbersh @@ -1787,16 +1787,14 @@ class Shell: "no tally lines came back. `graph` reads a TALLY store — " f"try `select stores from [class=tally]` to see which there are") - facts = timbergraph.extractor_facts([opts["using"]]) if "using" in opts else {} - units = timbergraph.tape_units(markers) - for m in metrics: - facts.setdefault(m, {}) - facts[m].setdefault("unit", units.get(m)) - scatter_x = opts.get("against") + facts, notes = timbergraph.facts_for( + metrics + ([scatter_x] if scatter_x else []), + markers, + [opts["using"]] if "using" in opts else []) + for note in notes: + print(f" {note}") if scatter_x: - facts.setdefault(scatter_x, {}) - facts[scatter_x].setdefault("unit", units.get(scatter_x)) lines, xlabel, ylabel, dropped = timbergraph.against( samples, metrics[0], scatter_x, tuple(by), None, None, opts.get("rate", False), opts.get("quantile"), facts)