Skip to content

fix(type_router): validate declared outputs#3575

Open
ihopenre-eng wants to merge 1 commit into
open-telemetry:mainfrom
ihopenre-eng:fix/type-router-output-validation
Open

fix(type_router): validate declared outputs#3575
ihopenre-eng wants to merge 1 commit into
open-telemetry:mainfrom
ihopenre-eng:fix/type-router-output-validation

Conversation

@ihopenre-eng

Copy link
Copy Markdown

Fixes #3569

Summary

  • add a minimum declared-output requirement to node wiring contracts
  • apply wiring-contract validation during static startup validation as well as pipeline construction
  • require processor:type_router to declare at least one output and cover the invalid fully connected topology with a regression test

Testing

  • cargo test --offline --bin df_engine --no-default-features --features crypto-ring validate_type_router_requires_declared_outputs -- --nocapture
  • cargo check -p otap-df-engine -p otap-df-controller -p otap-df-core-nodes -p otap-df-validation
  • cargo clippy -p otap-df-engine -p otap-df-controller -p otap-df-core-nodes -p otap-df-validation --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • chloggen validate --config rust/otap-dataflow/.chloggen/config.yaml

AI assistance

Codex assisted with repository navigation, implementation drafting, and local test orchestration. I reviewed the changes and verified the behavior with the checks above.

Require node factories to declare a minimum output count through their wiring contract and run that validation during static startup checks. This makes --validate-and-exit reject type routers that would fail during pipeline construction.
@ihopenre-eng
ihopenre-eng requested a review from a team as a code owner July 24, 2026 09:13
@github-actions github-actions Bot added area:pipeline Rust Pipeline Related Tasks lang:rust Pull requests that update Rust code area:processor Core and Contrib processor nodes area:engine Internal engine features area:validation Work related to extended validation testing size/L labels Jul 24, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 24, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-26 03:48:23 UTC.

  • Waiting on: Author
  • Next step: Address or respond to 1 review feedback item:
    • Top-level feedback: 1
    • For each item, reply to move the discussion forward, e.g. link to the commit that addresses it, explain why no change is needed, or ask a follow-up question.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers. If the last refreshed time above predates your latest reply or push, the dashboard hasn't processed it yet.

@lquerel lquerel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, i.e. the goal of keeping --validate-and-exit consistent with runtime startup is definitely right.

After reproducing #3569 against the parent commit, we found that the original analysis attributing the failure to missing type-router outputs was incorrect. We’re sorry for the confusion.

The type router recognizes three well-known named routes: logs, metrics, and traces. Under the existing configuration model, the outputs list is an optional declaration/allowlist; the actual ports can be established directly by connections such as router["logs"], router["metrics"], and router["traces"]. I verified that this configuration, without an outputs list, validates, starts ready, and processes all three signal types on the parent commit.

The exact configuration from #3569 actually fails because all traffic-generator signal weights default to zero: at least one of metric_weight, trace_weight, or log_weight must be > 0

Runtime calls TrafficConfig::validate(), but static validation only deserializes Config, so --validate-and-exit misses this semantic constraint.

Suggested fix:

  • Remove the minimum-declared-output requirement from type_router.
  • Reuse a single traffic-generator parse-and-validate helper from both from_config() and the factory’s validate_config.
  • Ensure static validation calls config.get_traffic_config().validate().
  • Replace the regression test with the original traffic-generator configuration and assert that validation reports the zero-weight error.
  • Add a positive test showing that one non-zero signal weight passes.
  • Preserve a positive test demonstrating that omitted outputs with explicit type-router connections remains valid.

This fixes the actual validation/runtime mismatch without rejecting previously valid router configurations. Thanks again, I’d be happy to review the revision.

Add @AaronRM for visibility

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

Labels

area:engine Internal engine features area:pipeline Rust Pipeline Related Tasks area:processor Core and Contrib processor nodes area:validation Work related to extended validation testing lang:rust Pull requests that update Rust code size/L

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Config validation (--validate-and-exit) accepts processor:type_router config that fails at runtime pipeline build (pipeline_build_failed)

2 participants