fix: populate sentry.sdk.name and sentry.sdk.version for console apps - #5483
Open
zkasuran wants to merge 1 commit into
Open
fix: populate sentry.sdk.name and sentry.sdk.version for console apps#5483zkasuran wants to merge 1 commit into
zkasuran wants to merge 1 commit into
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.72% -0.02%
==========================================
Files 513 513
Lines 18744 18744
Branches 3666 3666
==========================================
- Hits 14009 14007 -2
- Misses 3863 3864 +1
- Partials 872 873 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.