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.

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

Support gracefully stopping traces on-demand #2828

Merged
merged 48 commits into from
Nov 8, 2022

Conversation

schmittjoseph
Copy link
Member

@schmittjoseph schmittjoseph commented Nov 2, 2022

Summary

This PR introduces support for stopping a requested trace on demand while also collecting rundown, regardless of if an egress provider is specified. The following high-level changes were made to support this:

  • Operations now have a new Stopping state. This state indicates that the user requested the operation to stop, but it will take an indeterminate amount of time. Operations in this state can still be immediately cancelled.
    • To help users distinguish what operations can, and cannot, be gracefully stopped a new isStoppable property is added onto operations.
  • For API consistency, all API routes that supported sending the artifact back via the HTTP response now also record it in the operations table and supply the operations URL in the Location header.
    • To help users distinguish between operations, the egressProviderName is now recorded and reported for each operation.
    • As a result, request limit handling is now entirely handled by the operations table. There is no longer a need for the request tracking middleware or attributes.
  • Http response egressed based operations are now plumbed into the egress operations service. Using this approach, we can ensure operation state consistency since these operations have multiple ways of being cancelled or otherwise controlled. e.g. an HTTP based response can be cancelled via the operations API, or by the user stopping the request.
Release Notes Entry

Add support for gracefully stopping a requested trace on demand while also collecting rundown, via Stop Operation. All artifact requests are now also recorded under the Operations API, regardless of if they have an egress provider specified.

@schmittjoseph schmittjoseph added the update-release-notes Pull requests that should be mentioned in the release notes label Nov 2, 2022
@schmittjoseph schmittjoseph changed the title draft: Support gracefully stopping traces on-demand Support gracefully stopping traces on-demand Nov 2, 2022
@jander-msft jander-msft merged commit 95205d0 into dotnet:main Nov 8, 2022
@jander-msft jander-msft added the servicing-major Servicing fixes that is targeted for a major release (x.0.0 version) label Nov 10, 2022
@schmittjoseph schmittjoseph deleted the stop-trace-on-demand branch November 10, 2022 18:45
@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/3439533857

@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: Support gracefully stopping traces on-demand (#2828)
.git/rebase-apply/patch:1418: trailing whitespace.
        
.git/rebase-apply/patch:1434: trailing whitespace.
        
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
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/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs
A	src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs
M	src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs
Falling back to patching base and 3-way merge...
Removing src/Tools/dotnet-monitor/RequestLimitMiddleware.cs
Auto-merging src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs
CONFLICT (content): Merge conflict in src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs
CONFLICT (modify/delete): src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs deleted in HEAD and modified in Support gracefully stopping traces on-demand (#2828). Version Support gracefully stopping traces on-demand (#2828) of src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs left in tree.
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs
Removing src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitAttribute.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/LoggingExtensions.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Support gracefully stopping traces on-demand (#2828)
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
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/3446906728

@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: Support gracefully stopping traces on-demand (#2828)
.git/rebase-apply/patch:1418: trailing whitespace.
        
.git/rebase-apply/patch:1434: trailing whitespace.
        
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs
A	src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs
M	src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs
Falling back to patching base and 3-way merge...
Removing src/Tools/dotnet-monitor/RequestLimitMiddleware.cs
Auto-merging src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/TestHostHelper.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectTraceActionTests.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClientExtensions.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/HttpApi/ApiClient.cs
CONFLICT (modify/delete): src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs deleted in HEAD and modified in Support gracefully stopping traces on-demand (#2828). Version Support gracefully stopping traces on-demand (#2828) of src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/TraceTestUtilities.cs left in tree.
Removing src/Microsoft.Diagnostics.Monitoring.WebApi/RequestThrottling/RequestLimitAttribute.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Support gracefully stopping traces on-demand (#2828)
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 11, 2022
schmittjoseph added a commit that referenced this pull request Nov 11, 2022
* Support gracefully stopping traces on-demand (#2828)

* Minor fixes (#2882)
@jander-msft jander-msft removed the servicing-major Servicing fixes that is targeted for a major release (x.0.0 version) label Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update-release-notes Pull requests that should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants