Add dedicated error sampling #420
Conversation
bmermet
force-pushed
the
bmermet/error-sampling
branch
from
9800336 to
cf647fe
Compare
| @@ -320,3 +329,12 @@ func (a *Agent) watchdog() { | |||
|
|
|||
| info.UpdatePreSampler(*a.Receiver.preSampler.Stats()) | |||
| } | |||
|
|
|||
| func traceContainsError(trace model.Trace) bool { | |||
There was a problem hiding this comment.
There's a lot of functions which iterate over the entire trace (e.g. GetRoot, ChildrenMap, etc.). We should possibly only do this once when we receive the trace and compute various stats about the trace such as if it has errors, its root, the children map and other things we need.
This is just a remark, no action needed here. Possibly a worthy improvement to be done in another PR or during a potential refactor.
There was a problem hiding this comment.
Agreed, we should definitely think of consolidating all this logic to iterate only once on the trace. I'll keep that for another PR though.
cmd/trace-agent/sampler.go
Outdated
| @@ -31,6 +31,13 @@ func NewScoreSampler(conf *config.AgentConfig) *Sampler { | |||
| } | |||
| } | |||
|
|
|||
| // NewErrorsSampler creates a new empty error sampler ready to be started | |||
There was a problem hiding this comment.
Nit. Can we end this sentence with a period? Also, it would be helpful (at least to me) to know what an "errors sampler" is. Maybe this is the right place to write about that?
info/info.go
Outdated
| @@ -155,7 +156,7 @@ func publishSamplerInfo() interface{} { | |||
| return samplerInfo | |||
| } | |||
|
|
|||
| // UpdatePrioritySamplerInfo updates internal stats about priority sampking | |||
| // UpdatePrioritySamplerInfo updates internal stats about priority sampling | |||
There was a problem hiding this comment.
Please put a period at the end of this sentence, and the next one further down.
sampler/coresampler.go
Outdated
| @@ -33,6 +33,18 @@ const ( | |||
| defaultSignatureScoreSlope float64 = 3 | |||
| ) | |||
|
|
|||
| // EngineType represent the type of a sampler engine | |||
| type EngineType int | |||
There was a problem hiding this comment.
I like this a lot 👍 Gives for a much cleaner, easier to understand and read API.
sampler/scoresampler.go
Outdated
| return s | ||
| } | ||
|
|
||
| // NewErrorEngine returns an initialized Sampler |
There was a problem hiding this comment.
I think we can do a bit better with this comment. 😄
This PR add a dedicated error sampler to the trace-agent. The goal of this is to keep more error traces (which are the most interesting ones).
With this PR the
trace-agentwill maintain two different score samplers that will each have their own maximum TPS. One for traces containing errors and one for other traces. That way the TPS of errors that the agent sends to the API will be independent from the TPS of traces containing no errors.