Skip to content
This repository has been archived by the owner on Jul 5, 2020. It is now read-only.

Detect issue with POST/PUT/PATCH requests correlation and report in event source actionable event #903

Merged
merged 2 commits into from
May 11, 2018

Conversation

lmolkova
Copy link
Member

@lmolkova lmolkova commented May 3, 2018

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.

  • I ran Unit Tests locally.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated with one line description of the fix, and a link to the original issue.

@lmolkova lmolkova force-pushed the lmolkova/correlationIssueHeartbeat branch from 0b2afe0 to 83ef245 Compare May 3, 2018 23:43
Copy link
Contributor

@cijothomas cijothomas left a 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";
Copy link
Contributor

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;
Copy link
Contributor

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 ?

@lmolkova
Copy link
Member Author

lmolkova commented May 4, 2018

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.
We can guess that there are not much of such customers, but I'd prefer to measure it rather than guess.

@lmolkova lmolkova requested a review from TimothyMothra May 7, 2018 17:51
@lmolkova
Copy link
Member Author

lmolkova commented May 7, 2018

@cijothomas, are you ok with it?

@lmolkova lmolkova force-pushed the lmolkova/correlationIssueHeartbeat branch 2 times, most recently from 9c9d489 to 066d003 Compare May 10, 2018 18:49
@lmolkova lmolkova force-pushed the lmolkova/correlationIssueHeartbeat branch from 066d003 to 9613422 Compare May 10, 2018 18:53
@lmolkova lmolkova changed the title Detect issue with POST/PUT/PATCH requests correlation and report in heartbeat Detect issue with POST/PUT/PATCH requests correlation and report in event source actionable event May 10, 2018
@lmolkova
Copy link
Member Author

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 cijothomas closed this May 11, 2018
@cijothomas cijothomas reopened this May 11, 2018
@lmolkova
Copy link
Member Author

@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)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Error

@lmolkova
Copy link
Member Author

build issue is transient, related to artifacts publishing and does not repro on a manually triggered build.

@lmolkova lmolkova merged commit db6a223 into develop May 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants