Skip to content

Make DiagnosticsClient.StartEventPipeSessionAsync public#4394

Merged
davmason merged 1 commit into
dotnet:mainfrom
emz00:evgeny-starteventpipesessionasync-public
Nov 21, 2023
Merged

Make DiagnosticsClient.StartEventPipeSessionAsync public#4394
davmason merged 1 commit into
dotnet:mainfrom
emz00:evgeny-starteventpipesessionasync-public

Conversation

@emz00

@emz00 emz00 commented Nov 13, 2023

Copy link
Copy Markdown
Contributor

This allows user code to pass a cancellation token without wrapping the call to StartEventPipeSession() in Task.Run() or similar. Resolves #3727.

This allows user code to pass a cancellation token without wrapping the call to StartEventPipeSession() in Task.Run() or similar. Resolves dotnet#3727.
/// </returns>
internal Task<EventPipeSession> StartEventPipeSessionAsync(IEnumerable<EventPipeProvider> providers, bool requestRundown, int circularBufferMB, CancellationToken token)
public Task<EventPipeSession> StartEventPipeSessionAsync(IEnumerable<EventPipeProvider> providers, bool requestRundown,
int circularBufferMB = DefaultCircularBufferMB, CancellationToken token = default)

@emz00 emz00 Nov 13, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made circularBufferMB optional here consistent with the sync overload and extracted the default value into a constant to avoid duplicating it. This required making token optional as well.

@hoyosjs hoyosjs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. @noahfalk @davmason does exposing these sound good to y'all ?

@noahfalk

Copy link
Copy Markdown
Member

LGTM. @noahfalk @davmason does exposing these sound good to y'all ?

Seems fine to me as long functionally they are working well?

@mikelle-rogers mikelle-rogers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@loop-evgeny, will you please document this now that it is public?
It can be documented here: https://github.com/dotnet/docs/blob/main/docs/core/diagnostics/microsoft-diagnostics-netcore-client.md

emz00 added a commit to emz00/dotnet-docs that referenced this pull request Nov 17, 2023
emz00 added a commit to emz00/dotnet-docs that referenced this pull request Nov 17, 2023
@emz00

emz00 commented Nov 17, 2023

Copy link
Copy Markdown
Contributor Author

dotnet/docs#38299

@davmason davmason merged commit e76bdb8 into dotnet:main Nov 21, 2023
gewarren pushed a commit to dotnet/docs that referenced this pull request Nov 27, 2023
* Document DiagnosticsClient.StartEventPipeSessionAsync methods

These are being made public in dotnet/diagnostics#4394

* Update docs/core/diagnostics/microsoft-diagnostics-netcore-client.md

Co-authored-by: mikelle-rogers <45022607+mikelle-rogers@users.noreply.github.com>

* Add StartEventPipeSessionAsync to method summary list

---------

Co-authored-by: mikelle-rogers <45022607+mikelle-rogers@users.noreply.github.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make async overloads of DiagnosticsClient public

5 participants