Skip to content

Support resetting state when connecting to a previously monitored app #8154

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.

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

Merged
merged 15 commits into from
May 6, 2025

Conversation

wiktork
Copy link
Member

@wiktork wiktork commented Apr 25, 2025

Summary

Scenario
Dotnet-monitor profiling/exceptions/parameter capture is installed in an application. Then dotnet-monitor is terminated but the application is not. Dotnet-monitor is then restarted and is once again observing the same application.

Startuphook/profiler changes

  • Startup hooks only install once. This behavior has been maintained.
  • Profiler previously attempted to install again. We now detect that a profiler is already installed and reuse that channel.

Command handling
There are 3 threads that handle commands:

  • The connection thread, listening for a connection with the client. The message is processed and dispatched as appropriate:
  • For a simple message such as callstack collection, the message is sent to the native queue.
  • For startup hook messages (such as exceptions/parameter capture), the message is sent to the managed queue to be picked up by the startup hook for further processing
  • The new reset message. This message sends stop/start messages to the native and managed queues above. These effectively block until a response is received from the stop calls to ensure that all cached data has been cleared.

Exception reset

  • Exceptions rely on an event handler and occur synchronously on any thread.
  • For stopping, we first unhook from the exception handler. Then we synchronously wait for all outstanding exception processing to finish. Note the client is awaiting these operations to finish and will not finish the profiler initialization until then.
  • After all the data drained, we reset the pipelines and attach the event handler.

Parameters capture reset

  • We request that all outstanding requests are turned off. No further synchronization is necessary because parameter capturing has a concept of RequestId that matches up requests with returned data. So if a new client received old data, it simply discards it.

Callstacks

  • No action is needed, but we could further optimize by cancelling outstanding stack walks.

TODO:

  • Figure out a way to call stop when the client is disconnected and no new client is started.
  • Ideally this can be used for independent stop/start calls in the future. However, this will require additional changes to the client for exceptions.
  • Add tests for reset scenario.
Release Notes Entry

Added support for resetting in process features when dotnet-monitor is restarted.

@wiktork wiktork marked this pull request as ready for review April 30, 2025 17:44
@wiktork wiktork requested a review from a team as a code owner April 30, 2025 17:44
@wiktork wiktork force-pushed the dev/wiktork/reenable branch from 5c9a410 to c619266 Compare May 2, 2025 06:12
schmittjoseph
schmittjoseph previously approved these changes May 5, 2025
Copy link
Member

@schmittjoseph schmittjoseph left a comment

Choose a reason for hiding this comment

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

Signing off on the parameter capturing + profiler changes. Be sure to add a release notes entry and the relevant PR label

@wiktork wiktork dismissed stale reviews from jander-msft and schmittjoseph via 4fa3bb7 May 5, 2025 22:47
@wiktork wiktork force-pushed the dev/wiktork/reenable branch from 4fa3bb7 to 43ea29f Compare May 5, 2025 22:47
jander-msft
jander-msft previously approved these changes May 6, 2025
@wiktork wiktork force-pushed the dev/wiktork/reenable branch from c61ce14 to a2c284d Compare May 6, 2025 20:31
@wiktork wiktork merged commit 5baeadb into dotnet:main May 6, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants