Skip to content

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
schmittjoseph committed Oct 10, 2022
1 parent ebc9c63 commit 986ae74
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi
/// A stream that can monitor an event stream which is compatible with <see cref="EventPipeEventSource"/>
/// for a specific event while also passing along the event data to a destination stream.
/// </summary>
public class EventMonitoringPassthroughStream : Stream
public sealed class EventMonitoringPassthroughStream : Stream
{
private readonly Action<TraceEvent> _onPayloadFilterMismatch;
private readonly Action<TraceEvent> _onEvent;
Expand Down Expand Up @@ -47,7 +47,7 @@ public class EventMonitoringPassthroughStream : Stream
/// <param name="onEvent">A callback that will be invoked each time the requested event has been observed.</param>
/// <param name="onPayloadFilterMismatch">A callback that will be invoked if the field names specified in <paramref name="payloadFilter"/> do not match those in the event's manifest.</param>
/// <param name="sourceStream">The source event stream which is compatible with <see cref="EventPipeEventSource"/>.</param>
/// <param name="destinationStream">The destination stream to write events.</param>
/// <param name="destinationStream">The destination stream to write events. It must either be full duplex or be write-only.</param>
/// <param name="bufferSize">The size of the buffer to use when writing to the <paramref name="destinationStream"/>.</param>
/// <param name="callOnEventOnlyOnce">If true, the provided <paramref name="onEvent"/> will only be called for the first matching event.</param>
/// <param name="leaveDestinationStreamOpen">If true, the provided <paramref name="destinationStream"/> will not be automatically closed when this class is.</param>
Expand Down Expand Up @@ -102,19 +102,11 @@ public Task ProcessAsync(CancellationToken token)
/// <summary>
/// Stops monitoring for the specified stopping event. Data will continue to be written to the provided destination stream.
/// </summary>
public void StopMonitoringForEvent()
private void StopMonitoringForEvent()
{
_eventSource?.Dynamic.RemoveCallback<TraceEvent>(TraceEventCallback);
}

/// <summary>
/// Stops processing the event data, data will no longer be written to the provided destination stream.
/// </summary>
public void StopProcessing()
{
_eventSource?.StopProcessing();
}

private void TraceEventCallback(TraceEvent obj)
{
if (_payloadFilterIndexCache == null && !HydratePayloadFilterCache(obj))
Expand All @@ -140,19 +132,28 @@ private void TraceEventCallback(TraceEvent obj)
_onEvent(obj);
}

/// <summary>
/// Hydrates the payload filter cache.
/// </summary>
/// <param name="obj">An instance of the stopping event (matching provider, task name, and opcode), but without checking the payload yet.</param>
/// <returns></returns>
private bool HydratePayloadFilterCache(TraceEvent obj)
{
if (_payloadFilterIndexCache != null)
{
return true;
}

// If there's no payload filter, there's nothing to do.
if (_payloadFilter == null || _payloadFilter.Count == 0)
{
_payloadFilterIndexCache = new(capacity: 0);
return true;
}

// If the payload has fewer fields than the requested filter, we can never match it.
// NOTE: this function will only ever be called with an instance of the stopping event
// (matching provider, task name, and opcode) but without checking the payload yet.
if (obj.PayloadNames.Length < _payloadFilter.Count)
{
return false;
Expand All @@ -167,6 +168,7 @@ private bool HydratePayloadFilterCache(TraceEvent obj)
}
}

// Check if one or more of the requested filter field names did not exist on the actual payload.
if (_payloadFilter.Count != payloadFilterCache.Count)
{
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi
/// <summary>
/// Wraps a given stream but leaves it open on Dispose.
/// </summary>
public class StreamLeaveOpenWrapper : Stream
public sealed class StreamLeaveOpenWrapper : Stream
{
private readonly Stream _baseStream;

Expand Down

0 comments on commit 986ae74

Please sign in to comment.