Skip to content
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

Fix ParentSampledId being reset on Transaction #1130

Merged
merged 4 commits into from
Jul 17, 2021

Conversation

Tyrrrz
Copy link
Contributor

@Tyrrrz Tyrrrz commented Jul 16, 2021

Closes #1097

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2021

Codecov Report

Merging #1130 (d99b2ab) into main (fae3886) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1130   +/-   ##
=======================================
  Coverage   79.15%   79.15%           
=======================================
  Files         200      200           
  Lines        6614     6615    +1     
  Branches     1460     1460           
=======================================
+ Hits         5235     5236    +1     
  Misses        913      913           
  Partials      466      466           
Impacted Files Coverage Δ
src/Sentry/Transaction.cs 81.33% <100.00%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fae3886...d99b2ab. Read the comment docs.

transaction.ParentSpanId.Should().Be(SpanId.Parse("1000000000000000"));
transaction.IsSampled.Should().BeFalse();
sentryClient.Received(1).CaptureTransaction(Arg.Is<Transaction>(t =>
t.Name == "GET /person/{id}" &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was trasnaction?. and transaction was null so assert wasn't being called?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it was fine. The problem is that the transaction was created correctly, it was not sent correctly. So I widened the test scope to cover that (by testing against ISentryClient).

@bruno-garcia bruno-garcia enabled auto-merge (squash) July 17, 2021 18:11
@bruno-garcia bruno-garcia merged commit 68dd94d into main Jul 17, 2021
@bruno-garcia bruno-garcia deleted the fix-aspnetcore-traceid-propagation branch July 17, 2021 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sentry-trace ParentSpanId being lost from the ASP.NET Core Transaction
3 participants