[HarryMoss]

Summary

Adds optional trace.uid correlation to OCSF audit records emitted within a valid, sampled OpenTelemetry span, so operators can find the corresponding trace. Dedicated supervisors with OTLP tracing enabled emit INFO deny spans at the default log level, and companion audit records share one decision scope, including unsupported CONNECT required-middleware refusals.

This remains a draft pending the validation and maintainer decisions below.

Related Issue

Closes #2640

Changes

• Adds explicit correlation setters and emission-time extraction across all ten builders and direct/gateway emission. Explicit values take precedence; openshell-ocsf has no OpenTelemetry dependency.
• Uses the current native OCSF 1.8 output and exact profile-combination schema catalog. HTTP/API Trace exports are native; eight other classes use the documented OpenShell applicability extension, with reproducible composition and unchanged untraced schemas. Strict consumers may reject the extension; maintainer disposition remains pending.
• Preserves current Process/SSH builder requirements, destination authorization and OTLP relay behavior. The earlier 1.1/1.3 projection paths were removed upstream and are not restored.
• Groups companion findings and domain records under one deny scope, including the unsupported CONNECT required-middleware path. Preserves available HTTP policy context and integer ocsf.class_uid attributes; ordinary transport failures and allowed/audit/fail-open controls do not create enforced-denial spans.
• Documents trace lookup, caller-propagated gateway context, export/retention limits and the shared-supervisor export limitation.

Submitted revision

Head: fed4f94257a49d44b5380678ecd32c16b1bf64b3. Verified source tree: 4170b2aca8db928bdf5319a52e125a847ae6d42d. Integrated upstream snapshot: 42671c2dd2c02b744850e0bda72b99d5509969c1. The submitted tree exactly matches the locally validated review candidate. Existing commits and review history are preserved; the original 3fae097a branch remains unchanged.

Testing

macOS arm64, Rust 1.95.0, locked dependencies and repository default profiles. All results below apply to source tree 4170b2aca8db928bdf5319a52e125a847ae6d42d; server validation and Clippy completed on 2 November 2026.

Area	Passed	Ignored
OCSF / validator / OTel, including doctests	272	2
Supervisor-network library	1,572	2
Supervisor library	264	0
Gateway/server, including integration targets	2,636	38

• All three focused unsupported CONNECT correlation regressions passed; these are included in the network library total.
• All-target Clippy across the six affected crates passed with -D warnings.
• Rust formatting, Markdown lint, docs navigation and Fern passed. Fern reported zero errors and three existing warnings.
• Seven schema-generator safeguards, pinned Python lint/formatting, Bash syntax and catalog constraint-preservation checks passed. All 18 original untraced schemas remain byte-identical; 18 Trace variants cover all ten classes.

The earlier space-limited server attempt was resumed using the same source, toolchain and configuration. No implementation change was needed for the completed validation.

Fresh Linux compilation/tests and deployed default-level deny-to-Collector/Tempo validation with allowed/schema controls remain pending for this revision. Local loopback exporter tests do not establish a deployed pass. Kevin's independent review and rhuss's earlier deployed check apply only to 3fae097a5d4355eeff77746f3ea90e89228455e3.

Shared supervisor hosts currently initialize no OTLP exporters and provide no automatic sandbox trace correlation. NVIDIA CI approval/completion, maintainer disposition of the Trace applicability/explicit setter API and reviewer reassessment remain pending.

• Scoped local Rust, lint and docs checks pass
• Relevant unit and integration coverage added/updated
• Fresh Linux/deployed E2E validation completed for this revision

Checklist

• Follows Conventional Commits
• Commits contain DCO sign-offs
• Operator documentation updated
• Maintainer CI approval and required checks completed
• Reviewer reassessment completed

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[HarryMoss]

I have read the DCO document and I hereby sign the DCO.

[HarryMoss]

recheck

[HarryMoss]

The README conflict is resolved, and the updated branch passes the local checks. Could a maintainer please vet and approve the CI workflows? Fresh Linux/Collector/Tempo validation remains pending.

[rhuss]

cc-review Summary

@HarryMoss, please have your agent assess each of these comments and check it against the code before acting on it. Some may turn out not to apply, and I would rather you confirm or refute them than take them at face value.

This review comes from an agent-assisted run with several reviewers (cc-review: seven prompt reviewers plus Codex and CodeRabbit, then a skeptic/calibration pass). The findings and the scenarios in them come from the agents reading the code. Nothing was executed or reproduced, and I have not verified each scenario myself.

What Went Well

• crates/openshell-otel/src/correlation.rs current_ocsf_trace_correlation only correlates when the span context is both valid and sampled.
• crates/openshell-ocsf/src/trace.rs keeps openshell-ocsf free of OpenTelemetry through a OnceLock extractor, and enrich never overrides an explicit trace.
• crates/openshell-ocsf/src/events/base_event.rs set_trace adds the trace profile only once.
• crates/openshell-ocsf/src/format/downgrade.rs strips both the trace field and the profile for 1.1 and 1.3, with tests for each target.
• crates/openshell-supervisor-network/src/telemetry/tests.rs exported checks real OTLP/gRPC exports instead of only the JSON shape.

Findings

Severity	File	Description	Source
Important	crates/openshell-supervisor-network/src/telemetry.rs:32	Paired deny event and finding get different trace IDs	gator
Minor	crates/openshell-server/src/cli.rs:669	Gateway trace.uid is caller-propagated but documented as trusted	security
Minor	crates/openshell-supervisor-network/src/l7/middleware.rs:466	Capacity-exhausted finding marked deny when allowed	correctness
Minor	crates/openshell-supervisor-network/src/l7/middleware.rs:487	Load shedding traced as policy deny	goal-alignment
Minor	crates/openshell-supervisor-network/src/l7/middleware.rs:1004	denied: predicates untested	test-quality
Minor	crates/openshell-supervisor-network/src/proxy.rs:1261	Transparent middleware deny not parented on connect span (see below)	correctness
Minor	crates/openshell-supervisor-network/src/proxy.rs:5677	Non-canonical rejection gets no deny span	codex
Minor	crates/openshell-supervisor-network/src/proxy.rs:5956	CONNECT/FORWARD deny-span wiring untested	test-quality
Minor	crates/openshell-supervisor-network/src/proxy.rs:6072	Audit-mode L7 failure traced as enforced deny	codex
Minor	crates/openshell-supervisor-network/src/proxy.rs:6660	parent: is a no-op for non-deny events	architecture
Minor	crates/openshell-supervisor-network/src/proxy/trace_tests.rs:80	Debug deny test lacks count and uid checks	test-quality
Minor	crates/openshell-supervisor-network/src/telemetry.rs:11	Local ocsf_emit! shadows the upstream macro	architecture
Minor	crates/openshell-supervisor-network/src/telemetry.rs:41	Deny classification split between event and flag	architecture
Minor	crates/openshell-supervisor-network/src/telemetry.rs:55	Deny-span policy/host attributes never asserted	test-quality
Minor	crates/openshell-supervisor-network/src/telemetry.rs:64	One exported root span per deny, no rate gate	production
Minor	crates/openshell-ocsf/src/builders/network.rs:121	.trace() setter on 10 builders used only by tests	architecture
Minor	crates/openshell-otel/src/lib.rs:49	Supervisor filter moved into shared public API	architecture
Minor	docs/observability/ocsf-json-export.mdx:281	Docs mention the internal builder API	architecture

Outside the diff: crates/openshell-supervisor-network/src/proxy.rs:1261. The transparent-TCP fail-closed middleware deny calls emit_middleware_uninspectable without the live connect_span, unlike the sibling transparent denies, so at debug level this deny is not linked to the connect/authorize trace even though the docs say it should be. An optional parent parameter, or wrapping the call in connect_span.in_scope(...), would fix it.

Observation: the supervisor.egress.deny span and its openshell.policy.* keys live in telemetry.rs, separate from the supervisor.egress.* spans in proxy/egress.rs, and the attribute values come from string-keyed evidence lookups. Shared constants would keep the two in sync.

Review Details

• Findings posted: 18
• Findings reviewed and not posted: 0
• Gate outcome: FAIL (one Important finding)
• Participating reviewers: architecture, correctness, goal-alignment, production, security, test-quality, gator, codex (CodeRabbit ran with no findings)

[rhuss]

Minor: All ten builders gain a public .trace(TraceCorrelation) setter with typestate plumbing, but only tests call it; production correlation goes through the registered extractor.

Why this matters: That is about 130 near-identical lines of public API, and TraceCorrelation.uid is a public String whose 32-hex invariant is only documented. The PR description lists the setters as intended, so treat this as a design question.

Suggested fix: Either let tests use BaseEventData::set_trace and drop the setters, or keep one validating constructor with a private field.

Source: architecture agent

[HarryMoss]

Thanks for the review, @rhuss. The follow-up at 910509993e18659404e923a2aa33d772a4ae5655, following the upstream integration, groups companion deny records and terminal response failures under one decision span, corrects caller-propagated correlation wording, and keeps ordinary I/O failures out of enforced-denial spans. Forward evaluation-error spans retain the HTTP policy context, and ocsf.class_uid now exports as an integer.

The final source tree is e231ce833d5999a46d664121858cac22cdc526aa. Its network library suite passed 1,561 tests (2 ignored); five-crate all-target Clippy and formatting passed. OCSF/OTel and docs evidence is separately attributed to unchanged source in the updated PR description.

The docs now explicitly identify Trace applicability on the eight extra classes as an OpenShell extension beyond official OCSF 1.8 HTTP/API applicability. Please confirm whether that extension is acceptable. Fresh Linux and default-level Collector/Tempo validation with allowed/schema controls, NVIDIA CI approval/completion and reviewer reassessment remain pending. This remains Draft; earlier independent deployment results apply only to 3fae097a.

[rhuss]

@krishicks @TaylorMutch wdyt? Could we try to get this PR in? All findings from my review are addressed (I checked each against 9105099), and the branch is current with main, including #5019. What remains needs a maintainer call:

1. Trace profile beyond HTTP/API. OCSF 1.8 lists the Trace profile only for HTTP/API activity. The PR also applies it to eight other classes as a documented OpenShell extension, which strict OCSF consumers may reject. OK as is, or limit it to HTTP/API for now?
2. Public .trace() setters on all ten builders. Production code uses the registered extractor, and only tests call the setters. Keep them or drop them?
3. Schema catalog growth. Following the per-combination layout from feat(ocsf): validate OCSF events against exported JSON Schemas #5019, the PR adds a +trace variant of each of the 18 vendored schemas (~146k generated lines). Fine for now, or should profile variants be generated rather than vendored?

CI also still needs /ok to test.

@HarryMoss is there anything left before moving the PR out of draft? CI would cover the pending Linux build and tests once it is approved. Could you also resolve the threads I confirmed as fixed? That would make the PR easier for maintainers to scan. I'm happy to rerun the Podman + Collector/Tempo deny check on fed4f94 if that helps.

[HarryMoss]

Thanks @rhuss. Yes please, a fresh Podman + Collector/Tempo check on fed4f94257a49d44b5380678ecd32c16b1bf64b3 would be very helpful. The remaining runtime check is default-level deny correlation, including companion findings/domain records sharing the same trace, with allowed-traffic and schema controls. Please record the tested revision and configuration with the results.

Local macOS validation on that exact source tree is complete, including the affected Rust suites, six-crate all-target Clippy, schema checks and docs checks. Fresh Linux CI and the deployed check are still pending.

I have resolved the 16 review threads you confirmed as addressed and left the public .trace() setter discussion open for the maintainer decision.

@krishicks @TaylorMutch, could you please vet/approve CI and advise on the three choices above: the Trace applicability extension beyond HTTP/API, keeping or dropping the public setters, and vendored versus generated profile schemas? I can make the corresponding changes once the intended direction is clear.

I will keep this as a draft pending those decisions and the outstanding validation. Thanks for helping move this forward.

[krishicks]

Written by Claude Code, reviewed by @krishicks

Thanks @HarryMoss for the thorough work, and @rhuss for driving the review. Here are maintainer decisions on the three open questions, plus a few more items. Severity follows rhuss's scale. References are to fed4f94.

Decisions

Trace profile applicability: emit trace only where OCSF 1.8.0 declares it (HTTP Activity and API Activity). Schema conformance comes first for our audit output. A strict consumer rejects the whole record, not just the optional field. The 16 composed +trace schemas are not OCSF exports. schema.ocsf.io ignores profiles=...,trace for network_activity, detection_finding, and the other six classes. For example, the network_activity export with and without trace is byte-identical. trace_schema.py builds those files by grafting http_activity's definition onto each class. That makes the validator check our extension rather than OCSF conformance.

Public .trace() setters: drop them. No production code calls them. #2640 already says the agent's own trace context goes in a separate field, never in trace, so no override use case is coming.

Schema catalog: vendor only what update.sh fetches from the OCSF API. Once applicability matches OCSF, only the two native +trace files remain. Both come straight from the API with no composition step.

Findings

Severity	Item
Important	Restrict trace to HTTP/API Activity
Important	Audit-record changes need calling out
Minor	Make TraceCorrelation validated, drop builder setters
Minor	Set the OTLP log record's trace context
Minor	Derive deny classification from the event
Minor	Guard against the upstream ocsf_emit! in supervisor-network
Minor	Bound deny-span volume
Nit	deny_parent: on a site that never denies
Nit	Test harness cost
Nit	Commit structure

Important: restrict trace to HTTP/API Activity.

• Gate enrichment on the event class in enrich (crates/openshell-ocsf/src/trace.rs:32) and in BaseEventData::set_trace.
• Remove trace_schema.py, the 16 composed schemas, and the update.sh changes.
• Reduce the docs' extension section to one sentence: correlation applies to HTTP and API Activity.
• Knock-on effects in the supervisor:
  • Network Activity and Detection Finding records can no longer carry trace.uid.
  • A supervisor.egress.deny span whose primary record is one of those classes exports a span that no OCSF record points to.
  • Please open deny spans only for decisions with an HTTP Activity record. Drop the wiring that exists only for 4001 or 2004 sites.
• The feat(ocsf): add trace_id/span_id correlation fields to OCSF event builders #2640 acceptance criterion for network deny events needs amending to match. Network Activity correlation can return once OCSF declares the profile there.

Important: changes to audit records need calling out. Besides adding correlation, several existing records change meaning:

• These gain action=Denied and disposition=Blocked:
  • FORWARD parse errors (proxy.rs:2191);
  • non-canonical, GraphQL and JSON-RPC rejections (build_forward_rejection_event, proxy.rs:2135);
  • metadata-server 403/405 responses (google_cloud_metadata.rs:344).
• L7 evaluation errors get a new action and message (proxy.rs:2151).

These look like correct fixes. Those were denials that lacked an action. But they shift SIEM deny counts. Please put them in their own commit and mention them in the PR description and docs.

Minor: make TraceCorrelation validated and drop the builder setters. TraceCorrelation { pub uid: String } (trace.rs:17) accepts any string in a field documented as 32 lowercase hex. Make the field private behind a validating constructor, or build it from [u8; 16]. Then remove .trace() from the ten builders. Tests can drive the registered extractor with a real span, as ocsf_correlation.rs already does.

Minor: set the OTLP log record's trace context. log_record (crates/openshell-otel/src/ocsf.rs:374) puts trace only in the body. Log-to-trace links in OTLP backends use the record's own trace/span fields. These are OTLP fields, not OCSF attributes, so filling them from the active sampled span at emit time adds no conformance issue. It also brings correlation back for Network Activity and findings on the OTLP export path.

Minor: derive deny classification from the event. The decision still comes from three places:

• the event's action;
• the denied: macro arm (telemetry.rs:11);
• the denied argument to emit_decision (telemetry.rs:54).

is_denied (telemetry.rs:34) already reads DetectionFinding.action. Setting ActionId::Denied on findings that describe a refusal lets the flags go. It also records the refusal in the audit record itself, not only in the trace.

Minor: guard against the upstream ocsf_emit! in supervisor-network. A deny emitted with openshell_ocsf::ocsf_emit! instead of egress_emit! silently gets no deny span. opa.rs:148 and policy_dns/runtime.rs:17 still use the upstream macro. A disallowed-macros entry for openshell_ocsf::ocsf_emit in the crate's clippy config would catch new cases.

Minor: bound deny-span volume. Every denied decision exports a root span, and the workload controls how often it is denied. A tight retry loop against a blocked host can fill the batch exporter queue and cause the supervisor's other spans (startup, policy) to be dropped. Please add a per-destination rate limit on deny-span creation. OCSF records stay unaffected.

Nit: deny_parent: on a site that never denies. At proxy.rs:6853 (FORWARD upstream connect failure), the event never carries ActionId::Denied, so the arm does nothing. Use egress_emit!(event).

Nit: test harness cost. Each trace test starts a loopback OTLP gRPC server behind a global lock. Keep one or two end-to-end export tests and use an in-memory exporter for the rest.

Nit: commit structure. This would review and bisect better as separate commits:

1. ocsf/otel correlation plumbing and gateway registration;
2. supervisor deny spans;
3. the audit-record corrections above.