Fix DiagnosticSource + Disposable issues + service name doc #75
Conversation
This file contains 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
In order to make the API more simple we autosubscribe to outgoing HTTP calls and EFCore calls. Also added 2 tests to test non-ASP.NET Core scenarios + HttpDiagnosticsSubscriber registration.
And removed unused `using`s
With this unit tests can dispose the agent, which automatically unsubscribes from all diagnosticsource events.
Codecov Report
@@ Coverage Diff @@
## master #75 +/- ##
=========================================
+ Coverage 80.53% 81.33% +0.8%
=========================================
Files 34 35 +1
Lines 904 943 +39
Branches 115 119 +4
=========================================
+ Hits 728 767 +39
+ Misses 131 129 -2
- Partials 45 47 +2
Continue to review full report at Codecov.
|
|
Just for reference: the coverage report is probably wrong. @kuisathaverat is on it... |
|
@bmorelli25 |
gregkalapos
changed the title
Mpdreamz
requested changes
Jan 22, 2019
Also removed IDisposable from the Listeners
bmorelli25
requested changes
Jan 22, 2019
Elastic.Apm.All has a reference to all other agent projects and it contains convenient methods to activate the agent very easily and enable each agent component.
bmorelli25
approved these changes
Jan 22, 2019
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.
Added service name doc
Some refactorings:
TheApmMiddlewareExtensionautomatically subscribes to EFCore and HttpClient events. Additionally, theElastic.Apm.AspNetCorenow references theElastic.Apm.EntityFrameworkCoreproject.With this, registering the agent in ASP.NET Core became simpler:
app.UseElasticApm(Configuration);This was the old code:
`app.UseElasticApm(Configuration, new HttpDiagnosticsSubscriber(), new EfCoreDiagnosticsSubscriber());
Update:
Added
Elastic.Apm.Allproject that references every other agent project and contains anApmMiddlewareExtensionthat automatically subscribes to HttpClient and EFCore events. This way in typical ASP.NET Core apps (ASP.NET Core + EF Core) that referenceMicrosoft.AspNetCore.Allusers can just reference ourElastic.Apm.Allpackage and enable the agent with all its features (including EF Core and HttpClient monitoring) withapp.UseElasticApm(Configuration);. In other cases (e.g. ASP.NET Core without EF Core, or even no ASP.NET Core) the other packages can be referenced individually to avoid unnecessary dependencies. In those scenarios the diagnostic listeners must be enabled manually.Moved
TransactionContainerintoAgentComponents. With thatTransactionContainer.TransactionsbecameAgent.TransactionContainer, which we can reset in tests.Fix for Bug: Unsubscribing from DiagnosticSource events does not work #72.
As a consequence of fixing Bug: Unsubscribing from DiagnosticSource events does not work #72 the
ApmAgentalso became Disposable. With this in tests we can dispose the agent.