Add support for `Filter` and `Enrich` for OpenTelemetry activities #859

aradalvand · 2025-05-22T16:19:57Z

Resolves #857

Several important considerations:

1 dependency added: OpenTelemetry.Api to NATS.Client.Core — needed for the TracerProviderBuilder.AddNatsInstrumentation extension method. This makes NATS dependent on the so-called OpenTelemetry API, which is expected. But if you're strongly opposed to this, we could also remove this method. Won't take away much from the core functionality added here which are Filter and Enrich. (Dependency removed)
Since Telemetry.Send* methods were also used in places without dependency injection available (e.g. the NatsMsg struct), I couldn't integrate NatsInstrumentationOptions into the IOptions system (which would've probably been more ideal) — so I created a static Default property instead, which the Action<NatsInstrumentationOptions> input parameter effectively configures, and that property is then used by the Telemetry class.
I think it would also be useful to add an IsInternal flag to NatsInstrumentationContext (equivalent to the old NATS.Net.Internal activity source name) so that people could easily filter out lower-level communication (e.g. for request-replies), which could be verbose and chatty.

Made a number of tangential improvements too (if you disagree with any of them let me know and I could revert them back):

Made the methods of the internal Telemetry class public — this is the most conventional pattern in .NET (which this same codebase also follows in almost all other places).
Added isRemote: true to ActivityContext.TryParse — the OpenTelemetry specification states that span contexts extracted from text maps using propagators (e.g. DistributedContextPropagator) should be isRemote: true — see this link.
In NatsMsg<T>.Build I moved the code for activity creation before Deserialize is called, so that if the user's custom deserializer spawns an activity (which mine does, for example, so this would be useful for me), that's included in the trace. This makes things symmetric with how RequestAsync and PublishAsync work as well (they encompasses the ser/de step in their activities).

src/NATS.Client.Core/NATS.Client.Core.csproj

src/NATS.Client.Core/Internal/NatsInstrumentationOptions.cs

thompson-tomo · 2025-05-23T04:19:02Z

See couple of comments but I would also suggest waiting for #859 to be merged which would enable the context & options to reside in the abstractions package.

aradalvand · 2025-05-23T13:16:05Z

See couple of comments but I would also suggest waiting for #859 to be merged which would enable the context & options to reside in the abstractions package.

@thompson-tomo Waiting for what to be merged, sorry? #859 is this PR.

thompson-tomo · 2025-05-23T13:23:11Z

Ahh my mistake @aradalvand I meant #858

src/NATS.Client.Core/Internal/Telemetry.cs

aradalvand · 2025-05-24T00:42:04Z

I have no idea why the Windows test is failing... Weird.
Update: It seems to be failing without this PR's changes too:

Another update: looks like the main branch is also failing so we're in good company haha:
https://github.com/nats-io/nats.net/commits/main/

mtmk · 2025-05-24T12:34:49Z

test fail maybe a flapper. just kicked again

mtmk · 2025-05-24T12:38:20Z

may i move this PR onto release/2.7 branch? it might make it easier to coordinate with other otel stuff for us also we can hopefully release preview quicker

aradalvand · 2025-05-24T13:18:48Z

Sure @mtmk feel free to do that.

aradalvand · 2025-06-01T22:48:41Z

@mtmk Any idea about the timeout exception in two of the tests? Weird.

mtmk · 2025-06-02T09:27:13Z

@mtmk Any idea about the timeout exception in two of the tests? Weird.

Fetch_dispose_test is a flapper and the can not start to connect nats server is because test couldn't setup the nast server which is not related to test itself. i can re run them but need #875 to be merged first.

src/NATS.Client.Core/Internal/Telemetry.cs

…t activity context

mtmk

couple more question about public exposure

src/NATS.Client.Core/Internal/NatsInstrumentationOptions.cs

src/NATS.Client.Core/Internal/Telemetry.cs

mtmk

LGTM

mtmk · 2025-06-09T20:01:01Z

thanks @aradalvand apologies for the delay

aradalvand · 2025-06-09T20:04:16Z

@mtmk No worries at all! Thanks to you

* #851 Move Serialization Interface into Abstractions (#858) * Obsolete ReplyAsync method for NatsJsMsg (#839) * Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859) * #848 Tweak dependencies (#853) * #863 Fix naming of consumer group attribute (#870)

* #863 Fix naming of consumer group attribute (#870) Signed-off-by: James Thompson <[email protected]> * #848 Tweak dependencies (#853) * #848 Tweak dependencies Signed-off-by: James Thompson <[email protected]> * Update NATS.Client.Core.csproj Signed-off-by: James Thompson <[email protected]> * Update NATS.Client.Core.csproj Signed-off-by: James Thompson <[email protected]> * Make STJ explicit dependency for JetStream Signed-off-by: James Thompson <[email protected]> * Update NATS.Client.JetStream.csproj Signed-off-by: James Thompson <[email protected]> * Force sdk to 8.0.0 Signed-off-by: James Thompson <[email protected]> * Add newer stj for net 6 Signed-off-by: James Thompson <[email protected]> --------- Signed-off-by: James Thompson <[email protected]> * Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859) * Add support for `Filter` and `Enrich` for OpenTelemetry activities * Make `internal` methods in `internal Telemetry` `public` * Fix package versions and whatnot * Remove `TracerProviderBuilderExtensions` * Include `Deserialize` in the receive activity * Revert back accidental change * Add `ParentContext` to `NatsInstrumentationContext` * Make `GetActivityContext` public to provide the ability to get context activity context * Make preprocessor directive more accurate * Revert .csproj formatting * Move public artifacts out of the `Internal` namespace/folder * Fix build script --------- Co-authored-by: Ziya Suzen <[email protected]> * Obsolete ReplyAsync method for NatsJsMsg (#839) * Obsolete ReplyAsync method * Obsolete ReplyAsync method on interface * #851 Move Serialization Interface into Abstractions (#858) * Tweaks Signed-off-by: James Thompson <[email protected]> * Add missing dependencies Signed-off-by: James Thompson <[email protected]> * switch to system.memory Signed-off-by: James Thompson <[email protected]> * Added missing using Signed-off-by: James Thompson <[email protected]> * Reduce csproj file contents Signed-off-by: James Thompson <[email protected]> --------- Signed-off-by: James Thompson <[email protected]> * Release 2.7.0-preview.1 (#905) * #851 Move Serialization Interface into Abstractions (#858) * Obsolete ReplyAsync method for NatsJsMsg (#839) * Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859) * #848 Tweak dependencies (#853) * #863 Fix naming of consumer group attribute (#870) * fix: `NatsInstrumentationOptions.Default` gets reset each time (#907) * initialize `NatsInstrumentationOptions.Default` once * `NatsInstrumentationContext.ActivityContext` doesn't need to be nullable * no need for `GetActivityContext` on NatsJSMsg * Release 2.7.0-preview.2 (#908) * fix: `NatsInstrumentationOptions.Default` gets reset each time (#907) * Also, fixes from main * Fix build warnings (#912) * fix: `NatsJSConsumer` never disposing receive activities (#911) * Fix `NatsJSConsumer` never disposing receive activities * Use `ActivityEndingMsgReader` to `NatsJSFetch` and `NatsJSOrderedConsume` * Consolidate activity ending message readers into one class * Run `dotnet format` * Release 2.7.0-preview.3 (#922) * fix: `NatsJSConsumer` never disposing receive activities (#911) * Merge from main * chore: rework NatsOpt.Default initialization (#921) * chore: rework NatsOpt.Default initialization * chore: removed unused using statement * Fix kv ttl interface (#909) KV TTL should only be allowed on Create and Purge * Release 2.7.0-preview.4 (#931) Fixes merged from main * Release 2.7.0-preview.5 (#944) Merge from main aligning with 2.6.8 release. * Fix JetStream publish retry defaults (#939) Decision to retry jetstream publish requests should be letft to the application since it depends on the delivery and durability requirements of their solution. * Fix publish 503 test (#958) With 2.7.x we have made the default to not re-publish on failure. * Release 2.7.0-preview.6 (#956) * Fix JetStream publish retry defaults (#939) * Merge from main * Release 2.7.0-preview.7 (#970) Merged from main * fix: handle 408 Requests Pending responses for fetch requests (#973) * Handle 408 Requests Pending responses for fetch requests * Remove redundant `Console.WriteLine` that was put there for testing * Release 2.7.0-preview.8 (#986) * fix: handle 408 Requests Pending responses for fetch requests (#973) * (merge from main) Object store item size fix (#977) * Fix Ad-Hoc JSON Serializer to use Default Options (#984) * Fix connection state for consume (#959) * Fix connection state for consume * Enhance connection state handling with NatsConnectionFailedException * Improve error handling for connection failures and add support for configurable 503 error thresholds in JetStream consumers * Add tests for connection failure handling and configurable 503 error thresholds in JetStream consumers * Fix format * Fix test * Update INatsJsConsumer to return INatsJsMsg (#1004) --------- Signed-off-by: James Thompson <[email protected]> Co-authored-by: James Thompson <[email protected]> Co-authored-by: Arad Alvand <[email protected]> Co-authored-by: Yeong Jong Lim <[email protected]> Co-authored-by: regnrat <[email protected]>

* Replace `OperationCanceledException` with `NatsTimeoutException` (#1022) * Replace deprecated IndexRange package with Microsoft.Bcl.Memory to solve possible dependency conflicts (for netFramework/netStandard2.0) (#1009) * PeerInfo.Lag is wrong type (#1021) * (Merge Branch) Release 2.7 (#874) * Update INatsJsConsumer to return INatsJsMsg (#1004) * Fix connection state for consume (#959) * Fix Ad-Hoc JSON Serializer to use Default Options (#984) * fix: handle 408 Requests Pending responses for fetch requests (#973) * Fix publish 503 test (#958) * Fix JetStream publish retry defaults (#939) * Fix kv ttl interface (#909) * chore: rework NatsOpt.Default initialization (#921) * fix: `NatsJSConsumer` never disposing receive activities (#911) * Fix build warnings (#912) * fix: `NatsInstrumentationOptions.Default` gets reset each time (#907) * #851 Move Serialization Interface into Abstractions (#858) * Obsolete ReplyAsync method for NatsJsMsg (#839) * Add support for `Filter` and `Enrich` for OpenTelemetry activities (#859) * #848 Tweak dependencies (#853) * #863 Fix naming of consumer group attribute (#870)

aradalvand force-pushed the main branch 2 times, most recently from 36a4df8 to 1613fe2 Compare May 22, 2025 17:40

aradalvand marked this pull request as ready for review May 22, 2025 17:48

aradalvand mentioned this pull request May 22, 2025

Add Filter hook for OpenTelemetry activities #857

Closed

thompson-tomo suggested changes May 23, 2025

View reviewed changes

src/NATS.Client.Core/NATS.Client.Core.csproj Outdated Show resolved Hide resolved

src/NATS.Client.Core/Internal/NatsInstrumentationOptions.cs Show resolved Hide resolved

mtmk added the otel OpenTelemetry label May 23, 2025

thompson-tomo suggested changes May 23, 2025

View reviewed changes

src/NATS.Client.Core/Internal/Telemetry.cs Show resolved Hide resolved

aradalvand requested a review from mtmk May 23, 2025 16:34

aradalvand force-pushed the main branch from cc67ae5 to 9232135 Compare May 23, 2025 23:17

mtmk changed the base branch from main to release/2.7 May 24, 2025 15:40

mtmk added this to the 2.7 milestone May 25, 2025

mtmk reviewed Jun 9, 2025

View reviewed changes

src/NATS.Client.Core/Internal/Telemetry.cs Outdated Show resolved Hide resolved

aradalvand added 8 commits June 9, 2025 15:01

Add support for Filter and Enrich for OpenTelemetry activities

dd3a8f8

Make internal methods in internal Telemetry public

3701244

Fix package versions and whatnot

263756e

Remove TracerProviderBuilderExtensions

8cf21fc

Include Deserialize in the receive activity

053b6a3

Revert back accidental change

c2a4961

Add ParentContext to NatsInstrumentationContext

58d9bfa

Make GetActivityContext public to provide the ability to get contex…

0628808

…t activity context

aradalvand force-pushed the main branch from 6f79e2e to 0628808 Compare June 9, 2025 19:04

aradalvand added 2 commits June 9, 2025 15:11

Make preprocessor directive more accurate

a1414e6

Revert .csproj formatting

046f6fb

aradalvand force-pushed the main branch from 5170c5c to 046f6fb Compare June 9, 2025 19:17

mtmk reviewed Jun 9, 2025

View reviewed changes

src/NATS.Client.Core/Internal/NatsInstrumentationOptions.cs Show resolved Hide resolved

src/NATS.Client.Core/Internal/Telemetry.cs Show resolved Hide resolved

aradalvand and others added 2 commits June 9, 2025 15:23

Move public artifacts out of the Internal namespace/folder

93a2cd9

Fix build script

9789369

mtmk approved these changes Jun 9, 2025

View reviewed changes

mtmk merged commit 7d9cb11 into nats-io:release/2.7 Jun 9, 2025
23 of 26 checks passed

mtmk mentioned this pull request Jul 18, 2025

Release 2.7 #874

Merged

mtmk mentioned this pull request Jul 18, 2025

Release 2.7.0-preview.1 #905

Merged

aradalvand mentioned this pull request Jul 18, 2025

fix: NatsInstrumentationOptions.Default gets reset each time #907

Merged

mtmk mentioned this pull request Dec 19, 2025

Release 2.7.0 #1024

Merged

dependabot bot mentioned this pull request Dec 22, 2025

chore(deps): Bump NATS.Net from 2.6.11 to 2.7.0 four-robots/dotgnatly#85

Closed

dependabot bot mentioned this pull request Jan 12, 2026

chore(deps): Bump NATS.Net from 2.6.11 to 2.7.1 four-robots/dotgnatly#86

Open

Add support for Filter and Enrich for OpenTelemetry activities #859

Add support for Filter and Enrich for OpenTelemetry activities #859

Uh oh!

Conversation

aradalvand commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thompson-tomo commented May 23, 2025

Uh oh!

aradalvand commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thompson-tomo commented May 23, 2025

Uh oh!

Uh oh!

aradalvand commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtmk commented May 24, 2025

Uh oh!

mtmk commented May 24, 2025

Uh oh!

aradalvand commented May 24, 2025

Uh oh!

aradalvand commented Jun 1, 2025

Uh oh!

mtmk commented Jun 2, 2025

Uh oh!

Uh oh!

mtmk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mtmk left a comment

Choose a reason for hiding this comment

Uh oh!

mtmk commented Jun 9, 2025

Uh oh!

Uh oh!

aradalvand commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add support for `Filter` and `Enrich` for OpenTelemetry activities #859

Add support for `Filter` and `Enrich` for OpenTelemetry activities #859

aradalvand commented May 22, 2025 •

edited

Loading

aradalvand commented May 23, 2025 •

edited

Loading

aradalvand commented May 24, 2025 •

edited

Loading

aradalvand commented Jun 9, 2025 •

edited

Loading