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

Add Process Metadata Events #1276

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Process Metadata Events #1276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/TraceEvent/EventSources/ProcessMetadataEventSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using System;
using System.Diagnostics.Tracing;

namespace Microsoft.Diagnostics.Tracing
{
[EventSource(Name = "ProcessMetadataEventSource")]
Copy link
Member

Choose a reason for hiding this comment

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

This name typically wouldn't have "EventSource" at the end of it. Most of our naming conventions either do Microsoft-bla-bla or we use a dotted namespace name. Neither of these feel appropriate here however so I am trying to think what we should do. I don't think we need to block on it though.

public sealed class ProcessMetadataEventSource : EventSource
Copy link
Member

Choose a reason for hiding this comment

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

Does this EventSource ever get created or invoked when I run TraceEvent? I couldn't find any code elsewhere that used it though I suspect you needed it to create the parser generated code?

It is still useful to use as a reference for what would get emitted that we can talk about, but if its only role was going to a reference then maybe we can put the code somewhere else? @brianrob do we have any precedent around this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is only for reference.

{
public class Tasks
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we need them yet, but should we have keywords for Process, Thread, and Module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intentionally steered away from them, because it would seem like process metadata is a sort of monolith. I'm happy to add them now or later if you see it differently.

{
public const EventTask Process = (EventTask)1;
public const EventTask Thread = (EventTask)2;
public const EventTask Module = (EventTask)3;
}

[Event(1, Opcode = EventOpcode.Start, Task = Tasks.Process)]
public void ProcessStart(long ProcessId, long ParentProcessId, string Executable, string CommandLine)
Copy link
Member

Choose a reason for hiding this comment

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

Does CommandLine include the executable? If it does do we need Executable as a separate field?

{
this.WriteEvent(1, ProcessId, ParentProcessId, Executable, CommandLine);
}

[Event(2, Opcode = EventOpcode.Stop, Task = Tasks.Process)]
public void ProcessExit(long ProcessId, long ParentProcessId, int ExitCode, string Executable, string CommandLine)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Can we call this ProcessStop to match the start/stop naming guidance?

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.

{
this.WriteEvent(2, ProcessId, ParentProcessId, ExitCode, Executable, CommandLine);
}

[Event(3, Opcode = EventOpcode.Start, Task = Tasks.Thread)]
public void ThreadCreate(long ProcessId, long ThreadId, ulong StackBaseAddress, string ThreadName)
Copy link
Member

Choose a reason for hiding this comment

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

Question: For the process and thread events, I'm interpreting that these events are for when the action occurs (e.g. when a thread is created we trigger a ThreadCreate event). What about cases where the thread already existed when the trace starts. Do we need a way to encapsulate that? I know that you're looking to avoid rundown - do you have a recommendation on how to handle this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rundown events or capture state events would have the same format, so I thought we could overload this, but I can also just add a specific thing for "Rundown", or maybe a more neutral term for catch up events.

Copy link
Member

Choose a reason for hiding this comment

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

I do think it would be worth knowing if the thread has existed longer than the trace or not. Perhaps something similar to Thread/DCStart? Also, if you don't mind, let's change to Thread/Start and Thread/Stop top match existing precedent.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need a way to encapsulate that? I know that you're looking to avoid rundown - do you have a recommendation on how to handle this?

I agree that it would be worthwhile to distinguish information that is snapshotting the current state of the system from events that are indicating a state change is occuring at a specific point in time. Differently named events are certainly one way to represent that but I am going to suggest we hold off exploring too deeply before we've come to some decision whether we want the capability to slice regions out of a trace file and process the events in that time range in isolation. That slicing functionality seems like a pretty useful capability generally and I know its something Bing in particular benefits from, but it doesn't appear to play well with rundown style approaches that place a large number of stateful events at only a single point in the file. I think most formats that do support that kind of slicing wind up having something like keyframes, up-to-date snapshots of information that are repeated on a regular cadence.

{
this.WriteEvent(3, ProcessId, ThreadId, StackBaseAddress, ThreadName);
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a definition of what string to expect in ThreadName? For example .Net has a Thread.Name property and Linux has pthread_getname_np() but I don't know if the strings returned by those APIs are guaranteed to be equal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting a name change? I suppose it could be either, but I think adding two thread name events would be overkill.

Copy link
Member

@noahfalk noahfalk Sep 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I am fine with the argument name (and keeping it singular). I am just trying to sort out what data do we expect to be provided? On Windows we have precedent from the kernel events but on all other platforms we have to define what is the equivalent data we are going to populate this with.

}

[Event(4, Opcode = EventOpcode.Stop, Task = Tasks.Thread)]
public void ThreadDestroy(long ProcessId, long ThreadId, ulong StackBaseAddress, string ThreadName)
{
this.WriteEvent(4, ProcessId, ThreadId, StackBaseAddress, ThreadName);
}

[Event(5, Opcode = EventOpcode.Start, Task = Tasks.Module)]
public void ModuleLoad(long ProcessId, ulong LoadAddress, long ModuleSize, Guid DebugGuid, int DebugAge, string ModuleFilePath, string DebugModuleFileName)
Copy link
Member

Choose a reason for hiding this comment

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

Is the DebugGuid a PDBGuid, or were you going for a cross-platform representation, and avoiding the term PDB? If so, is Guid used on Linux? I know for shared objects it's the build id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As luck has it, ELF Debug IDs are 20-bytes. so I contemplated making this a byte[] but then thought about MachO which is also a GUID without any age, so I thought we could encode it as 0. And we could hack into the ELF IDs as guid + age to take the 20 bytes.

I just picked this and not a byte array because it was more convenient. @noahfalk thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha - yes, interested in @noahfalk's thoughts here. Do we have any other precedent?

Copy link
Member

Choose a reason for hiding this comment

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

I'd be tempted to encode it as a byte[] or string. Fixed size types would be a little more performant but given these shouldn't be frequent events I'd favor an encoding that feels straightforward.

If the goal is to be able to look up the images and symbols on a symbol server then the data we need is:
format - PE/ELF/MachO

  • PE - filesize, timestamp, filename, debug signature, age, and the codeview debug directory major/minor version (major=0x100,minor=504D is a sentinel value that indicates portable pdb is supported - PE spec).
  • ELF - build-id and filename
  • MachO - uuid and name

SSQP spec

{
this.WriteEvent(5, ProcessId, LoadAddress, ModuleSize, DebugGuid, DebugAge, ModuleFilePath, DebugModuleFileName);
}

[Event(6, Opcode = EventOpcode.Stop, Task = Tasks.Module)]
public void ModuleUnload(long ProcessId, long LoadAddress, long ModuleSize, string ModuleFilePath)
{
this.WriteEvent(6, ProcessId, LoadAddress, ModuleSize, ModuleFilePath);
}
}
}
Loading