Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

HttpHandlerDiagnosticListener: Write response event once when request is redirected #19195

Merged
merged 1 commit into from
May 1, 2017
Merged

HttpHandlerDiagnosticListener: Write response event once when request is redirected #19195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 41 additions & 8 deletions ...stem.Diagnostics.DiagnosticSource/src/System/Diagnostics/HttpHandlerDiagnosticListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,14 +603,33 @@ private void RaiseRequestEvent(HttpWebRequest request)

private void RaiseResponseEvent(HttpWebRequest request, HttpWebResponse response)
{
// Response event could be received several times for the same request
if (request.Headers[RequestIdHeaderName] != null)
// Response event could be received several times for the same request in case it was redirected
// IsLastResponse checks if response is the last one (no more redirects will happen)
// based on response StatusCode and number or redirects done so far
if (request.Headers[RequestIdHeaderName] != null && IsLastResponse(request, response))
{
// only send Stop if request was instrumented
this.Write(RequestStopName, new { Request = request, Response = response });
}
}

private bool IsLastResponse(HttpWebRequest request, HttpWebResponse response)
{
if (request.AllowAutoRedirect)
{
if (response.StatusCode == HttpStatusCode.Ambiguous || // 300
response.StatusCode == HttpStatusCode.Moved || // 301
response.StatusCode == HttpStatusCode.Redirect || // 302
response.StatusCode == HttpStatusCode.RedirectMethod || // 303
response.StatusCode == HttpStatusCode.RedirectKeepVerb) // 307
{
return s_autoRedirectsAccessor(request) >= request.MaximumAutomaticRedirections;
}
}

return true;
}

private static void PrepareReflectionObjects()
{
// At any point, if the operation failed, it should just throw. The caller should catch all exceptions and swallow.
Expand All @@ -624,26 +643,39 @@ private static void PrepareReflectionObjects()
s_writeListField = s_connectionType?.GetField("m_WriteList", BindingFlags.Instance | BindingFlags.NonPublic);

// Second step: Generate an accessor for HttpWebRequest._HttpResponse
FieldInfo field = typeof(HttpWebRequest).GetField("_HttpResponse", BindingFlags.NonPublic | BindingFlags.Instance);

string methodName = field?.ReflectedType.FullName + ".get_" + field.Name;
if (!string.IsNullOrEmpty(methodName))
FieldInfo responseField = typeof(HttpWebRequest).GetField("_HttpResponse", BindingFlags.NonPublic | BindingFlags.Instance);
if (responseField != null)
{
string methodName = responseField.ReflectedType.FullName + ".get_" + responseField.Name;
DynamicMethod getterMethod = new DynamicMethod(methodName, typeof(HttpWebResponse), new Type[] { typeof(HttpWebRequest) }, true);
ILGenerator generator = getterMethod.GetILGenerator();
generator.Emit(OpCodes.Ldarg_0);
generator.Emit(OpCodes.Ldfld, field);
generator.Emit(OpCodes.Ldfld, responseField);
generator.Emit(OpCodes.Ret);
s_httpResponseAccessor = (Func<HttpWebRequest, HttpWebResponse>)getterMethod.CreateDelegate(typeof(Func<HttpWebRequest, HttpWebResponse>));
}

// Third step: Generate an accessor for HttpWebRequest._AutoRedirects
FieldInfo redirectsField = typeof(HttpWebRequest).GetField("_AutoRedirects", BindingFlags.NonPublic | BindingFlags.Instance);
if (redirectsField != null)
{
string methodName = redirectsField.ReflectedType.FullName + ".get_" + redirectsField.Name;
DynamicMethod getterMethod = new DynamicMethod(methodName, typeof(int), new Type[] { typeof(HttpWebRequest) }, true);
ILGenerator generator = getterMethod.GetILGenerator();
generator.Emit(OpCodes.Ldarg_0);
generator.Emit(OpCodes.Ldfld, redirectsField);
generator.Emit(OpCodes.Ret);
s_autoRedirectsAccessor = (Func<HttpWebRequest, int>)getterMethod.CreateDelegate(typeof(Func<HttpWebRequest, int>));
}

// Double checking to make sure we have all the pieces initialized
if (s_connectionGroupListField == null ||
s_connectionGroupType == null ||
s_connectionListField == null ||
s_connectionType == null ||
s_writeListField == null ||
s_httpResponseAccessor == null)
s_httpResponseAccessor == null ||
s_autoRedirectsAccessor == null)
{
// If anything went wrong here, just return false. There is nothing we can do.
throw new InvalidOperationException("Unable to initialize all required reflection objects");
Expand Down Expand Up @@ -688,6 +720,7 @@ private static void PerformInjection()
private static Type s_connectionType;
private static FieldInfo s_writeListField;
private static Func<HttpWebRequest, HttpWebResponse> s_httpResponseAccessor;
private static Func<HttpWebRequest, int> s_autoRedirectsAccessor;

#endregion
}
Expand Down
59 changes: 26 additions & 33 deletions src/System.Diagnostics.DiagnosticSource/tests/HttpHandlerDiagnosticListenerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ public void TestReflectInitializationViaSubscription3()

// Just make sure some events are written, to confirm we successfully subscribed to it. We should have
// at least two events, one for request send, and one for response receive
Assert.True(eventRecords.Records.Count >= 2,
"Didn't get two or more events from Http Diagnostic Listener. Something is wrong.");
Assert.Equal(2, eventRecords.Records.Count);
}
}

Expand All @@ -103,7 +102,7 @@ public async Task TestBasicReceiveAndResponseEvents()
// Just make sure some events are written, to confirm we successfully subscribed to it. We should have
// at least two events, one for request send, and one for response receive
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key.EndsWith("Start")));
Assert.True(eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")) >= 1);
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")));

// Check to make sure: The first record must be a request, the next record must be a response.
// The rest is unknown number of responses (it depends on # of redirections)
Expand All @@ -119,13 +118,6 @@ public async Task TestBasicReceiveAndResponseEvents()
Assert.Equal("System.Net.Http.Desktop.HttpRequestOut.Stop", stopEvent.Key);
WebRequest stopRequest = ReadPublicProperty<WebRequest>(stopEvent.Value, "Request");
Assert.Equal(startRequest, stopRequest);

foreach (var pair in eventRecords.Records)
{
WebRequest thisRequest = ReadPublicProperty<WebRequest>(pair.Value, "Request");
Assert.Equal("System.Net.Http.Desktop.HttpRequestOut.Stop", pair.Key);
Assert.Equal(startRequest, thisRequest);
}
}
}

Expand Down Expand Up @@ -184,7 +176,7 @@ public async Task TestActivityIsCreated()
await new HttpClient().GetAsync("http://www.bing.com");

Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key.EndsWith("Start")));
Assert.True(eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")) >= 1);
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")));

WebRequest thisRequest = ReadPublicProperty<WebRequest>(eventRecords.Records.First().Value, "Request");
var requestId = thisRequest.Headers["Request-Id"];
Expand Down Expand Up @@ -285,7 +277,7 @@ public void TestDynamicPropertiesOnReceiveAndResponseEvents()
// Just make sure some events are written, to confirm we successfully subscribed to it. We should have
// at least two events, one for request send, and one for response receive
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key.EndsWith("Start")));
Assert.True(eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")) >= 1);
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")));


// Check to make sure: The first record must be a request, the last record must be a response. Records in
Expand Down Expand Up @@ -317,6 +309,7 @@ public void TestDynamicPropertiesOnReceiveAndResponseEvents()
[Fact]
public void TestMultipleConcurrentRequests()
{
ServicePointManager.DefaultConnectionLimit = Int32.MaxValue;
var parentActivity = new Activity("parent").Start();
using (var eventRecords = new EventObserverAndRecorder())
{
Expand Down Expand Up @@ -347,24 +340,28 @@ public void TestMultipleConcurrentRequests()

// Issue all requests simultaneously
HttpClient httpClient = new HttpClient();
List<Task<HttpResponseMessage>> tasks = new List<Task<HttpResponseMessage>>();

Dictionary<string, Task<HttpResponseMessage>> tasks = new Dictionary<string, Task<HttpResponseMessage>>();

CancellationTokenSource cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
foreach (string url in requestData.Keys)
{
tasks.Add(httpClient.GetAsync(url, cts.Token));
tasks.Add(url, httpClient.GetAsync(url, cts.Token));
}

Task.WaitAll(tasks.ToArray());
// wait up to 10 sec for all requests and suppress exceptions
Task.WhenAll(tasks.Select(t => t.Value).ToArray()).ContinueWith(tt => {}).Wait();

// Examine the result. Make sure we got them all.
// Examine the result. Make sure we got all successful requests.

// Just make sure some events are written, to confirm we successfully subscribed to it. We should have
// at least two events, one for request send, and one for response receive
Assert.Equal(requestData.Count, eventRecords.Records.Count(rec => rec.Key.EndsWith("Start")));
Assert.True(eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")) >= requestData.Count);
// exactly 1 Start event per request and exaclty 1 Stop event per response (if request succeeded)
var successfulTasks = tasks.Where(t => t.Value.Status == TaskStatus.RanToCompletion);

Assert.Equal(tasks.Count(), eventRecords.Records.Count(rec => rec.Key.EndsWith("Start")));
Assert.Equal(successfulTasks.Count(), eventRecords.Records.Count(rec => rec.Key.EndsWith("Stop")));

// Check to make sure: We have a WebRequest and a WebResponse for each URL
// Check to make sure: We have a WebRequest and a WebResponse for each successful request
foreach (var pair in eventRecords.Records)
{
object eventFields = pair.Value;
Expand Down Expand Up @@ -393,16 +390,9 @@ public void TestMultipleConcurrentRequests()
var childSuffix = requestId.Substring(0, parentActivity.Id.Length);
Assert.True(childSuffix.IndexOf('.') == childSuffix.Length - 1);

WebRequest previousSeenRequest = tuple?.Item1;
WebResponse previousSeenResponse = tuple?.Item2;

// We see have seen an HttpWebRequest before for this URL/host, make sure it's the same one,
// Then update the tuple with the request object, if we didn't have one
Assert.True(previousSeenRequest == null || previousSeenRequest == request,
"Didn't expect to see a different WebRequest object going to the same url host for: " +
request.RequestUri.OriginalString);
Assert.Null(requestData[request.RequestUri.OriginalString]);
requestData[request.RequestUri.OriginalString] =
new Tuple<WebRequest, WebResponse>(previousSeenRequest ?? request, previousSeenResponse);
new Tuple<WebRequest, WebResponse>(request, null);
}
else
{
Expand Down Expand Up @@ -431,12 +421,15 @@ public void TestMultipleConcurrentRequests()
}
}

// Finally, make sure we have request and response objects for every entry
// Finally, make sure we have request and response objects for every successful request
foreach (KeyValuePair<string, Tuple<WebRequest, WebResponse>> pair in requestData)
{
Assert.NotNull(pair.Value);
Assert.NotNull(pair.Value.Item1);
Assert.NotNull(pair.Value.Item2);
if (successfulTasks.Any(t => t.Key == pair.Key))
{
Assert.NotNull(pair.Value);
Assert.NotNull(pair.Value.Item1);
Assert.NotNull(pair.Value.Item2);
}
}
}
}
Expand Down