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

Driver ETW events #2668

Merged
merged 5 commits into from
May 12, 2023
Merged

Driver ETW events #2668

merged 5 commits into from
May 12, 2023

Conversation

michaelcfanning
Copy link
Member

@michaelcfanning michaelcfanning commented May 12, 2023

  • NEW: Provide new ETW telemetry for runtime behavior, provider SarifDriver, guid c84480b4-a77f-421f-8a11-48210c1724d4.

This change proposes ETW telemetry for a range of general, useful data points:

  • Start/Stop for scan target retrieval (typically from disk)
  • Time spent in overall file enumeration phase
  • Time spent analyzing a discrete scan target
  • Time spent in aggregate for an individual rule
  • Notice that a failure result was generated, by rule + severity level
  • Also includes a general per-scan-target event and per-rule event. The intent here is that consumers can create arbitrary event ids that would allow filter/querying against them to collect arbitrary things.

Open:

  • I haven't added collecting data for time spent in the logging phase
  • We should think about the pattern of updating/extending these in client tools that build on the driver. e.g., should we simply propose these users derive from DriverEventSource and add their own events? The 'danger' here is a failure to override provider name + guid. We could protect against this by declaring an abstract class for the core event generation. All of this would require adding more code to permit injection of a customized/derived event source.
  • This code doesn't propose/author a test strategy yet. It is being actively tested by the sarif pattern matcher tool while we think about desing.

@michaelcfanning michaelcfanning changed the title Driver events Driver events [DRAFT] May 12, 2023
@michaelcfanning michaelcfanning changed the title Driver events [DRAFT] Driver ETW events [DRAFT] May 12, 2023
}
else
{
if (Stream.CanSeek) { this.Stream.Seek(0, SeekOrigin.Begin); }

Check warning

Code scanning / CodeQL

Dereferenced variable may be null

Variable [this.Stream](1) may be null at this access as suggested by [this](2) null check.
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems its false-positive, this.contents is definitely null at here, only way falls to this else block is this.Stream != null


if (this.Contents != null)
{
return (ulong?)this.Contents.Length;
Copy link
Member Author

Choose a reason for hiding this comment

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

this.Contents.Length

this suggests s test gap

@@ -692,7 +692,7 @@ public void AnalyzeCommand_TracesInMemory()
levels: BaseLogger.ErrorWarningNote,
kinds: BaseLogger.Fail);

var target = new EnumeratedArtifact(FileSystem.Instance) { Uri = uri, Contents = string.Empty };
var target = new EnumeratedArtifact(FileSystem.Instance) { Uri = uri, Contents = "A", SizeInBytes = 1 };
Copy link
Member Author

Choose a reason for hiding this comment

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

SizeInBytes

remove this

@michaelcfanning michaelcfanning marked this pull request as ready for review May 12, 2023 17:46
@michaelcfanning michaelcfanning changed the title Driver ETW events [DRAFT] Driver ETW events May 12, 2023
@michaelcfanning michaelcfanning merged commit fafcf98 into main May 12, 2023
@michaelcfanning michaelcfanning deleted the driver-events branch May 12, 2023 18:10
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.

3 participants