fix: populate sentry.sdk.name and sentry.sdk.version for console apps - #5483
Conversation
Co-Authored-By: Claude (Anthropic) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5483 +/- ##
==========================================
+ Coverage 74.73% 74.75% +0.01%
==========================================
Files 513 513
Lines 18744 18749 +5
Branches 3666 3667 +1
==========================================
+ Hits 14009 14015 +6
+ Misses 3863 3862 -1
Partials 872 872 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jamescrosswell
left a comment
There was a problem hiding this comment.
Thanks @zkasuran - generally this looks good. I've made a couple of minor suggestions... just for housekeeping.
Address review feedback on getsentry#5483: move sentry.sdk.name and sentry.sdk.version as a unit so they never come from different sources (matching Enricher and Scope) and assert Constants.SdkName in the regression tests. Co-Authored-By: Claude (Anthropic) <noreply@anthropic.com>
|
Thanks for the review @jamescrosswell. Both points are addressed in 8bba369. Verified locally in Docker ( |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8bba369. Configure here.
Address the review feedback and Bugbot finding on getsentry#5483. The previous else-if fired whenever either Name or Version was null, so an integration that sets Name but leaves Version null (its GetVersion() can return null) had its name relabelled to the default SDK name. The default now applies only when both fields are unset, matching the both-or-neither intent. Name and version are set independently after that. Adds a regression test on the log and metric paths for the name-only case. Co-Authored-By: Claude (Anthropic) <noreply@anthropic.com>
jamescrosswell
left a comment
There was a problem hiding this comment.
Awesome - thanks for the contribution @zkasuran ! 🙏🏻
Updated [Selenium.WebDriver.ChromeDriver](https://github.com/jsakamoto/nupkg-selenium-webdriver-chromedriver/) from 152.0.7977.7500 to 152.0.7977.8200. <details> <summary>Release notes</summary> _Sourced from [Selenium.WebDriver.ChromeDriver's releases](https://github.com/jsakamoto/nupkg-selenium-webdriver-chromedriver//releases)._ No release notes found for this version range. Commits viewable in [compare view](https://github.com/jsakamoto/nupkg-selenium-webdriver-chromedriver//commits). </details> Updated [Sentry](https://github.com/getsentry/sentry-dotnet) from 6.9.0 to 6.10.0. <details> <summary>Release notes</summary> _Sourced from [Sentry's releases](https://github.com/getsentry/sentry-dotnet/releases)._ ## 6.10.0 ### Features ✨ - feat: Logs sent via `SentrySdk.Logger` no longer require `EnableLogs` by @jamescrosswell in [#5512](getsentry/sentry-dotnet#5512) - feat: `SentryOptions.EnableMetrics` is obsolete and ignored by @jamescrosswell in [#5509](getsentry/sentry-dotnet#5509) ### Fixes 🐛 - fix: Prevent managed exceptions from leaking as NSExceptions, resulting in duplicate exception capture on iOS by @jpnurmi in [#5525](getsentry/sentry-dotnet#5525) - fix(profiling): release the EventPipe session when the SDK shuts down by @jamescrosswell in [#5470](getsentry/sentry-dotnet#5470) - fix: Memory leak in Sentry.Profiling due to EventLog interning tables growing indefinitely by @jamescrosswell in [#5503](getsentry/sentry-dotnet#5503) - fix: Attachments not being sent properly when Spotlight is enabled by @XAN9xXx in [#5511](getsentry/sentry-dotnet#5511) - fix: Heap dump files are now deleted from disk once they have been sent to Sentry by @XAN9xXx in [#5481](getsentry/sentry-dotnet#5481) - fix: populate sentry.sdk.name and sentry.sdk.version for console apps by @zkasuran in [#5483](getsentry/sentry-dotnet#5483) ### Dependencies ⬆️ #### Deps - chore(deps): update Java SDK to v8.54.0 by @github-actions in [#5517](getsentry/sentry-dotnet#5517) - chore(deps): update Cocoa SDK to v9.26.1 by @github-actions in [#5516](getsentry/sentry-dotnet#5516) - chore(deps): update CLI to v3.7.0 by @github-actions in [#5520](getsentry/sentry-dotnet#5520) - chore(deps): update Native SDK to v0.16.4 by @github-actions in [#5508](getsentry/sentry-dotnet#5508) - chore(deps): update Java SDK to v8.53.0 by @github-actions in [#5484](getsentry/sentry-dotnet#5484) ### Other - deps: update perfview (removes the .il suffix from profile module names) by @jamescrosswell in [#5502](getsentry/sentry-dotnet#5502) Commits viewable in [compare view](getsentry/sentry-dotnet@6.9.0...6.10.0). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details>
Pinned [Sentry.AspNetCore](https://github.com/getsentry/sentry-dotnet) at 6.10.0. <details> <summary>Release notes</summary> _Sourced from [Sentry.AspNetCore's releases](https://github.com/getsentry/sentry-dotnet/releases)._ ## 6.10.0 ### Features ✨ - feat: Logs sent via `SentrySdk.Logger` no longer require `EnableLogs` by @jamescrosswell in [#5512](getsentry/sentry-dotnet#5512) - feat: `SentryOptions.EnableMetrics` is obsolete and ignored by @jamescrosswell in [#5509](getsentry/sentry-dotnet#5509) ### Fixes 🐛 - fix: Prevent managed exceptions from leaking as NSExceptions, resulting in duplicate exception capture on iOS by @jpnurmi in [#5525](getsentry/sentry-dotnet#5525) - fix(profiling): release the EventPipe session when the SDK shuts down by @jamescrosswell in [#5470](getsentry/sentry-dotnet#5470) - fix: Memory leak in Sentry.Profiling due to EventLog interning tables growing indefinitely by @jamescrosswell in [#5503](getsentry/sentry-dotnet#5503) - fix: Attachments not being sent properly when Spotlight is enabled by @XAN9xXx in [#5511](getsentry/sentry-dotnet#5511) - fix: Heap dump files are now deleted from disk once they have been sent to Sentry by @XAN9xXx in [#5481](getsentry/sentry-dotnet#5481) - fix: populate sentry.sdk.name and sentry.sdk.version for console apps by @zkasuran in [#5483](getsentry/sentry-dotnet#5483) ### Dependencies ⬆️ #### Deps - chore(deps): update Java SDK to v8.54.0 by @github-actions in [#5517](getsentry/sentry-dotnet#5517) - chore(deps): update Cocoa SDK to v9.26.1 by @github-actions in [#5516](getsentry/sentry-dotnet#5516) - chore(deps): update CLI to v3.7.0 by @github-actions in [#5520](getsentry/sentry-dotnet#5520) - chore(deps): update Native SDK to v0.16.4 by @github-actions in [#5508](getsentry/sentry-dotnet#5508) - chore(deps): update Java SDK to v8.53.0 by @github-actions in [#5484](getsentry/sentry-dotnet#5484) ### Other - deps: update perfview (removes the .il suffix from profile module names) by @jamescrosswell in [#5502](getsentry/sentry-dotnet#5502) Commits viewable in [compare view](getsentry/sentry-dotnet@6.9.0...6.10.0). </details> Updated [Sentry.Extensions.Logging](https://github.com/getsentry/sentry-dotnet) from 6.9.0 to 6.10.0. <details> <summary>Release notes</summary> _Sourced from [Sentry.Extensions.Logging's releases](https://github.com/getsentry/sentry-dotnet/releases)._ ## 6.10.0 ### Features ✨ - feat: Logs sent via `SentrySdk.Logger` no longer require `EnableLogs` by @jamescrosswell in [#5512](getsentry/sentry-dotnet#5512) - feat: `SentryOptions.EnableMetrics` is obsolete and ignored by @jamescrosswell in [#5509](getsentry/sentry-dotnet#5509) ### Fixes 🐛 - fix: Prevent managed exceptions from leaking as NSExceptions, resulting in duplicate exception capture on iOS by @jpnurmi in [#5525](getsentry/sentry-dotnet#5525) - fix(profiling): release the EventPipe session when the SDK shuts down by @jamescrosswell in [#5470](getsentry/sentry-dotnet#5470) - fix: Memory leak in Sentry.Profiling due to EventLog interning tables growing indefinitely by @jamescrosswell in [#5503](getsentry/sentry-dotnet#5503) - fix: Attachments not being sent properly when Spotlight is enabled by @XAN9xXx in [#5511](getsentry/sentry-dotnet#5511) - fix: Heap dump files are now deleted from disk once they have been sent to Sentry by @XAN9xXx in [#5481](getsentry/sentry-dotnet#5481) - fix: populate sentry.sdk.name and sentry.sdk.version for console apps by @zkasuran in [#5483](getsentry/sentry-dotnet#5483) ### Dependencies ⬆️ #### Deps - chore(deps): update Java SDK to v8.54.0 by @github-actions in [#5517](getsentry/sentry-dotnet#5517) - chore(deps): update Cocoa SDK to v9.26.1 by @github-actions in [#5516](getsentry/sentry-dotnet#5516) - chore(deps): update CLI to v3.7.0 by @github-actions in [#5520](getsentry/sentry-dotnet#5520) - chore(deps): update Native SDK to v0.16.4 by @github-actions in [#5508](getsentry/sentry-dotnet#5508) - chore(deps): update Java SDK to v8.53.0 by @github-actions in [#5484](getsentry/sentry-dotnet#5484) ### Other - deps: update perfview (removes the .il suffix from profile module names) by @jamescrosswell in [#5502](getsentry/sentry-dotnet#5502) Commits viewable in [compare view](getsentry/sentry-dotnet@6.9.0...6.10.0). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Closes #5352
What changed and why
Structured logs and trace metrics from a plain console app were going out with no
sentry.sdk.nameand nosentry.sdk.versionattribute. The same code underASP.NET Core was fine. These attributes identify the SDK that produced the data, so
a whole class of apps was shipping logs and metrics that could not be attributed to
the .NET SDK.
Root cause:
SentryAttributes.SetDefaultAttributesreads the SDK fields off theSdkVersionit is handed:On the logs path (
SentryLog.cs:153) and the metrics path(
SentryMetric.Factory.cs:23) the value passed in isscope.Sdk. Both sites try tofall back with
?? SdkVersion.Instance, but that fallback is dead:Scope.Sdkis anon-null auto-initialized property (
Scope.cs:277,public SdkVersion Sdk { get; } = new();),so
scope?.Sdk ?? SdkVersion.Instancealways resolves toscope.Sdk. In a consoleapp nothing populates that object, so
NameandVersionstay null and both guardsabove are false. Framework integrations do not hit this: ASP.NET Core fills
scope.SdkinSentryMiddleware.cs:257-258, so its logs carry the attributes.The fix falls back per field to the populated
SdkVersion.Instanceat the one placeboth paths share:
When an integration has already set
scope.Sdk.Name, that value is non-null so the??short circuits and the integration still wins. The fallback only supplies avalue where the field would otherwise be null.
SdkVersion.Instanceis the sameobject the envelope header uses, so logs and metrics now agree with the envelope.
Tests
SentryLogTests.SetDefaultAttributes_EmptyScopeSdk_UsesSdkInstancebuilds alog with a fresh
new Scope(options)(the console case) and assertssentry.sdk.name == "sentry.dotnet"with a non-emptysentry.sdk.version.SentryMetricTests.SetDefaultAttributes_EmptySdk_UsesSdkInstancedoes thesame for a metric built with
new SdkVersion().SentryLogTests.WriteTo_Envelope_MinimalSerializedSentryLogandSentryMetricTests.WriteTo_Envelope_MinimalSerializedSentryMetricwere pinning theold payload with no SDK attributes. They now include the SDK name and version,
which is the correct serialized form after the fix.
The existing
Protocol_Default_VerifyAttributestests never caught this because theypre-populate the Sdk before calling
SetDefaultAttributes.Verification
Verified locally in Docker (
mcr.microsoft.com/dotnet/sdk:10.0.302, the exact SDKpinned by
global.json, host runs net10.0):Failed: 0, Passed: 2533, Skipped: 5, Total: 2538.SentryAttributes.cswhile keeping the tests:Failed: 4, Passed: 2529.The four failures are the two new tests plus the two corrected serialization tests,
which reproduces the bug.
dotnet format --verify-no-changeson the changed files: no changes.Changelog
The commit and this PR lead with
fix:, so craft categorizes it under Fixes atrelease time. Per CONTRIBUTING.md I have not edited
CHANGELOG.mdby hand. Let meknow if you want a custom
### Changelog Entrywith more detail than the title.AI disclosure
AI assistance (Claude, Anthropic) was used to trace the root cause, write the fix and
the tests, then run the suite. I own the change, reviewed it and verified it locally
before submitting. Verified: the full
Sentry.Testssuite on net10.0 (2533 passing,0 failing); the bug reproduced by reverting only the source file (4 failing);
dotnet format --verify-no-changesclean on the changed files.