Skip to content

Fix TraceEvent CaptureState API to work with keywords not previously enabled in session#2222

Merged
brianrob merged 6 commits intomainfrom
copilot/fix-2132-2
Jun 5, 2025
Merged

Fix TraceEvent CaptureState API to work with keywords not previously enabled in session#2222
brianrob merged 6 commits intomainfrom
copilot/fix-2132-2

Conversation

Copy link
Contributor

Copilot AI commented Jun 5, 2025

The TraceEventSession.CaptureState API was not working correctly when called with keywords that hadn't already been enabled for the provider in the current ETW session. This caused CaptureState events to not be triggered, even though the API call appeared to succeed.

Problem

When a provider was enabled with specific keywords and then CaptureState was called with different keywords, the capture state events would not be generated:

// Enable provider with one set of keywords
const long MICROSOFT_KEYWORD_MEASURES = 0x0000400000000000;
session.EnableProvider(Microsoft_ML_ONNXRuntime_Provider, TraceEventLevel.Verbose, MICROSOFT_KEYWORD_MEASURES);

// CaptureState with different keywords - this would fail silently
const ulong ORTTraceLoggingKeyword_Session = 0x1;
session.CaptureState(Microsoft_ML_ONNXRuntime_Provider, ORTTraceLoggingKeyword_Session);

Solution

The fix queries the ETW session to determine the currently enabled keywords for the provider and merges them with the requested capture state keywords before sending the command:

  1. Query existing keywords: Uses the existing GetEnabledKeywordsForProviderAndSession method to retrieve currently enabled keywords
  2. Merge keywords: Combines existing keywords with requested keywords using bitwise OR
  3. Re-enable provider: If existing keywords are found, first re-enables the provider with merged keywords
  4. Send capture state: Sends the CAPTURE_STATE command with the merged keywords

This ensures that CaptureState works consistently regardless of previously enabled keywords, matching the behavior of tools like xperf:

# This works in xperf regardless of previously enabled keywords
xperf -capturestate SessionName 3a26b1ff-7484-7484-7484-15261f42614d:1:5

Changes

  • Modified TraceEventSession.CaptureState method to query and merge existing provider keywords
  • Maintained backward compatibility - no breaking changes to the public API
  • Added keyword merging logic before sending ETW commands
  • Preserved all existing error handling and parameter validation

Fixes #2132.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Query existing provider keywords and merge with requested keywords before CaptureState.
Enables provider with merged keywords if necessary to ensure CaptureState works
even when using keywords that haven't been previously enabled.

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Copilot AI changed the title [WIP] TraceEvent - CaptureState API does not seem to work when using keywords that have not already been enabled in the session Fix TraceEvent CaptureState API to work with keywords not previously enabled in session Jun 5, 2025
Copilot AI requested a review from brianrob June 5, 2025 20:49
}
// Query existing keywords and merge them with requested keywords before capture state
ulong mergedKeywords = matchAnyKeywords;
Guid* providerPtr = stackalloc Guid[1];
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using stackalloc, you should be able to use the fixed keyword to convert the Guid to a Guid*.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Replaced stackalloc with fixed keyword using an array to get the Guid pointer. Commit: 2659beb

ulong mergedKeywords = matchAnyKeywords;
Guid* providerPtr = stackalloc Guid[1];
*providerPtr = providerGuid;
long? existingKeywords = GetEnabledKeywordsForProviderAndSession(providerPtr, (ulong)m_SessionId);
Copy link
Member

Choose a reason for hiding this comment

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

We will also need the level. Please modify GetEnabledKeywordsForProviderAndSession instead return a struct containing the keywords and level. You can rename it GetEnabledInfoForProviderAndSession. The level in the setruct should be of type TraceEventLevel. Then, pass the level to the call to EnableTraceEx2 that enables the provider. Leave the call to CAPTURE_STATE with a Verbose level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Modified the method to return EnabledProviderInfo struct containing both keywords and level, renamed it to GetEnabledInfoForProviderAndSession, and now pass the returned level to EnableTraceEx2. CAPTURE_STATE still uses Verbose level as requested. Commit: 9c4bdaf

Copilot AI and others added 2 commits June 5, 2025 21:18
…level support

Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
Co-authored-by: brianrob <6210322+brianrob@users.noreply.github.com>
@brianrob
Copy link
Member

brianrob commented Jun 5, 2025

@ivberg, do you have a repro handy that you can test this with?

@brianrob
Copy link
Member

brianrob commented Jun 5, 2025

Spoke with @ivberg offline and I have tested this locally.

@brianrob brianrob marked this pull request as ready for review June 5, 2025 23:12
marklio
marklio previously approved these changes Jun 5, 2025
EnabledProviderInfo? existingInfo = GetEnabledInfoForProviderAndSession(&providerGuid, (ulong)m_SessionId);
if (existingInfo.HasValue)
{
mergedKeywords = matchAnyKeywords | existingInfo.Value.MatchAnyKeywords;
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: mergedKeywords |= existingInfo.Value.MatchAnyKeywords; better reflects the "merge" operation and the content of the variable. I would also write it this way to avoid the future bug where you merge something before this and lose it because you're not accumulating.

@brianrob brianrob merged commit 1c22582 into main Jun 5, 2025
5 checks passed
@brianrob brianrob deleted the copilot/fix-2132-2 branch June 5, 2025 23:53
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.

TraceEvent - CaptureState API does not seem to work when using keywords that have not already been enabled in the session

3 participants