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.

Sign up for GitHub

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

Jump to bottom

Fixing crash in dump-events #2717

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Fixing crash in dump-events #2717

merged 4 commits into from
Sep 29, 2023

Conversation

scottoneil-ms
Copy link
Contributor

I'm not 100% sure why this event started getting into my etl logs and triggering crashes, but I suspect the right answer is to treat it as another way of recognizing session start.

michaelcfanning
michaelcfanning approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@michaelcfanning michaelcfanning left a comment

Choose a reason for hiding this comment

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

:shipit:

michaelcfanning
michaelcfanning reviewed Sep 29, 2023
View reviewed changes
src/Sarif.Driver/DumpEventsCommand.cs
}

case "EventTrace/PartitionInfoExtension":
Copy link
Member

Choose a reason for hiding this comment

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

PartitionInfoExtension

well, okey-doke. :) you collected the legacy form, it appears.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's obvious what I did to trigger legacy form mode instead of V2, I'd love to be corrected. This is the command I'm using. [1]

[1] Sarif.PatternMatcher.cli.exe analyze ..\..\..\..\..\..\AzureDevOps\ --recurse true --max-file-size-in-kb 999999999 --output .\ado.sarif --level "Error;Warning;Note" --plugin ".\SEC101.SecurePlaintextSecretsInternal.json" --log ForceOverwrite --insert ContextRegionSnippets --etw .\ADO-Run.etl

@michaelcfanning
Copy link
Member

Suggest adding a release note on this in ReleaseNotes.md, similar to following. Please replace NullReferenceException with the actual type of the exception you see when we have this mismatched start/stop pair.

BUG: Resolve trace parsing NullReferenceException exception by updating dump-events command to process PartitionInfoExtension session event as we do PartitionInfoExtensionV2.

@scottoneil-ms
Update ReleaseHistory.md
b6de96c 
Updating release notes for invalid operation exception fix.
@scottoneil-ms
Update ReleaseHistory.md
f15f8e7 
Fixing single quotes in release notes.
@michaelcfanning michaelcfanning merged commit 3fcf228 into main Sep 29, 2023
8 checks passed
@michaelcfanning michaelcfanning deleted the scott-003 branch September 29, 2023 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelcfanning michaelcfanning michaelcfanning approved these changes

@EasyRhinoMSFT EasyRhinoMSFT Awaiting requested review from EasyRhinoMSFT EasyRhinoMSFT is a code owner

@marmegh marmegh Awaiting requested review from marmegh

@cfaucon cfaucon Awaiting requested review from cfaucon cfaucon is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@scottoneil-ms @michaelcfanning