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

Initial support for profiling - internal release #1955

Closed
bruno-garcia opened this issue Sep 28, 2022 · 10 comments
Closed

Initial support for profiling - internal release #1955

bruno-garcia opened this issue Sep 28, 2022 · 10 comments
Labels
Feature New feature or request

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Sep 28, 2022

Possibly through the same apis used by dotnet trace
It uses speedscope format.

Related things:
iOS profiler:

Android:

NodeJS:

Python:

Resources

dotnet-trace

  • is a standalone CLI program
  • works only with .NET Core 3.0 Preview 5 or later
  • either attaches to a running process or starts a process itself, depending on arguments
  • always captures .nettrace format, if speedscope is specified, it's just converted at the end of the run
  • may be possible to bundle as a precompiled lib with Native AOT?
  • actually profiling NativeAOT compiled code wouldn't be possible at least until .NET 8: Native AOT Diagnostics in .NET 8 dotnet/runtime#79241
@mattjohnsonpint mattjohnsonpint added Feature New feature or request Impact: Large labels Sep 28, 2022
@mattjohnsonpint
Copy link
Contributor

@cove
Copy link

cove commented Feb 2, 2023

This would be helpful for us

@bruno-garcia
Copy link
Member Author

Came in on Discord:

Hi everyone - new user here 🙂 - Is there a recommendation for profiling on dotnet? We used the older version for a while and now with our signup noticed that dotnet isn't offered. Sorry if this is addressed elsewhere

@vaind
Copy link
Collaborator

vaind commented Feb 16, 2023

After making a small change to the dotnet-trace command-line app to connect to its own process (ignore the --process-id arg and use Process.GetCurrentProcess().Id instead), it seems to be perfectly happy about profiling itself. See the attached profiles and a screenshot from speedscope

dotnet-trace.exe_20230216_122438.zip

image

With the dotnet-trace code being licensed under MIT, it seems like a good candidate for cherry-picking an in-process profiling implementation that could be part of the Sentry SDK. We would likely need a way to filter-out profiling-related events so they don't confuse people?

The CPU usage of the whole dotnet-trace executable as reported by the process monitor on my PC was reporting between 0.0 and 0.2 % - since it wasn't actually doing anything else than collecting the trace, it should represent the actual usage of sampler collection (and writing to file).

image

@vaind
Copy link
Collaborator

vaind commented Feb 24, 2023

Status update:

I have rolled the nettrace processing directly in a fork of dotnet-trace (see the current working version here: https://github.com/vaind/diagnostics/tree/sentry-profiling) and while I am producing a JSON which I hope is correct, but I haven't had luck getting it to sentry.io yet. Likely the issue is with how I'm pushing the envelope with the profile manually (through sentry-CLI) and it doesn't get associated with the transaction. Or it's just invalid - can't tell at the moment because I don't see what's happening on the server :/

profile.zip

@phacops
Copy link

phacops commented Feb 24, 2023

Two PRs need to get in in order to accept your profile:

Looking at your profile, a few things I saw you'd need to correct:

  • timestamp needs to be RFC3339 formatted, like 2023-03-01T10:10:10.123456789+06:00 (the more precision the better, down to the nanosecond if possible).
  • os.build should be os.build_number and os.build_number should be os.version
  • transaction.id needs to be a uuid4 without -
  • there's no field id in thread_metadata values

@phacops
Copy link

phacops commented Feb 24, 2023

Also, at what rate are you sampling? We use 101Hz in our other SDKs.

@vaind
Copy link
Collaborator

vaind commented Feb 27, 2023

Note to self:

The raw addresses have to be associated with some entity that knows its symbolic name. At this point things work very differently for native code and code that is JIT compiled on the fly:

  • For native code, TraceEvent must find the DLL that includes the code, for this it needs information about all DLLs that where loaded in the process and what addresses they are loaded at. These are the kernel ImageLoad events.
  • For JIT compiled code it needs to know the code ranges of all JIT compiled methods. For this it needs special .NET or Jscript events specifically designed for this purpose.

If the necessary events are not present, the best that can be done is to show the address value as a hexadecimal number (which is not very helpful). Thus it is critical that these events be present. Complicating this is the fact that in many scenario of long running processes. If the process lives longer than the collection interval, then there can be image loads or JIT compilation that occurred before the trace started. We need these events as well. To get them the ETW providers involved support something called 'CAPTURE_STATE' which causes them to emit events for all past image loads or JIT compilations. The logic for capturing data must explicitly include logic for triggering this CAPTURE_STATE.

@phacops
Copy link

phacops commented Feb 28, 2023

Both PRs have been merged and deployed so no more blocker on our side.

@vaind vaind mentioned this issue Mar 2, 2023
19 tasks
@bruno-garcia bruno-garcia changed the title Support for profiling Support for profiling for Desktop/CLI (whole process) Apr 19, 2023
@bruno-garcia bruno-garcia changed the title Support for profiling for Desktop/CLI (whole process) Initial support for profiling - internal release Apr 19, 2023
@bruno-garcia
Copy link
Member Author

Closing this through #2206

Follow ups are: #2315 and #2316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
Archived in project
Development

No branches or pull requests

5 participants