googleapis / googleapis/google-cloud-go
profiler: add Start function with a context to abort profiling
- Dominant language
- Go
- Stars
- 4.5k
- Forks
- 1.6k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 109
Description
**Is your feature request related to a problem? Please describe.**
I want to be able to stop the profiling and restart it - possibly with another config even.
However, the current API with `profiler.Start()` doesn't support that.
**Describe the solution you'd like**
Possibly a `ctx` argument for the `Start()` function, however, this is likely not possible due to backwards compatibility, therefore a new function could be added:
```go
// Start starts a goroutine to collect and upload profiles. The
// caller must provide the service string in the config. See
// Config for details. Start should only be called once. Any
// additional calls will be ignored.
func Start(cfg Config, options ...option.ClientOption) error {
startError := startOnce.do(func() error {
return start(context.Background(), cfg, options...)
})
return startError
}
// StartWithContext starts a goroutine to collect and upload profiles. The
// caller must provide the service string in the config. See
// Config for details. StartWithContext should only be called once. Any
// additional calls will be ignored.
// When the context is cancalled the profiling is aborted
func StartWithContext(ctx context.Context, cfg Config, options ...option.ClientOption) error {
startError := startOnce.do(func() error {
return start(ctx, cfg, options...)
})
return startError
}
func start(ctx context.Context, cfg Config, options ...option.ClientOption) error {
...
}
```
I think that [`pollProfilerService`](https://github.com/timofurrer/google-cloud-go/blob/852a230c929a4a2614ecb1c485bcedd1b3f8bcae/profiler/profiler.go#L624-L624) service also needs to change to respect exiting the `for`-loop on context cancellation.
When that is implemented I think the code and test code can be refactored to remove `i < config.numProfiles` and `profilerDone` channel and use a context during tests.
**Describe alternatives you've considered**
- use a `profiler.Config` field with a context - that would avoid adding `StartWithContext`, but is also kinda weird from an API perspective
**Additional context**
I haven't looked at the profiler internals and I'm not sure if it's even possible to simply stop and restart the profiling without leaving any traces of it.
I'm also open to contribute this myself if you are willing to accept the change :)
Contributor guide
Assessment
This issue has not been assessed yet.