Skip to content

Fix null-key tag in OpenTelemetry receive fallback#1205

Merged
mtmk merged 1 commit into
release/3.0from
fix-otel-receive-null-tag
Jul 1, 2026
Merged

Fix null-key tag in OpenTelemetry receive fallback#1205
mtmk merged 1 commit into
release/3.0from
fix-otel-receive-null-tag

Conversation

@mtmk

@mtmk mtmk commented Jul 1, 2026

Copy link
Copy Markdown
Member

The receive-path telemetry fallback (used when the connection is not a fully connected NatsConnection) allocated a fixed-size tag array but only populated the reply-to slot conditionally, leaving a default null-keyed entry that was passed to StartActivity when there was no reply-to. Size the array to the tags actually set, matching the send path, and add a regression test over the fallback branch.

The non-NatsConnection receive path allocated a fixed 10-slot tag array but
only filled the reply-to slot when a reply-to was present, leaving a default
{null, null} entry that was handed to StartActivity. Size the array to the
tags actually set, matching the send path.
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 1m 44s —— View job


PR Review

  • Gather context
  • Read changed files
  • Analyze correctness and security
  • Post findings

Summary

The fix is correct and minimal. The fallback branch in StartReceiveActivity previously allocated a fixed 10-slot tag array regardless of whether replyTo was set, leaving a default {null, null} KeyValuePair at index 9 when no reply-to was present. The PR sizes the array dynamically (9 base + 1 conditional), matching the pattern already used in the primary (connected) path.

No issues found. A couple of minor observations for awareness:

Observation 1 — queueGroup is always emitted in the fallback path (pre-existing)

In the fallback branch (Telemetry.cs:294), QueueGroup is unconditionally written at index 3 even when queueGroup is null, producing a tag with a non-null key but a null value:

tags[3] = new KeyValuePair<string, object?>(Constants.QueueGroup, queueGroup); // value may be null

The primary path (lines 281–282) conditionally adds it only when non-null. This asymmetry is pre-existing and unrelated to this PR — it doesn't cause a null-key issue — but it means the fallback always emits a QueueGroup tag even for non-queue subscriptions. No action needed here; just noting for future consistency cleanup.

Observation 2 — Test covers the regression well

The test correctly targets the fallback branch by using an unstarted NatsConnection (no ServerInfo), wires up an ActivityListener with AllData sampling before creating the connection, and asserts both:

  • No tags have a null key
  • The ReplyTo tag is present iff replyTo was supplied

Both [InlineData(null)] and [InlineData("_INBOX.reply")] cases are covered. Listener disposal via using var is correct.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

@scottf scottf 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.

LGTM

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.

2 participants