Skip to content

Separate manifest expansion events from transformation (#344)#375

Draft
leynos wants to merge 1 commit into
mainfrom
issue-344-expansion-event-boundary
Draft

Separate manifest expansion events from transformation (#344)#375
leynos wants to merge 1 commit into
mainfrom
issue-344-expansion-event-boundary

Conversation

@leynos

@leynos leynos commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #344

Manifest expansion no longer emits tracing side effects from the transformation path; it reports filtering through returned data and the caller owns the telemetry policy, as proposed.

Changes

  • src/manifest/expand.rs: when_allows returns an optional FilteredEntry (section, bounded entry-name hash, iteration index, when expression length — the same privacy-preserving fields the old inline event logged); expand_foreach returns an ExpansionReport (counts + events). All tracing usage (including the has_subscriber guard) is gone from the module.
  • src/manifest/mod.rs: new orchestrator-side trace_expansion_report emits the same filtered-entry and summary debug events as before; from_str_named calls it after expansion.

Testing

  • expand_foreach_returns_filtering_stats now also asserts the report carries one event per removed entry with correct sections.
  • The tracing-capture test (renamed trace_expansion_report_emits_debug_event_for_filtered_entry) first asserts expansion emits nothing itself, then verifies the orchestrator-side emitter produces the previous event shape (hash present, raw name/expression absent).

Validation

  • make check-fmt / make lint / make test — pass (37 suites)

🤖 Generated with Claude Code

Summary by Sourcery

Separate manifest expansion from telemetry by returning structured filtering reports instead of emitting traces directly.

New Features:

  • Return an ExpansionReport from manifest expansion that includes filtering statistics and per-entry filtering metadata.

Enhancements:

  • Refactor manifest expansion to collect filtered entry data via FilteredEntry records and move tracing emission to an orchestrator-side helper.
  • Ensure manifest expansion remains telemetry-free while preserving previous debug logging semantics through a new trace_expansion_report helper.

Tests:

  • Extend manifest expansion tests to validate the contents of the new ExpansionReport, including per-section filtered events.
  • Update tracing capture tests to assert that expansion emits no telemetry and that trace_expansion_report reproduces the expected debug events.

`expand_foreach` mutated the manifest while `when_allows` emitted
debug telemetry directly (guarded by a subscriber check), embedding
observability policy in the domain transformation path.

Expansion is now telemetry-free. `when_allows` returns an optional
`FilteredEntry` (section, bounded name hash, iteration index,
expression length — the same privacy-preserving fields as before),
`expand_section` accumulates them, and `expand_foreach` returns an
`ExpansionReport` carrying both the per-section counts and the
per-entry events.

The caller owns the tracing policy: `manifest::from_str_named` passes
the report to the new `trace_expansion_report`, which emits the same
filtered-entry and summary debug events as before.

Tests updated to the new boundary: the stats test asserts the
report's events as well as its counts, and the tracing-capture test
verifies expansion emits nothing itself before exercising the
orchestrator-side emitter.
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: da9fff6b-e12f-4d44-b09c-a893d2e53b20

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-344-expansion-event-boundary

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai

sourcery-ai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Separates manifest expansion from telemetry by having expansion return structured filtering data instead of emitting tracing events directly, and introduces an orchestrator-side tracer that logs the same privacy-preserving information based on the expansion report.

Sequence diagram for manifest expansion and telemetry separation

sequenceDiagram
    participant from_str_named
    participant expand_foreach
    participant trace_expansion_report
    participant tracing

    from_str_named->>expand_foreach: expand_foreach(doc, jinja) : ExpansionReport
    expand_foreach-->>from_str_named: ExpansionReport
    from_str_named->>trace_expansion_report: trace_expansion_report(report)
    loop per FilteredEntry
        trace_expansion_report->>tracing: debug(filtered manifest entry by when expression)
    end
    trace_expansion_report->>tracing: debug(expanded manifest foreach and when directives)
Loading

File-Level Changes

Change Details Files
Make manifest expansion return a structured report with filtering stats and per-entry metadata instead of logging via tracing.
  • Introduce FilteredEntry to carry bounded, non-sensitive metadata about entries removed by when expressions, including section, hashed name, iteration index, and when-expression length.
  • Introduce ExpansionReport to aggregate FilteringStats and a vector of FilteredEntry records as the outcome of expansion.
  • Change expand_foreach to return ExpansionReport, collecting filtered entries from targets and actions and building stats from their counts.
  • Change expand_section and expand_target to accumulate FilteredEntry instances instead of simple counters when entries are filtered.
  • Refactor when_allows to return an optional FilteredEntry on filter instead of a boolean, removing all tracing calls and the has_subscriber helper.
src/manifest/expand.rs
Move tracing responsibility to the orchestrator by adding a helper that emits debug telemetry based on the expansion report and wiring it into manifest loading.
  • Import ExpansionReport in the manifest module alongside expand_foreach.
  • Add trace_expansion_report helper that iterates over filtered entries and logs per-entry debug events plus a summary counts event, matching the previous event shape but driven by data.
  • Update from_str_named to call expand_foreach, capture the returned ExpansionReport, and then invoke trace_expansion_report before final rendering.
src/manifest/mod.rs
Update tests to validate the new ExpansionReport structure and to ensure expansion is telemetry-free while the orchestrator-side tracer preserves previous logging behavior.
  • Extend expand_foreach_returns_filtering_stats test to assert ExpansionReport.stats matches expected counts, that there is one filtered-entry event per removed entry, and that section attribution is correct.
  • Adjust tracing-capture test to rename it for trace_expansion_report, ensure expand_foreach emits no telemetry, then call trace_expansion_report and verify the resulting debug event has the same privacy-preserving shape as before, including hashed entry name and absence of raw secrets.
src/manifest/expand_tests.rs
src/manifest/expand_test_cases/tracing_capture.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#344 Refactor manifest expansion so that it reports filtering details via returned data or an observer interface, and does not emit tracing/telemetry directly from the transformation logic.
#344 Move ownership of tracing/telemetry policy for manifest expansion to the caller/orchestrator layer instead of the low-level expansion helpers.
#344 Update existing filtering and tracing tests to reflect the new boundary (no telemetry from expansion, telemetry from caller) and ensure the test/lint/format suite passes.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate manifest expansion events from transformation logic

1 participant