Skip to content

Add OpenTelemetry Package#1172

Merged
mtmk merged 6 commits into
release/3.0from
nats-otel-pkg
Jun 25, 2026
Merged

Add OpenTelemetry Package#1172
mtmk merged 6 commits into
release/3.0from
nats-otel-pkg

Conversation

@mtmk

@mtmk mtmk commented May 29, 2026

Copy link
Copy Markdown
Member

@synadia-claude-reviewer

synadia-claude-reviewer Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 2m 55s —— View job


Code Review

  • Read CLAUDE.md for repo guidelines
  • Examine PR diff and changed files
  • Review code for correctness, security, and performance
  • Post inline comments for specific issues
  • Write overall review summary

This PR adds a thin NATS.Client.OpenTelemetry package with two extension methods that wire NatsTelemetry.SourceName into OpenTelemetry's TracerProviderBuilder and MeterProviderBuilder. The tracing side is correct and functional. Two issues found:

1. AddNatsClientInstrumentation(MeterProviderBuilder) is a no-op (medium)

The library only instantiates an ActivitySource("NATS.Net") — there is no System.Diagnostics.Metrics.Meter created anywhere. Registering the Meter name with AddMeter(...) will succeed silently but never receive any instrument data. The docstring on NatsTelemetry.SourceName ("The same name is used for both the ActivitySource and the Meter") is therefore incorrect today. This needs to be resolved by either:

  • Adding a static readonly Meter in Internal.Telemetry (to back the metrics promise), or
  • Removing the MeterProviderBuilder overload and updating the docstring until metrics are actually emitted.

See inline comments on NatsTelemetry.cs:7 and NatsInstrumentationExtensions.cs:21.

2. Duplicate string constant that can silently diverge (low)

Internal.Telemetry.NatsActivitySource is hardcoded to "NATS.Net" independently of NatsTelemetry.SourceName. If the public constant is ever updated, the internal ActivitySource won't follow. The fix is one line:

// Internal/Telemetry.cs line 9
public const string NatsActivitySource = NatsTelemetry.SourceName;

The rest of the PR — the TracerProviderBuilder extension, the project structure, IsPackable inheritance from src/Directory.Build.props, target framework multi-targeting, and the tests — all look correct.

@mtmk mtmk changed the title otel: add OpenTelemetry package Add OpenTelemetry package May 29, 2026
@mtmk mtmk changed the title Add OpenTelemetry package Add OpenTelemetry Package May 29, 2026
Comment thread src/NATS.Client.Core/NatsTelemetry.cs
Comment thread src/NATS.Client.OpenTelemetry/NatsInstrumentationExtensions.cs
@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown

mtmk added 2 commits June 12, 2026 07:23
Mirrors AddRabbitMQInstrumentation(configure): exposes a discoverable
entry point to set Filter and Enrich instead of reaching into the Core
static. Writes the process-wide NatsInstrumentationOptions.Default, so
configuration is global, not per provider.
@mtmk mtmk linked an issue Jun 15, 2026 that may be closed by this pull request
mtmk added 2 commits June 23, 2026 11:54
FilterSubjects compiles NATS subject patterns (include/exclude, with * and >
wildcards) into a NatsInstrumentationOptions.Filter predicate, combined with any
existing filter. Document it alongside the operation.duration histogram bucket
view, which stays out of the package to avoid pulling in the OpenTelemetry SDK
dependency.
@mtmk mtmk added the ready label Jun 24, 2026

@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

# Conflicts:
#	tools/site_src/documentation/advanced/opentelemetry.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NATS.Client.OpenTelemetry package

2 participants