-
-
Notifications
You must be signed in to change notification settings - Fork 227
feat(metrics): Trace-connected Metrics (Implementation) #4834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4834 +/- ##
==========================================
- Coverage 73.79% 72.82% -0.97%
==========================================
Files 483 494 +11
Lines 17551 17830 +279
Branches 3461 3515 +54
==========================================
+ Hits 12952 12985 +33
- Misses 3746 3988 +242
- Partials 853 857 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| _attributes ??= new Dictionary<string, object>(); | ||
|
|
||
| _attributes[key] = new SentryAttribute(value); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SetAttribute wraps value incorrectly causing data loss during serialization
High Severity
The public SetAttribute method wraps values in new SentryAttribute(value) and stores them in _attributes which is typed as Dictionary<string, object>. During serialization, SentryAttributeSerializer.WriteAttribute is called with attribute.Value as an object, which invokes the object? overload instead of the SentryAttribute overload. Since SentryAttribute doesn't match any supported types in WriteAttributeValue, it falls through to the default case calling ToString(), resulting in all attribute values being serialized as the string "Sentry.Protocol.SentryAttribute" instead of their actual values. The internal SetAttributes methods correctly store raw values without wrapping, creating an inconsistency. The fix is to store value directly without wrapping in SentryAttribute.
🔬 Verification Test
Why verification test was not possible: This is a .NET/C# codebase that would require setting up the complete build environment and test infrastructure to run. However, the bug can be verified by tracing the code path: SetAttribute stores new SentryAttribute(value) in a Dictionary<string, object>, then WriteTo iterates this dictionary and calls SentryAttributeSerializer.WriteAttribute(writer, attribute.Key, attribute.Value, logger). Since attribute.Value is typed as object, the object? overload is called which doesn't handle SentryAttribute type and falls through to value.ToString() in the else branch of WriteAttributeValue.
| ### Features | ||
|
|
||
| - Extended `SentryThread` by `Main` to allow indication whether the thread is considered the current main thread ([#4807](https://github.com/getsentry/sentry-dotnet/pull/4807)) | ||
| - Add _experimental_ support for [Sentry trace-connected Metrics](https://docs.sentry.io/product/explore/metrics/) ([#4834](https://github.com/getsentry/sentry-dotnet/pull/4834)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 🚫 The changelog entry seems to be part of an already released section
## 6.1.0.
Consider moving the entry to the## Unreleasedsection, please.
…rics Upgrades Sentry .NET SDK from 6.0.0 to 6.1.0-alpha.1 to test the new experimental trace-connected metrics API from PR getsentry/sentry-dotnet#4834. Changes: - Upgrade Sentry SDK to 6.1.0-alpha.1 - Add SentryClientMetrics decorator that emits metrics to Sentry - Enable experimental metrics in all apps (Server, Console, Android) - Use SentryClientMetrics in DI registrations The new metrics API provides: - AddCounter: for counting events (files, uploads, errors) - RecordDistribution: for value distributions (uploaded bytes) - RecordGauge: for point-in-time values (jobs in flight) Metrics are automatically correlated with the active trace/span. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…rics Upgrades Sentry .NET SDK from 6.0.0 to 6.1.0-alpha.1 to test the new experimental trace-connected metrics API from PR getsentry/sentry-dotnet#4834. Changes: - Upgrade Sentry SDK to 6.1.0-alpha.1 - Add SentryClientMetrics decorator that emits metrics to Sentry - Enable experimental metrics in all apps (Server, Console, Android) - Use SentryClientMetrics in DI registrations (Core, Server, Console) - Fix duplicate ClientMetrics registration in Core/Startup.cs The new metrics API provides: - AddCounter: for counting events (files, uploads, errors) - RecordDistribution: for value distributions (uploaded bytes) - RecordGauge: for point-in-time values (jobs in flight) Metrics are automatically correlated with the active trace/span. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…rics Upgrades Sentry .NET SDK from 6.0.0 to 6.1.0-alpha.1 to test the new experimental trace-connected metrics API from PR getsentry/sentry-dotnet#4834. Changes: - Upgrade Sentry SDK to 6.1.0-alpha.1 - Add SentryClientMetrics decorator that emits metrics to Sentry - Make ClientMetrics methods virtual for proper polymorphism - Enable experimental metrics in all apps (Server, Console, Android) - Use SentryClientMetrics in DI registrations (Core, Server, Console) - Fix duplicate ClientMetrics registration in Core/Startup.cs The new metrics API provides: - AddCounter: for counting events (files, uploads, errors) - RecordDistribution: for value distributions (uploaded bytes) - RecordGauge: for point-in-time values (jobs in flight) Metrics are automatically correlated with the active trace/span. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implementation of Trace-connected Metrics
See https://develop.sentry.dev/sdk/telemetry/metrics/
This changeset contains the initial implementation of Trace-connected Metrics for Sentry.
Related changesets