Match OpenTelemetry subject filters without per-message allocation

[mtmk]

The subject filter installed by FilterSubjects split the subject into a string array on every traced publish/subscribe when a filter is configured. Match the subject against the pre-tokenized patterns by walking it token by token with spans, dropping the per-message allocation. Adds coverage for empty-token boundaries (trailing and consecutive dots).

[synadia-claude-reviewer]

Claude finished @mtmk's task in 3m 39s —— View job

---

PR Review

• Gather context and understand the PR
• Read changed files and examine implementation
• Review for correctness, performance, and security
• Post findings

---

Summary

The optimization is correct and sound. Eliminating string.Split('.') on the hot path (called once per traced publish/subscribe when a filter is configured) removes a meaningful allocation — the span-based walk is a clean drop-in.

Correctness trace for key cases:

Subject	Pattern	Old result	New result
orders.new	orders.>	✓	✓
orders	orders.>	✗	✗
foo.bar	foo.*	✓	✓
foo.bar.baz	foo.*	✗	✗
foo	foo.*	✗	✗

The exhausted flag correctly tracks whether the last subject token has been consumed: set to true only when IndexOf('.') returns -1 (no more dots), used in the final return exhausted to enforce equal token counts, and checked in the > branch as !exhausted to ensure at least one token remains for > to absorb.

Two minor comments left inline:

1. src/NATS.Client.OpenTelemetry/NatsInstrumentationOptionsExtensions.cs:101 — The > branch comment "token i must exist" is imprecise: !exhausted actually means "the previous iteration found a dot" (so remaining still has characters), not "subject has a token at index i". For valid NATS subjects this distinction is irrelevant; for trailing-dot subjects remaining would be "" here. Suggested clarification.

2. tests/NATS.Net.OpenTelemetry.Tests/NatsInstrumentationExtensionsTest.cs:119 — The two new test methods test behaviour for protocol-invalid subjects (trailing/consecutive dots are not legal in NATS). The tests are a reasonable regression guard (pinning "consistent with old Split behaviour"), but their names could suggest these are expected NATS matches rather than implementation-detail preservation.

Neither is a bug. The allocation removal is correct, and the edge-case tests match the old code's behaviour exactly.

[github-actions]

🚀 Deployed on https://6a45833cb9ecf1dfe30dabd0--teal-klepon-7e92d0.netlify.app

[mtmk]

@claude the latest commit addresses your review comments, please re-check.

[synadia-claude-reviewer]

Claude finished @mtmk's task in 45s —— View job

---

Re-checking PR after latest commit

• Gather context
• Read changed files to verify review comments were addressed
• Post updated review findings

---

Both previous review comments have been fully addressed in commit 3f2f621.

Comment 1 — > branch comment precision (NatsInstrumentationOptionsExtensions.cs:101)

The imprecise // token i must exist comment has been replaced with the suggested text:

return !exhausted; // '>' needs at least one remaining subject token; exhausted means the subject is fully consumed

✅ Resolved.

Comment 2 — Test method naming (NatsInstrumentationExtensionsTest.cs:118–142)

Both test methods have been renamed to include invalid_subject_ and each now has a leading comment that explicitly calls out the "not protocol compliance, pins Split behaviour" contract:

// Trailing dots are not valid in NATS subjects; this pins the matcher to the same
// behavior as the previous Split-based implementation, not protocol compliance.
public void FilterSubjects_invalid_subject_empty_trailing_token(...)

// Consecutive dots are not valid in NATS subjects; this pins the matcher to the same
// behavior as the previous Split-based implementation, not protocol compliance.
public void FilterSubjects_invalid_subject_greater_than_spans_empty_tokens(...)

✅ Resolved — the intent is now unambiguous.

No remaining concerns. The PR is good to merge.

[scottf]

LGTM