Persist custom and OOTB log fields in state

[snowp]

Persists custom and OOTB SDK fields in typed state namespaces and resolves them as virtual fields with OOTB > captured log > custom precedence. State admission is authoritative: accepted writes remain live whether journal-backed or bounded in memory; a capacity-rejected runtime update preserves the existing state and metadata, while a rejected startup field is removed from both.

While fields are dual-written, normal and previous-process logs retain their existing inline field maps. Previous-process replay uses its captured state snapshot only for matcher and workflow evaluation, preventing current-process values from leaking into that path. Ring-buffer field elision remains a separately gated rollout.

Handling bd tail, field association with crash reports, etc. are not handled as part of this PR.

[Copilot]

🟡 Changes recommended

Persistence failures alter live logging behavior, and several typed-state workflow and matcher paths are incorrect or unreachable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds persistent, typed state namespaces for custom and SDK-owned log fields, based on the schema introduced by API PR #204.

Changes:

• Persists typed custom and OOTB fields and exposes them to matchers/workflows.
• Extends state storage, recovery, and scope handling.
• Removes the synthetic session ID from state snapshot artifacts.

File summaries

File	Description
fuzz/src/versioned_kv_journal.rs	Updates state payload imports.
bd-workflows/src/workflow.rs	Centralizes state-value conversion.
bd-workflows/src/metrics.rs	Supports new scopes in metric extraction.
bd-workflows/src/engine_test_helpers.rs	Maps new scopes in tests.
bd-workflows/src/config.rs	Parses new scopes and typed values.
bd-state/src/lib_test.rs	Tests field-scope persistence.
bd-resilient-kv/src/versioned_kv_journal/store.rs	Stores new scoped maps.
bd-resilient-kv/src/versioned_kv_journal/recovery.rs	Updates payload import.
bd-resilient-kv/src/versioned_kv_journal/mod.rs	Updates state value types.
bd-resilient-kv/src/versioned_kv_journal/framing_test.rs	Updates test imports.
bd-resilient-kv/src/tests/versioned_kv_store_test.rs	Updates test imports.
bd-resilient-kv/src/tests/mod.rs	Updates test value helpers.
bd-resilient-kv/src/scope.rs	Adds custom and OOTB scopes.
bd-resilient-kv/src/lib.rs	Re-exports updated payload types.
bd-proto/src/protos/state/with_source/state_payload.rs	Generated schema binding update.
bd-proto/src/protos/state/with_source/scope.rs	Generated scope binding update.
bd-proto/src/protos/state/state_payload.rs	Generated schema binding update.
bd-proto/src/protos/state/scope.rs	Generated scope binding update.
bd-proto/src/protos/state/mod.rs	Updates generated-module payload aliasing.
bd-logger/src/state_upload.rs	Removes snapshot artifact session ID.
bd-logger/src/metadata.rs	Exposes initial persistent fields.
bd-logger/src/async_log_buffer.rs	Persists field updates in state.
bd-logger/src/async_log_buffer_test.rs	Tests typed persistence conversion.
bd-log-matcher/src/matcher.rs	Reads typed fields from state.
bd-log-matcher/src/matcher_test.rs	Tests typed state matching.
bd-api/src/api.rs	Maps API scopes to state scopes.

Review details

Suppressed comments (3)

bd-logger/src/async_log_buffer.rs:911

• A journal write failure now prevents the custom field from being applied to subsequent logs. This changes the existing inline behavior during disk-full/capacity/I/O errors even though persistence is ancillary; warn about the persistence failure, but still update metadata_collector.

                  {
                    log::warn!("failed to persist custom log field ({key:?}): {e}");
                    continue;

bd-logger/src/async_log_buffer.rs:925

• An OOTB field update is dropped from live log metadata whenever persistence fails. Preserve the prior best-effort inline behavior by logging the journal error and continuing with update_ootb_field.

                  {
                    log::warn!("failed to persist OOTB log field ({key:?}): {e}");
                    continue;

bd-logger/src/async_log_buffer.rs:950

• A persistence error now suppresses removal from the in-memory metadata collector, so the field remains attached to logs despite remove_log_field. Log the persistence failure but still perform the live removal.

                  if let Err(e) = state_store.remove(Scope::CustomFields, &field_name).await {
                    log::warn!("failed to remove custom log field ({field_name:?}): {e}");
                    continue;
                  }

• Files reviewed: 27/27 changed files
• Comments generated: 5
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Pre-initialization transitions can be lost, unsupported values can match as empty strings, and persistence failures can leave matching state inconsistent with inline fields.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

bd-logger/src/async_log_buffer.rs:988

• The metadata value is updated before this write, so a failed update leaves an older OOTB state value authoritative. field_value_with_state checks that persisted value before the concrete log fields, causing subsequent logs to carry the new inline value while workflows match and extract the old one. This error path needs an in-memory override or rollback so both views agree.

                    Err(e) => {
                      log::warn!("failed to persist OOTB log field ({key:?}): {e}");
                      continue;

bd-logger/src/async_log_buffer.rs:1032

• The custom field is removed from metadata before the journal deletion. If deletion fails, the stale custom state becomes the virtual fallback, so workflows continue matching and extracting a field that is no longer emitted inline. Preserve a tombstone/override or roll back the metadata removal when persistence fails.

                    Err(e) => {
                      log::warn!("failed to remove custom log field ({field_name:?}): {e}");
                    },

• Files reviewed: 32/32 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

State-backed matching has JSON and presence regressions, and server updates can mutate SDK-owned namespaces.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 32/32 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Matcher regressions and stale custom-state handling can produce incorrect workflow and matching results.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

bd-log-matcher/src/matcher.rs:612

• IsSetValue uses this string-conversion path, so a present typed state value such as binary, map, or array now reports as unset because state_value_as_cow correctly returns None for non-stringable data. State is_set must test key/value presence independently from conversion; the existing test establishes that even an empty value is considered set.

      Self::State(scope, flag_key) => state
        .get(*scope, flag_key)
        .and_then(|value| ResolvedFieldValue::State(value).as_cow()),

bd-log-matcher/src/matcher.rs:609

• This bypasses FieldsRef::field_value, changing existing string matcher behavior when a captured field has an unsupported string type but a same-key matching-only field is stringable: the old code fell through to the matching-only value, while this now selects the captured value and returns None. Use field_value_with_state here after making that helper preserve FieldsRef::field_value semantics.

      Self::Field(field_key) => resolved_field_value_with_state(fields, state, field_key)
        .and_then(ResolvedFieldValue::as_cow),

• Files reviewed: 33/33 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Scalar metric state extraction still emits unsupported typed values as empty tags, conflicting with the corrected multi-tag behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 32/32 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Virtual-field fallback can bypass field transformations, and batch clears still mishandle journal-capacity failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

bd-logger/src/test/stats_integration.rs:968

• These assertions still rely on wall-clock polling and can time out under a slow or contended test runner. Since the transition under test is completion of the async stats flush/rotation, wait on an explicit completion notification or lifecycle gate from that worker rather than repeatedly reading the index file.

• Files reviewed: 33/33 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

State overlays can bypass filter transformations and can override captured OOTB values during previous-process replay.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 35/35 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The builder deep-clones and retains the complete previous-state snapshot, potentially duplicating up to 1 GiB of state.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 36/36 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The cross-cutting persistence, wire-format, and previous-process replay changes warrant final human validation.

Review details

• Files reviewed: 38/38 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mattklein123]

Isn't this a problem in the future when we don't dual write? We have to be able to redact/transform what winds up in state also. Maybe this PR is too bug but is there any reason to not just do this now?

IIRC even today we do transforms before matching so how is this any different?

[snowp]

Yeah part of this is that it was getting too complex since you end up having to create a new map that holds all the re-written fields from state that needs to be plumbed through in addition to the state reader which holds the current feature flags.

This is all a bit messy since if we want to rewrite state in a way that is reflected in timeline the state transformation needs to be consistently applied in the snapshot as well, but the current Filter logic allows different transformations for different logs. I figured for now we'd keep this simple and figure out how we want this to work for state separately. I can update the comment to be clearer on the path forward, it's something I need to figure out

[mattklein123]

Unrelated to this PR but we should be more consistent about log::warn in the SDK. in the field it's just compiled code taking up space that no one sees. Not sure the best way to handle this as it is useful in local dev.

[snowp]

Yeah I agree, it's probably worth a wide discussion. One thing that I have considered is having our own logging facade so that we can do log levels in dev and keep user-visible logs separate. That way we can continue to use this in dev without polluting the release binary

[mattklein123]

This is an AI instruction that I've been meaning to add. It has a tendency to be lazy and leave crud around because it doesn't want to update tests. Make it clean this up or do it in a follow up. Same below if possible. Sometimes these can be moved to helpers inside the test code. Another cleanup is to use a builder vs. having N constructors.

[mattklein123]

This is where I think I would expect filter/transform to be applied before inserting, same below. This and below can probably be a shared helper.

[mattklein123]

Pretty sure we have already had this conversation several times but I don't understand why we need N maps here vs. a key that includes the type. Seems simpler and probably uses less resources.