-
Notifications
You must be signed in to change notification settings - Fork 61
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
[deployer] Add Profiling #629
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Pyroscope profiling support to the deployer, updating monitoring configuration, AWS security groups, and documentation to incorporate the new profiling service.
- Adds Pyroscope version constants, service configuration, and installation command updates in Rust code.
- Updates AWS security groups and instance documentation to expose the new profiling port.
- Includes Pyroscope dependencies in the example project configuration.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
deployer/src/ec2/services.rs | Added Pyroscope constants, service file content, and configuration generation. |
examples/flood/Cargo.toml | Added Pyroscope dependencies. |
deployer/src/ec2/aws.rs | Updated port mapping to reference new profiling port constants. |
deployer/src/ec2/mod.rs | Updated documentation to include Pyroscope and added new port definitions. |
deployer/src/ec2/create.rs | Integrated Pyroscope service and configuration file handling during deployment. |
Comments suppressed due to low confidence (2)
examples/flood/Cargo.toml:32
- Verify if 'pyroscope_pprofrs' is correctly spelled. It might be intended to be 'pyroscope_pprof' or another similar name.
pyroscope_pprofrs = "0.2.8"
deployer/src/ec2/services.rs:165
- [nitpick] For consistency with generate_prometheus_config where the IP is named 'ip', consider renaming 'private_ip' to 'ip'.
for (name, private_ip, region) in binary_instances {
This code regularly segfaults on ubuntu (afaict because of a bug in pprof). My guess is that (may have been fixed here but not released yet: grafana/pyroscope-rs#192) ![]() It should be possible to avoid by writing our own "backend" (If it is the issue): https://github.com/grafana/pyroscope-rs/blob/main/pyroscope_backends/pyroscope_pprofrs/src/lib.rs |
Didn't help:
Could potentially have to do with running on ARM? |
debugging:
|
Even in dedicated thread it panics:
|
Consider adding (if we just use
|
TODO
runtime::tokio
support forcollect_system_metrics()
,expose_metrics()
, andpush_profiles()