Skip to content

Commit

Permalink
[Dotnet Monitor] Standardize DiagnosticFilterString, No Longer Assume…
Browse files Browse the repository at this point in the history 
… TraceEvent Arg Ordering (#3425)

* Set AspNetTriggerSourceConfiguration's DiagnosticFilterString to be the same as the one in HttpRequestSourceConfiguration.

* Updated diagnostic filter string; getting arguments by name (instead of index)

* Small tweaks

* Spacing

* Update src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/HttpRequestSourceConfiguration.cs

Co-authored-by: Justin Anderson <[email protected]>

* Tweaks for Wiktor

Co-authored-by: Justin Anderson <[email protected]>
  • Loading branch information
@mikem8361 @jander-msft
mikem8361 and jander-msft committed Oct 18, 2022
1 parent 668845a commit c937556
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 42 deletions.
23 changes: 1 addition & 22 deletions ...rosoft.Diagnostics.Monitoring.EventPipe/Configuration/AspNetTriggerSourceConfiguration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,6 @@ public AspNetTriggerSourceConfiguration(float? heartbeatIntervalSeconds = null)
_heartbeatIntervalSeconds = heartbeatIntervalSeconds;
}

/// <summary>
/// Filter string for trigger data. Note that even though some triggers use start OR stop,
/// collecting just one causes unusual behavior in data collection.
/// </summary>
/// <remarks>
/// IMPORTANT! We rely on these transformations to make sure we can access relevant data
/// by index. The order must match the data extracted in the triggers.
/// </remarks>
private const string DiagnosticFilterString =
"Microsoft.AspNetCore/Microsoft.AspNetCore.Hosting.HttpRequestIn.Start@Activity1Start:-" +
"ActivityId=*Activity.Id" +
";Request.Path" +
";ActivityStartTime=*Activity.StartTimeUtc.Ticks" +
"\r\n" +
"Microsoft.AspNetCore/Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop@Activity1Stop:-" +
"ActivityId=*Activity.Id" +
";Request.Path" +
";Response.StatusCode" +
";ActivityDuration=*Activity.Duration.Ticks" +
"\r\n";

public override IList<EventPipeProvider> GetProviders()
{
if (_heartbeatIntervalSeconds.HasValue)
Expand All @@ -62,7 +41,7 @@ public override IList<EventPipeProvider> GetProviders()
eventLevel: EventLevel.Verbose,
arguments: new Dictionary<string,string>
{
{ "FilterAndPayloadSpecs", DiagnosticFilterString }
{ "FilterAndPayloadSpecs", HttpRequestSourceConfiguration.DiagnosticFilterString }
})
};
}
Expand Down
10 changes: 7 additions & 3 deletions ...icrosoft.Diagnostics.Monitoring.EventPipe/Configuration/HttpRequestSourceConfiguration.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ public HttpRequestSourceConfiguration()
RequestRundown = true;
}

private const string DiagnosticFilterString =
// This string is shared between HttpRequestSourceConfiguration and AspNetTriggerSourceConfiguration
// due to an issue that causes the FilterAndPayloadSpecs to be overwritten but never reverted.
// This caused http traces to interfere with AspNet* triggers due to having different arguments.
internal const string DiagnosticFilterString =
"Microsoft.AspNetCore/Microsoft.AspNetCore.Hosting.HttpRequestIn.Start@Activity1Start:-" +
"Request.Scheme" +
";Request.Host" +
Expand All @@ -33,9 +36,10 @@ public HttpRequestSourceConfiguration()
";ActivityIdFormat=*Activity.IdFormat" +
"\r\n" +
"Microsoft.AspNetCore/Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop@Activity1Stop:-" +
"Response.StatusCode" +
"ActivityId=*Activity.Id" +
";Request.Path" +
";Response.StatusCode" +
";ActivityDuration=*Activity.Duration.Ticks" +
";ActivityId=*Activity.Id" +
"\r\n" +
"HttpHandlerDiagnosticListener/System.Net.Http.HttpRequestOut@Event:-" +
"\r\n" +
Expand Down
50 changes: 33 additions & 17 deletions src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/AspNet/AspNetTrigger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using Microsoft.Diagnostics.Tracing;
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel.DataAnnotations;
using System.Diagnostics;
using System.Linq;
Expand All @@ -17,6 +16,10 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe.Triggers.AspNet
/// </summary>
internal abstract class AspNetTrigger<TSettings> : ITraceEventTrigger where TSettings : AspNetTriggerSettings
{
private const string ActivityId = "activityid";
private const string Path = "path";
private const string StatusCode = "statuscode";
private const string ActivityDuration = "activityduration";
private const string Activity1Start = "Activity1/Start";
private const string Activity1Stop = "Activity1/Stop";
private static readonly Guid MicrosoftAspNetCoreHostingGuid = new Guid("{adb401e1-5296-51f8-c125-5fda75826144}");
Expand Down Expand Up @@ -61,16 +64,41 @@ public bool HasSatisfiedCondition(TraceEvent traceEvent)
{
int? statusCode = null;
long? duration = null;
string activityId = string.Empty;
string path = string.Empty;

AspnetTriggerEventType eventType = AspnetTriggerEventType.Start;

System.Collections.IList arguments = (System.Collections.IList)traceEvent.PayloadValue(2);
string activityId = ExtractByIndex(arguments, 0);
string path = ExtractByIndex(arguments, 1);

foreach (var argument in arguments)
{
if (argument is IEnumerable<KeyValuePair<string, object>> argumentsEnumerable)
{
string key = (string)argumentsEnumerable.First().Value;
string value = (string)argumentsEnumerable.Last().Value;

if (key.Equals(ActivityId, StringComparison.OrdinalIgnoreCase))
{
activityId = value;
}
else if (key.Equals(Path, StringComparison.OrdinalIgnoreCase))
{
path = value;
}
else if (key.Equals(StatusCode, StringComparison.OrdinalIgnoreCase))
{
statusCode = int.Parse(value);
}
else if (key.Equals(ActivityDuration, StringComparison.OrdinalIgnoreCase))
{
duration = long.Parse(value);
}
}
}

if (traceEvent.EventName == Activity1Stop)
{
statusCode = int.Parse(ExtractByIndex(arguments, 2));
duration = long.Parse(ExtractByIndex(arguments, 3));
eventType = AspnetTriggerEventType.Stop;

Debug.Assert(statusCode != null, "Status code cannot be null.");
Expand All @@ -82,7 +110,6 @@ public bool HasSatisfiedCondition(TraceEvent traceEvent)

//Heartbeat only
return HasSatisfiedCondition(timeStamp, eventType: AspnetTriggerEventType.Heartbeat, activityId: null, path: null, statusCode: null, duration: null);

}

/// <summary>
Expand Down Expand Up @@ -111,17 +138,6 @@ internal bool HasSatisfiedCondition(DateTime timestamp, AspnetTriggerEventType e
}
return false;
}

private static string ExtractByIndex(System.Collections.IList arguments, int index)
{
IEnumerable<KeyValuePair<string, object>> values = (IEnumerable<KeyValuePair<string, object>>)arguments[index];
//The data is internally organized as two KeyValuePair entries,
//The first entry is { Key, "KeyValue"}
//The second is { Value, "Value"}
//e.g.
//{{ Key:"StatusCode", Value:"200" }}
return (string)values.Last().Value;
}
}

internal enum AspnetTriggerEventType
Expand Down

0 comments on commit c937556

Please sign in to comment.