profiler: add profiler package #598
Conversation
This change simply copies over the private Go profiler into the `profiler` package of `dd-trace-go`.
This change removes the option of adding a custom logger and instead uses the already existing logger from dd-trace-go which will prefix messages with `Datadog Tracer %version%: %message%`
This change temporarily removes environment variables until we decide for a standardized naming.
gbbr
force-pushed
the
gbbr/profiler
branch
2 times, most recently
from
cada2b5
to
4404084
Compare
This change adds `WithService` to the tracer as an alias for `WithServiceName` which now becomes marked as deprecated. It also renames `WithServiceName` to `WithService` in the `profiler` package. Additionally, these are added to the example, along with the version tag.
| } | ||
| } | ||
|
|
||
| // WithTags specifies a set of tags to be attached to the profiler. These may help |
There was a problem hiding this comment.
Extremely minor suggestion
| // WithTags specifies a set of tags to be attached to the profiler. These may help | |
| // WithTags specifies a set of tags to be attached to the profiles. These may help |
| func TestDefaultConfig(t *testing.T) { | ||
| t.Run("base", func(t *testing.T) { | ||
| cfg := defaultConfig() | ||
| assert := assert.New(t) |
There was a problem hiding this comment.
Some places we use assert := assert.New(t) and some places we pass t for every call to assert.
Is there a good reason to do one over the other in different places? If not I think it looks more consistent to stick with one.
There was a problem hiding this comment.
I think it's fine where it's just one line of code, e.g. assert.True(t, ok) vs adding more code which defines a variable only to be used once. Arguably there's places where it's used more than once and could be changed...
| }, nil | ||
| } | ||
|
|
||
| func mutexProfile(cfg *config) (*profile, error) { |
There was a problem hiding this comment.
heapProfile, cpuProfile, blockProfile, and mutexProfile look almost identical except for some string values and the profile function that is called.
What do you think about unifying them?
There was a problem hiding this comment.
Do you have an example of what that'd look like? We may want to reconsider after trying to put it together 🙂I prefer verbosity if it adds to simplicity.
This change adds support for Datadog Profiling.
The `profiler` package is used as a standalone package and must be started separately from the tracer. Because it is part of the same repository, it will be imported using the same domain:
```
import "gopkg.in/DataDog/dd-trace-go.v1/profiler"
```
To start the profiler, make sure you pass it your Datadog API Key:
```
if err := profiler.Start(profiler.WithAPIKey("123key")); err != nil {
log.Fatalf("Error starting profiler: %v", err)
}
defer profiler.Stop()
```
Various other options may be used and can be seen in the documentation: http://godoc.org/gopkg.in/DataDog/dd-trace-go.v1/profiler/
This change adds support for Datadog Profiling.
The
profilerpackage is used as a standalone package and must be started separately from the tracer. Because it is part of the same repository, it will be imported using the same domain:To start the profiler, make sure you pass it your Datadog API Key:
Various other options may be used and can be seen in the documentation: http://godoc.org/gopkg.in/DataDog/dd-trace-go.v1/profiler/