feat: introduce observed, observed_macros and observed_testing crates - #671
Conversation
|
| Crate | Baseline | Baseline commit | This PR | Minimum required | Status |
|---|---|---|---|---|---|
observed |
new crate | — | 0.24.0 | 0.24.0 | ✅ ok |
observed_macros |
new crate | — | 0.24.0 | — |
⚠️ observed_macros — manual proc-macro review detail
`observed_macros` is a proc-macro-only crate. cargo-semver-checks intentionally skips proc-macro targets because they have no supported library API surface. Review exported macro names, accepted input syntax, diagnostics, and generated output manually; build and test results do not establish public API SemVer compatibility.
Proc-macro API compatibility must be reviewed manually; successful builds and tests do not establish SemVer compatibility. This check is informational and does not block the merge.
fe2ec4d to
6f1df24
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #671 +/- ##
=========================================
Coverage 100.0% 100.0%
=========================================
Files 503 532 +29
Lines 57406 59660 +2254
=========================================
+ Hits 57406 59660 +2254
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Approved, feel free to ignore my agent. |
Move the `observed` telemetry crate family from the internal ox-sdk monorepo into this repository, so the crates live alongside the rest of the open-source Oxidizer surface and can be published to crates.io. `observed` provides structured, typed telemetry events with scoped enrichment, data-classification-driven redaction and per-field routing to logs and metrics. `observed_macros` supplies the `#[event]` attribute and `#[derive(Enrichment)]`; it is re-exported through `observed` and is not meant to be depended on directly. `observed_testing` is the internal (`publish = false`) harness and integration-test suite for both. Changes made while moving: * `observed` no longer depends on `opentelemetry`. `Value` is an owned enum, so an `opentelemetry` major bump is no longer a breaking change for consumers that only emit typed events. The crate keeps `opentelemetry` as a dev-dependency for its examples and benchmarks. * The `metric_number_of` helper moved into `observed_testing`, which was its only consumer in this repository. * `observed_testing` uses `testing_aids::FutureTestExt` and `testing_aids::poll_once` for manual future polling instead of the ox-sdk-only helper crate. * Tests and examples use `expect` with a message rather than `unwrap`, to satisfy this workspace's `clippy::unwrap_used` policy. * `observed_testing` is registered in `automation::INTERNAL_CRATES`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the domain vocabulary the moved crates use to `.spelling` (enrichments, fieldless, idents, integrators, outsized, pluggable, prefiltering, sendable, signedness, snapshotting, stackable, triages, UCUM, untargeted). `Pluggable` was already listed, but Hunspell does not match a capitalised entry against a lowercase occurrence, so the lowercase form is needed too; likewise `ident` does not cover `idents`. Also switch the British `signalling` to the US `signaling` in the reentrancy guard docs, matching the en_US dictionary the gate uses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The repository's per-package line-coverage gate defaults to 100%, and the first CI run put `observed` at 98.3%, `observed_macros` at 99.2% and `observed_testing` at 96.6%. Measuring locally with the repo's coverage recipes pointed at concrete gaps rather than whole untested areas: * `observed::Text` had no test for its `Hash` impl, which hashes contents so that the same text stored as a literal and as an `Arc` is interchangeable as a map key. * `observed::Value` never displayed an `F64`, a `BoolArray`, an `F64Array` or a `StringArray`, and was never built from a `Text`, a `Vec<Text>` or a `Vec<&'static str>`. * `observed_macros::field_attrs` never peeled a reference in `strip_reference`, and never walked a token `Group`, which is the only path on which `mentions_any_type_param` recurses. * `observed_testing::MockProcessor::with_flush_error` had no caller, so the failing arm of its `flush` was dead in this repository. Each new test fails against the code without the behaviour it pins. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The `static-analysis` job runs `cargo +nightly fmt --all` with `--config-path ./unstable-rustfmt.toml`, which is not the `rustfmt.toml` that `just anvil-fmt` uses. The two configs differ, and three files that `anvil-fmt` accepts were rejected by the workspace-wide check. Format them with the config the job actually uses; both configs are now clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`observed_testing` is an internal test harness (`publish = false`, listed in `automation::INTERNAL_CRATES`), not a library with an API contract of its own. Mutating its assertion helpers asks for tests that assert on the assertions, which is not a useful thing to own. This puts it alongside the crates already excluded for the same reason: `automation`, `testing_aids`, and the `rest_over_grpc` example and test crates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add minimal targeted unit tests covering the mutants reported by pr-mutants: enrichment entry/slot, interop conversions, metadata event/field/metric accessors, processing event state and view, sink core, text and value helpers, plus the observed_macros event and lib paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9a5253c to
f4a5e26
Compare
Five issues reported in review, each with a test that pins it: - sink: acquire the reentrancy guard after the event value is built, not before. Building an event runs ordinary user code, so a field initializer calling a helper that emits its own telemetry had that telemetry silently dropped - even when it targeted an unrelated sink. The guard exists for processor reentrancy, so it now covers dispatch only. - context: make the guard returned by `Transfer::apply_current_thread` `!Send`. Dropping it restores the *current* thread's slot heads, so moving it to another thread wiped that thread's live enrichment and stranded the origin thread's transferred entries. - value: `from_redacted` discarded a `RedactedDisplay` error and exported whatever prefix had already been written. A failed redaction now erases the value. The reentrant path no longer uses `to_string`, which panics on such an error. - macros: derive default telemetry keys through `Ident::unraw`, so a field written as `r#type` is exported as `type` rather than `r#type`, in both `#[event(...)]` and `#[derive(Enrichment)]`. - macros: fully qualify every path generated into `visit_fields` instead of importing them. The imports were in scope for the consumer's `#[data_class(...)]` expression, so a consumer type named `Value` was shadowed by `observed::Value` and valid input failed to compile. Expansion snapshots are regenerated for the qualified paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage gate reported `observed` at 99.9% against its 100% minimum, from a single line: the `scratch.clear()` branch taken when a nested `from_redacted` call - one whose thread-local scratch buffer is already borrowed by an outer call - has its redaction fail part-way. The previous test covered that branch only on the buffered path. This one drives both edge cases at once: an outer `RedactedDisplay` that re-enters `from_redacted` with an inner value that fails after writing a prefix, so the fallback allocation is used and must also be erased. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The mutation gate reported one surviving mutant on all three platforms: `replace <impl Drop for ThreadBoundGuard>::drop with ()`. It survives because the impl is genuinely unobservable - the wrapper holds the enrichment `Guard` as a field, so that guard's own `Drop` restores the slot heads whether or not the wrapper's `Drop` body runs. No test can distinguish the two, which makes this a dead guard rather than a missing test. The impl only existed because `clippy::empty_drop` rejects an empty body and the method's `-> impl Drop` return type demanded the trait. Holding the `Guard` directly and returning `impl Sized` removes both pressures. The `!Send` property this wrapper exists for is unchanged - it comes from the `PhantomData<Rc<()>>` field, not from the `Drop` impl - and was re-verified: moving the guard to another thread still fails to compile with `Rc<()> cannot be sent between threads safely`. The enrichment slot and context-propagation suites confirm the restore still happens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implements the decision on #discussion_r3784613672 and #discussion_r3784614604: widen `Value`, then tighten the macro, so an unredacted value keeps its own type instead of being widened, saturated or stringified. `Value` gains a `U64` variant plus `From` for the remaining integer widths (`i8`, `i16`, `u8`, `u16`, `u64`, `usize`, `isize`). `u64` gets its own variant rather than folding into `I64` because more than half its range does not fit there, and byte and request counters live exactly in that half. `Value` is `#[non_exhaustive]`, so the new variant is source-compatible for matchers outside the crate. `u128` and `i128` stay unsupported - no telemetry backend represents them. `#[event(...)]` now rejects, at compile time, the three shapes that used to be accepted and then record nothing: - a classified metric value field, which `from_redacted` renders as a string that carries no measurement; - a non-numeric `gauge`/`histogram` value, which was previously not type-checked at all; - `u128`/`i128`, with a diagnostic naming the width instead of the old `Value: From<u64> is not satisfied` from inside the expansion. `counter` still requires an unsigned integer and `updown_counter` a signed one; `gauge`/`histogram` accept any supported numeric, floats included. Two call sites in this repo were silently broken and are fixed: `otel_log_record.rs::MemoryUsage`, a metric-only event whose classified gauge emitted no measurement, and the `optional_fields` example, whose histogram targeted a classified non-numeric newtype. `metric_number_of` gains a `U64` arm - without it a `u64` counter would still record nothing - and the OTel example maps `U64` explicitly rather than through its debug-string fallback. Docs updated to match: the `#[event(...)]` attribute reference, DESIGN.md, FEATURES.md, the `Value` conversion table, and CHANGELOG.md. A stale TODO in metric_routing.rs promising classified metric values is removed, since that is now expressly rejected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two CI failures from the metric-value-contract change, both mine: - `clippy::unnecessary_trailing_comma` on the new assertion in metric_routing.rs. Plain `cargo clippy` does not enable this lint; `just anvil-clippy` does, which is the actual gate. It broke `pr-fast` on linux and linux-arm plus `static-analysis`. - Three words new to the `Value` docs that the Hunspell dictionary does not carry: `stringified`, `stringifying`, `matchers`. `.spelling` is compiled without affix flags, so `matcher` being present does not cover `matchers` - every inflection needs its own line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The coverage gate reported `observed_macros` at 99.9% against its 100%
minimum. The uncovered line was
`NumericKind::Float => unreachable!("gauge and histogram return early")`
in the diagnostic-wording match: `required` is assigned only
`UnsignedInt` or `SignedInt`, because gauge and histogram return before
it is computed, so that arm can never run and no test can reach it.
Pairing each instrument kind with its own wording in the single match on
`kind` removes the arm rather than papering over it, which also drops
the `unreachable!` - a panic that could never fire but still had to be
read as if it could.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removed comments explaining the separation of U64 from I64.
`RedactedDisplay::fmt` and `RedactedToString::to_redacted_string` both take `&dyn Redactor`, so requiring a concrete `RedactionEngine` was narrower than the redaction those functions actually perform. A processor can now drive redaction with any strategy without wrapping it in an engine first. Changed signatures: - `Value::from_redacted` - `EnrichmentEntry::redacted_value` - `FieldValueFn`, and so every field/enrichment getter closure Existing callers are unaffected: `RedactionEngine` implements `Redactor`, so `&engine` still coerces at the call site, and the generated getter closures are unannotated. No macro or snapshot change was needed. This also narrows the public API to `data_privacy_core`: the `allowed_external_types` entry `data_privacy::redaction_engine::RedactionEngine` is replaced by `data_privacy_core::redactor::Redactor`, matching what `templated_uri` already does. `data_privacy` remains a dependency solely for `Sensitive<T>`, which `data_class = <expr>` generates and which has no `data_privacy_core` equivalent; the unused `__private::RedactionEngine` re-export is dropped. Test `from_redacted_accepts_a_bare_redactor` pins the new capability - it passes a bare `SimpleRedactor`, which did not compile before this change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
martintmk
left a comment
There was a problem hiding this comment.
I played with the branch a bit and I think this is really nice.
Two nits that we can address in follow-up:
- Support for histogram boundaries. This is something that is encoded in defaults for each histogram and we should preserve this. See MetricsHandler for example.
- Have you thought about using
CompactStrinText? Could help with some small allocations. - I could see the following pattern being repeated when dimensions are shared for both logs and metrics:
#[dimension(logs = "server.url.port", metrics = "server.url.port")]
Maybe also allow this syntax?
#[dimension("server.url.port", metrics = true)] // logs included by default
Move the
observedtelemetry crate family from the internal ox-sdk monorepo into this repository, so the crates live alongside the rest of the open-source Oxidizer surface and can be published to crates.io. This is the publication step for the telemetry-enrichment work; until now every change had to be transferred between the two repositories by hand.The crates
observed(0.24.0) — structured, typed telemetry events. Each event is a struct with a data classification on every field. Enrichment entries are scoped, stackable and propagate across threads and async tasks. One event struct can produce a log and a metric with independent field subsets per signal.observed_macros(0.24.0) — the#[event(...)]attribute and#[derive(Enrichment)]. Re-exported throughobserved; do not depend on it directly.observed_testing(publish = false) — theMockProcessorharness and the integration-test suite for both crates.Changes made during the move
observedno longer depends onopentelemetry.Valueis an owned enum, so anopentelemetrymajor bump is no longer a breaking change for a crate that only emits typed events.opentelemetrystays as a dev-dependency, because the examples and benchmarks still export through an OpenTelemetry sink.metric_number_ofhelper moved intoobserved_testing, which is its only consumer in this repository.observed_testingdrives futures by hand withtesting_aids::FutureTestExtandtesting_aids::poll_onceinstead of the ox-sdk-only helper crate.expectwith a message instead ofunwrap, to satisfy this workspace'sclippy::unwrap_usedpolicy.#![doc(html_logo_url)]and#![cfg_attr(coverage_nightly, ...)]attributes are enabled, matching the convention of the other crates here.observed_testingis registered inautomation::INTERNAL_CRATES.all_the_timeandthread_localare added to[workspace.dependencies].Effects
opentelemetryand its transitive tree; a crate that does export must now convert values itself.unwrap_used, external-type approval, coverage and mutation testing — which are stricter than the ones the crates were developed under.Valuevariant set becomes public vocabulary rather than an implementation detail.#[non_exhaustive]keeps adding a variant source-compatible for matchers, but an exporter outside this repository needs a catch-all arm.Validation
just check,just anvil-clippy,just anvil-fmt,just test(6628 passed),just test-docs,just readme-check,just anvil-license-headers,just anvil-external-types,just anvil-doc-buildandcargo macheteall pass locally.cargo spellcheckcannot be installed on this machine and mutation testing has not been run locally; both are left to CI.