-
Notifications
You must be signed in to change notification settings - Fork 267
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
feat: initial pyroscope integration #1006
Conversation
why the linter is complaining about
now is beyond me. |
//nolint:staticcheck // SA1019 Existing use of deprecated but supported dial option. | ||
conn, err := grpc.Dial(cli.addr, grpc.WithInsecure(), grpc.WithContextDialer(dialerFunc)) | ||
conn, err := grpc.Dial(cli.addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(dialerFunc)) |
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.
I do not know why the linter started complaining for this here, but this seemed like the best solution
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.
seems resolved by 310cd2a
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.
utAck
//nolint:staticcheck // SA1019 Existing use of deprecated but supported dial option. | ||
conn, err := grpc.Dial(cli.addr, grpc.WithInsecure(), grpc.WithContextDialer(dialerFunc)) | ||
conn, err := grpc.Dial(cli.addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(dialerFunc)) |
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.
seems resolved by 310cd2a
Co-authored-by: Rootul P <[email protected]>
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.
LGTM!
…elestia-core into evan/pyroscope-integration
ok apologies, had a last minute addition after the first round of reviews where I added the ability to specify which profiles to run in the config, along with changing the defaults see c9794fa its identical to the rest of the additions to the config |
@evan-forbes can you add some documentation here. I would like to be able to run this |
leaving as a draft and WIP until I test this out more. not ready for review closes #1004 --------- Co-authored-by: Rootul P <[email protected]>
Description
leaving as a draft and WIP until I test this out more. not ready for review
closes #1004