Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for a StoppingEvent on CollectTrace action #2557

Merged
merged 28 commits into from
Oct 11, 2022
Merged

Add support for a StoppingEvent on CollectTrace action #2557

merged 28 commits into from
Oct 11, 2022

Conversation

schmittjoseph
Copy link
Member

@schmittjoseph schmittjoseph commented Sep 27, 2022 •
edited
Loading

Summary

Add support for stopping a CollectTrace action when it observes a specific task name & opcode from an event provider. An optional payload filter can also be specified, which supports a subset of all possible fields on the specified event's payload.

Release Notes Entry

Add the option to specify a StoppingEvent on the CollectTrace action. The trace will be stopped once either the duration is reached or the specified event occurs.

@schmittjoseph schmittjoseph marked this pull request as ready for review September 27, 2022 16:22
@schmittjoseph schmittjoseph requested a review from a team as a code owner September 27, 2022 16:22
@schmittjoseph schmittjoseph added the update-release-notes Pull requests that should be mentioned in the release notes label Sep 28, 2022
jander-msft
jander-msft reviewed Oct 3, 2022
View reviewed changes
src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs Outdated Show resolved Hide resolved
src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/CollectTraceTests.cs Outdated Show resolved Hide resolved
src/Tests/Microsoft.Diagnostics.Monitoring.UnitTestApp/Scenarios/TraceEventsScenario.cs Outdated Show resolved Hide resolved
src/Tests/Microsoft.Diagnostics.Monitoring.UnitTestApp/Scenarios/TraceEventsScenario.cs Outdated Show resolved Hide resolved
src/Tests/Microsoft.Diagnostics.Monitoring.UnitTestApp/Scenarios/TraceEventsScenario.cs Outdated Show resolved Hide resolved
src/Tests/Microsoft.Diagnostics.Monitoring.UnitTestApp/Scenarios/TraceEventsScenario.cs Outdated Show resolved Hide resolved
src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs Outdated Show resolved Hide resolved
src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs Outdated Show resolved Hide resolved
wiktork
wiktork previously approved these changes Oct 5, 2022
View reviewed changes
@schmittjoseph schmittjoseph merged commit 1449f49 into dotnet:main Oct 11, 2022
@schmittjoseph schmittjoseph deleted the stop-trace-on-event branch October 11, 2022 20:07
@schmittjoseph schmittjoseph mentioned this pull request Oct 18, 2022
Merged
@schmittjoseph
Copy link
Member Author

/backport to release/7.x

@github-actions
Copy link
Contributor

Started backporting to release/7.x: https://github.com/dotnet/dotnet-monitor/actions/runs/3439602200

@github-actions
Copy link
Contributor

@schmittjoseph backporting to release/7.x failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch changes.patch

Applying: Add support for a StoppingEvent on CollectTrace action (#2557)
.git/rebase-apply/patch:142: trailing whitespace.
        
.git/rebase-apply/patch:158: trailing whitespace.
        
.git/rebase-apply/patch:167: trailing whitespace.
        
.git/rebase-apply/patch:176: trailing whitespace.
        
.git/rebase-apply/patch:614: trailing whitespace.
        
warning: squelched 2 whitespace errors
warning: 7 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	documentation/schema.json
M	src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs
M	src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs
M	src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx
M	src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TestAppScenarios.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.csproj
M	src/Tests/Microsoft.Diagnostics.Monitoring.UnitTestApp/Program.cs
M	src/Tools/dotnet-monitor/Strings.Designer.cs
M	src/Tools/dotnet-monitor/Strings.resx
Falling back to patching base and 3-way merge...
Auto-merging src/Tools/dotnet-monitor/Strings.resx
Auto-merging src/Tools/dotnet-monitor/Strings.Designer.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.UnitTestApp/Program.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests.csproj
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TestAppScenarios.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs
Auto-merging documentation/schema.json
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add support for a StoppingEvent on CollectTrace action (#2557)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

schmittjoseph added a commit to schmittjoseph/dotnet-monitor that referenced this pull request Nov 10, 2022
@schmittjoseph
Add support for a StoppingEvent on CollectTrace action (dotnet#2557)
fc5d75a
schmittjoseph added a commit to schmittjoseph/dotnet-monitor that referenced this pull request Nov 10, 2022
@schmittjoseph
Add support for a StoppingEvent on CollectTrace action (dotnet#2557)
5bcc383
@schmittjoseph schmittjoseph mentioned this pull request Nov 10, 2022
Merged
schmittjoseph added a commit that referenced this pull request Nov 11, 2022
@schmittjoseph
[release/7.x] Add support for a StoppingEvent on CollectTrace action (#…
55946db 
…2884)

* Add support for a StoppingEvent on CollectTrace action (#2557)

* Fix analyzer issues

* Revert VS automatic changes
@lateralusX lateralusX mentioned this pull request Aug 29, 2023
Closed
@mdh1418 mdh1418 mentioned this pull request Oct 25, 2023
Merged
hoyosjs pushed a commit to dotnet/diagnostics that referenced this pull request Nov 2, 2023
@mdh1418
[tools][trace] Add stopping event options to dotnet-trace (#4363)
66f7ca0 
Fixes #3125

This PR provides users another method to stop a dotnet-trace via a
stopping event similar to that of dotnet-monitor, originally implemented
in dotnet/dotnet-monitor#2557.

Three arguments are added to the `dotnet-trace collect` command to
specify a stopping event:
| Argument | Description |
|----------|----------|
|`--stopping-event-provider-name` | A string, parsed as-is, that will
stop the trace upon hitting an event with the matching provider name.
For a more specific stopping event, additionally provide
`--stopping-event-event-name` and/or `--stopping-event-payload-filter`.
|
| `--stopping-event-event-name` | A string, parsed as-is, that will stop
the trace upon hitting an event with the matching event name. Requires
`--stopping-event-provider-name` to be set. For a more specific stopping
event, additionally provide `--stopping-event-payload-filter`. |
| `--stopping-event-payload-filter` | A string, parsed as
[payload_field_name]:[payload_field_value] pairs separated by commas,
that will stop the trace upon hitting an event with a matching payload.
Requires `--stopping-event-provider-name` and
`--stopping-event-event-name` to be set. |

Note: Though technically `--stopping-event-payload-filter` can be set
without needing a `--stopping-event-event-name`, this may lead to
mismatched payloads should another `TraceEvent` under the same provider
not have that particular payload field name. Until there is a good
reason to stop a trace given a payload filter regardless of the event
name, we require `--stopping-event-event-name` to be set whenever
`--stopping-event-payload-filter` is provided.

To stop a trace at a particular event, dotnet-monitor's
[approach](https://github.com/dotnet/dotnet-monitor/blob/0820b6911f3ac47b6b5ec867ac906699e5c15787/src/Tools/dotnet-monitor/Trace/TraceUntilEventOperation.cs#L47)
using an
[EventMonitor](https://github.com/dotnet/diagnostics/blob/main/src/Microsoft.Diagnostics.Monitoring.EventPipe/EventMonitor.cs)
is adopted. Upon hitting a TraceEvent with a matching ProviderName,
EventName (if specified), and PayloadFilter (if specified), we trigger
dotnet-trace's fallback logic to stop the EventPipeSession before the
EventStream ends.

Note: As the EventStream is being parsed asynchronously, there will be
some events that pass through between the time a trace event matching
the specified stopping event arguments is parsed and the
EventPipeSession is stopped.

In addition, this PR modifies `EventMonitor` to use the
`ClrTraceEventParser` to parse `TraceEvent` objects under the
`Microsoft-Windows-DotNETRuntime` provider, and the
`DynamicTraceEventParser` otherwise. The `ClrTraceEventParser` is
generated to understand the ETW event manifest for
`Microsoft-Windows-DotNETRuntime` events. The previous implementation
defaulting to `DynamicTraceEventParser` would not work on non-Windows
platforms such as OSX which could not parse the payload to populate
`PayloadNames` and `PayloadValue(i)` because there was no manifest
available. On the other hand, Windows is able to locate manifest data
for known events through the OS.

-------------------

## Testing

With an Android App
``` C#
    private static void PrintA()
    {
        Console.WriteLine("A");
        Thread.Sleep(1000);
    }
    
    ...
    
    private static void PrintK()
    {
        Console.WriteLine("K");
        Thread.Sleep(1000);
    }

    public static void Main(string[] args)
    {
        Console.WriteLine("Hello, Android!"); // logcat
        PrintA();
        PrintB();
        PrintC();
        PrintD();
        PrintE();
        PrintF();
        PrintG();
        PrintH();
        PrintI();
        PrintJ();
        PrintK();

        while (true)
        {
            Thread.Sleep(100);
        }
    }
```

Running dotnet-dsrouter to connect the diagnostic tooling with the
android device
`./artifacts/bin/dotnet-dsrouter/Debug/net6.0/dotnet-dsrouter android -v
debug`

Tracing with a stopping event args provided
`./artifacts/bin/dotnet-trace/Debug/net6.0/dotnet-trace collect -p 16683
--providers Microsoft-Windows-DotNETRuntime:0x1F000080018:5
--stopping-event-provider-name Microsoft-Windows-DotNETRuntime
--stopping-event-event-name Method/JittingStarted
--stopping-event-payload-filter MethodName:PrintA`


[dotnet-dsrouter_20231024_165648.nettrace.zip](https://github.com/dotnet/diagnostics/files/13147788/dotnet-dsrouter_20231024_165648.nettrace.zip)

There are no `Method/JittingStarted` events with MethodName `PrintC`
through `PrintK` in the `.nettrace`, showing that the trace was indeed
stopped after seeing the `PrintA` event. The events after `PrintA` are
an artifact of the second note above. Below is the JITStats of the
.nettrace opened in perfview, showing that the last method was `PrintB`.

<img width="1128" alt="JITStatsPrintA"
src="https://github.com/dotnet/diagnostics/assets/16830051/1742baf4-b528-43c3-aef3-b00a576f2fb8">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jander-msft jander-msft jander-msft approved these changes

@wiktork wiktork Awaiting requested review from wiktork

Assignees
No one assigned
Labels
update-release-notes Pull requests that should be mentioned in the release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schmittjoseph @wiktork @jander-msft