-
Notifications
You must be signed in to change notification settings - Fork 67
Detect issue with POST/PUT/PATCH requests correlation and report in event source actionable event #903
Conversation
0b2afe0
to
83ef245
Compare
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.
I am not sure if sending additional this info via HB is really required. Perhaps just use the .net framework version already calculated by HB module, and use it.. Or send a new heartbeat with precise .netframework/asp.netruntime information.
Telemetry like that will be more valuable to us and for customers as well.
@@ -19,12 +20,16 @@ | |||
/// </summary> | |||
public class RequestTrackingTelemetryModule : ITelemetryModule | |||
{ | |||
private readonly IList<string> handlersToFilter = new List<string>(); | |||
private const string CorrelationStatusHeartbeatProperty = "CorrelationHealthStatus_RequestWithContent"; |
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.
Lets use the minimum required characters. "CorHS_ReqWCont"
"NA" instead of notsupported :) to minimize payload size
private const string CorrelationStatusHeartbeatProperty = "CorrelationHealthStatus_RequestWithContent"; | ||
|
||
// if HttpApplicaiton.OnRequestExecute is available, we don't attempt to detect any correlation issues | ||
private static bool correlationIssuesDetectionComplete = typeof(HttpApplication).GetMethod("OnExecuteRequestStep") != null; |
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.
Is there an exact version of .net framework when this will be available? I believe heartbeat already sents out .NET Framework information - can we use that alone ?
As we have seen in func tests, .NET version does not really matter. What matters is the version of ASP.NET and even then, customers that do not have POST, PUT or PATCH requests (e.g. serving static websites) would never experience the issue. |
@cijothomas, are you ok with it? |
9c9d489
to
066d003
Compare
066d003
to
9613422
Compare
discussed with @Dmitry-Matveev and @cijothomas user actionable event source event is typical way to achieve it and could be checked from internal telemetry and customer telemetry. And there is no need to build additional UX features for it. |
@cijothomas ping :) |
[Event(49, | ||
Keywords = Keywords.Diagnostics | Keywords.UserActionable, | ||
Message = ".NET 4.7.1 is not installed, correlation for HTTP requests with body is not possible", | ||
Level = EventLevel.Warning)] |
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.
Error
build issue is transient, related to artifacts publishing and does not repro on a manually triggered build. |
See #797.
We have fixed the issue with #898 for users that have .NET 4.7.1 installed (all Azure WebApp users).
Azure Cloud Services do not have 4.7.1 by default.
For users that do not have 4.7.1 we recommend installing .NET 4.7.1 (without retargeting the app) or follow workaround mentioned in the #797.
We also want to see how many users are affected by the issue, so we detect it and
set heartbeat property[Updated] send user actionable event source event.We'll see if need to notify users in UI based on the number of affected users. For now, we just ensuring data is there.
For significant contributions please make sure you have completed the following items: