Skip to content

feat(examples): add key expression argument to z_pub_thr and z_sub_thr#2549

Open
kydos wants to merge 5 commits into
eclipse-zenoh:mainfrom
kydos:feat/thr-ex-key
Open

feat(examples): add key expression argument to z_pub_thr and z_sub_thr#2549
kydos wants to merge 5 commits into
eclipse-zenoh:mainfrom
kydos:feat/thr-ex-key

Conversation

@kydos

@kydos kydos commented Apr 6, 2026

Copy link
Copy Markdown
Member

Description

Allow users to specify a custom key expression for the throughput examples via a CLI argument. This makes it possible to benchmark router throughput scaling across multiple instances using either shared or distinct key expressions without modifying the source.

What does this PR do?

See above.

Why is this change needed?

Because otherwise it is not possible to run throughput test on a user specified key-expr.

Related Issues

Closes #2548


🏷️ Label-Based Checklist

Based on the labels applied to this PR, please complete these additional requirements:

Labels: new feature

🆕 New Feature Requirements

Since this PR adds a new feature:

  • Feature scope documented - Clear description of what the feature does and why it's needed
  • Minimum necessary code - Implementation is as simple as possible, doesn't overcomplicate the system
  • New APIs well-designed - Public APIs are intuitive, consistent with existing APIs
  • Comprehensive tests - All functionality is tested (happy path + edge cases + error cases)
  • Examples provided - Usage examples in code comments or separate example files
  • Documentation added - New docs explaining the feature, its use cases, and API
  • Feature flag considered - Can the feature be enabled/disabled for gradual rollout?
  • Performance impact assessed - Memory, CPU, storage implications measured
  • Integration tested - Feature works with existing features

Consider: Can this feature be split into smaller, incremental PRs?

Instructions:

  1. Check off items as you complete them (change - [ ] to - [x])
  2. The PR checklist CI will verify these are completed

This checklist updates automatically when labels change, but preserves your checked boxes.

Allow users to specify a custom key expression for the throughput
examples via a CLI argument. This makes it possible to benchmark router
throughput scaling across multiple instances using either
shared or distinct key expressions without modifying the source.

Closes eclipse-zenoh#2548
Copilot AI review requested due to automatic review settings April 6, 2026 14:20
@kydos kydos added the new feature Something new is needed label Apr 6, 2026

Copilot AI 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.

Pull request overview

Adds a CLI argument to the throughput examples so users can choose the key expression at runtime, enabling benchmarking across multiple instances without editing source (addresses #2548).

Changes:

  • Add --key-expr (KeyExpr<'static>) argument to z_pub_thr and z_sub_thr (default: test/thr).
  • Add --key-expr argument to the shared-memory throughput publisher example (z_pub_shm_thr) and plumb it through parse_args().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
examples/examples/z_sub_thr.rs Adds --key-expr and uses it for subscriber declaration; refactors arg parsing.
examples/examples/z_pub_thr.rs Adds --key-expr and uses it for publisher declaration.
examples/examples/z_pub_shm_thr.rs Adds --key-expr and passes it through parse_args() to publisher declaration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.background()
.wait()
.unwrap();
.expect("Failed to open Zenoh session");

Copilot AI Apr 6, 2026

Copy link

Choose a reason for hiding this comment

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

The error message passed to expect() is misleading here: the failing operation is the subscriber declaration/background start, not opening the Zenoh session (which already happened above). Use an error message that reflects the actual failing call (e.g., declaring/starting the subscriber), or move the expect() to the zenoh::open(...) call.

Suggested change
.expect("Failed to open Zenoh session");
.expect("Failed to declare/start Zenoh subscriber");

Copilot uses AI. Check for mistakes.

@fuzzypixelz fuzzypixelz Apr 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@kydos Copilot is right here, but I would rather write "Failed to declare subscriber".

Comment thread examples/examples/z_sub_thr.rs Outdated
Comment thread examples/examples/z_pub_thr.rs Outdated
Comment thread examples/examples/z_pub_shm_thr.rs Outdated
@codecov

codecov Bot commented Apr 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.82%. Comparing base (9545f32) to head (7ec87c3).
⚠️ Report is 22 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2549      +/-   ##
==========================================
- Coverage   74.83%   74.82%   -0.02%     
==========================================
  Files         399      399              
  Lines       59373    59373              
==========================================
- Hits        44433    44426       -7     
- Misses      14940    14947       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kydos and others added 4 commits April 8, 2026 13:23
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature Something new is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Throughput example should allow to specify the key expression

3 participants