-
Notifications
You must be signed in to change notification settings - Fork 206
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
Expose Transaction.Context.Request and Transaction.Context.Response #134
Conversation
Codecov Report
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 82.69% 83.05% +0.36%
==========================================
Files 36 37 +1
Lines 1144 1151 +7
Branches 183 184 +1
==========================================
+ Hits 946 956 +10
Misses 125 125
+ Partials 73 70 -3
Continue to review full report at Codecov.
|
Reverted Reason: This value-behaviour and forcing users to set everything at once would be strange in C#. 😢 |
Assert.True(payloadSender.FirstTransaction.Context.Response.Finished); | ||
Assert.Equal(200, payloadSender.FirstTransaction.Context.Response.StatusCode); | ||
} | ||
|
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.
Maybe it's worth adding a test that sets a property of Request/Response object after construction.
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.
👍 Added a test doing that.
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.
LGTM
* move assertions over to fluent assertions * Add extensions for should duration assertions * move new Asserts over after rebase
This reverts commit 5ba803e.
This reverts commit 842eda0.
Also remove some unused variables
…agent-dotnet into ExposeContextV2
Potential solution to #124 - 2. Version, follow-up from #130
Types that we make public moved from
Elastic.Apm.Model.Payload
toElastic.Apm.Api
.This makes both
Context.Request
andContext.Response
public.I also madeUpdate: reverted, we useRequest
,Response
,Url
, andSocket
to astruct
.class
.With this dangerous code like this won't compile:And code like this will work and won't throw, even if Request is not initialized:This'll also work and not throw:I'll follow up on this, maybe we'll need to adjust our serialization to this, but I think it's worth it.