Report intermediate requests for lost and restored Activity#898
Report intermediate requests for lost and restored Activity#898
Conversation
Src/Web/Web.Nuget/Package.nuspec
Outdated
| <dependency id="Microsoft.ApplicationInsights" version="[$coresdkversion$]" /> | ||
| <dependency id="Microsoft.ApplicationInsights.WindowsServer" version="$version$" /> | ||
| <dependency id="Microsoft.AspNet.TelemetryCorrelation" version="1.0.1" /> | ||
| <dependency id="Microsoft.AspNet.TelemetryCorrelation" version="1.0.2" /> |
There was a problem hiding this comment.
nuspec and packages config should be aligned on the required version of this package
SergeyKanzhelev
left a comment
SergeyKanzhelev
left a comment
There was a problem hiding this comment.
Will this logic work when TelemetryCorrelation will be fixed? I mean when customer updates TelemetryCorrelation http module, but does not upgrade Application Insights?
Yes, when TelemetryCorrelation will start setting Activity.Current (instead of creating child Activity), ApplicaitonInsights without any change would pick it up. |
|
@MS-TimothyMothra @cijothomas could you please have a look? |
lmolkova
force-pushed
the
lmolkova/ActivityPost
branch
from
d0a7df9 to
41813f9
Compare
| string restoredActivityId = null; | ||
| using (var httpClient = new HttpClient()) | ||
| { | ||
| var request = new HttpRequestMessage(HttpMethod.Post, string.Format($"http://{Apps[TestConstants.WebApiName].ipAddress}/api/values")); |
There was a problem hiding this comment.
Please add a comment above this line to indicate that this Post request will trigger a StartOperation in the testapp. (to make it clear how dependeny item is produced by not making an actual http outbound request)
| set BuildRoot=%~dp0..\bin\Release\ | ||
| set VSTestPath=%PROGRAMFILES(x86)%\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\CommonExtensions\Microsoft\TestWindow\vstest.console.exe | ||
|
|
||
| CALL "%VSTestPath%" "%BuildRoot%Test\E2ETests\E2ETests\DependencyCollectionTests.dll" /TestCaseFilter:"TestCategory=Core20" /logger:trx |
There was a problem hiding this comment.
did you intend to make this change? or accidently checked in!
There was a problem hiding this comment.
oh, did not intend to remove those
| intermediateRequest.Context.Operation.Id = activity.RootId; | ||
| intermediateRequest.Context.Operation.ParentId = activity.ParentId; | ||
| intermediateRequest.ResponseCode = null; | ||
| intermediateRequest.Properties.Add("AI internal", "Execute request handler step"); |
There was a problem hiding this comment.
this is a hint to users that it is some kind of technical, internal AI stuff that leaked into their telemetry, rather than a real request.
4 participants
Fix Issue #797 for the case when .NET 4.7.1 is installed on the machine (Azure WebApps case).
When request processing starts, Microsoft.AspNet.TelemetryCorrelation module creates a new Activity and fires Start Event. ApplicationInsights receives this event and creates RequestTelmetery.
RequestTelemetry gets all correlation Ids from the Activity module created. (Nothing has changed here)
Right before the ASP.NET (or WCF) phase in request processing, the module checks if Activity was lost (it happens in some cases including POST requests). In versions 1.0.0-1.0.1 TelemetryCorrelation module attempted to restore an Activity by creating a child one and did not notify AI about it.
It resulted in following E2E viewer behavior:
As the request is a grand-parent of dependency, and nothing has reported in the middle, a correct correlation between them is not possible.
It also sens a new event about such activity, so we can track intermediate request and build better UI .
This is temporary view, until dotnet/corefx#29208 is released along with .NET Core 2.2. At this point, TelemetryCorrelation will be able to assign
Activity.Currentand dependency would be a direct child of the request.For significant contributions please make sure you have completed the following items:
Next steps (not part of this PR):