Add RequestStackwalk parameter to EventPipeSession #4290
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Client-side of dotnet/runtime#84077 and the implementation of #3696.
To simplify the interface I made
EventPipeSessionConfiguration
public and introduced a new method in the DiagnosticsClient:Task<EventPipeSession> StartEventPipeSessionAsync(EventPipeSessionConfiguration configuration, CancellationToken token)
. This is the only method that supports disabling the stackwalk so no additional overloads with a new bool parameter and no synchronous counterpart. I believe it'd be easier to use and maintain a single async method with the options rather than creating more overloads or default parameters but I may not have all the context here so please correct me if you think it's a bad idea.To deal with the backward compatibility I only use
CollectTracingV3
when necessary i.e. whenRequestStackwalk
option is set to false. I think it's a good compromise between the added complexity and potentially surprising behavior:CollectTracingV2
CollectTracingV3
doesn't exist server side: there'd be no clear error message but it's documented in the option summary.CollectTracingV2
orCollectTracingV3
may be used transparently for the userCollectTracingV4
The alternative is to implement version negotiation of some sort but I'd like to have your opinion before attempting this as handling the errors correctly wouldn't be easy (f.e. in my current fork I just hide the exception)
The testing turned out to be a bit complex as I needed to convert EventPipe stream to
TraceLog
to be able to read the stacktraces. I couldn't achieve that without writing data to a file. Afaiu the stackwalk may not work correctly without the rundown that only happens at the end of the session so I wonder if looking at the stacktraces with a live session is even possible (though iirc netfw+ETW could do that back in the days) ?Thanks for your time !