cmd/trace-agent: force sampling of transactions with priority 2 #463
Conversation
sampler/prioritysampler.go
Outdated
| @@ -22,7 +22,8 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| samplingPriorityKey = "_sampling_priority_v1" | |||
| // SamplingPriorityKey is the key of the sampling priority value in the metrics dictionnary of the root span | |||
| SamplingPriorityKey = "_sampling_priority_v1" | |||
sampler/prioritysampler.go
Outdated
| @@ -22,7 +22,8 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| samplingPriorityKey = "_sampling_priority_v1" | |||
| // SamplingPriorityKey is the key of the sampling priority value in the metrics dictionnary of the root span | |||
There was a problem hiding this comment.
s/dictionnary/map
Dictionary is more of a Python term :-)
cmd/trace-agent/agent.go
Outdated
| if pt.Root == nil { | ||
| return 0, false | ||
| } | ||
| priorityFloat, hasPriority := pt.Root.Metrics[sampler.SamplingPriorityKey] |
There was a problem hiding this comment.
I guess the general way is to use ok for the second arg. So maybe you'd wanna use something like p, ok.
gbbr
changed the title
|
Can you please prefix your commit with the affected package name? E.g: when merging, just squash and use the PR title, I've updated it for you. |
Makes sure that when a span from a trace with a sampling priority of 2 matches the critera to be analyzed it is kept even if the rate based sampling policy would discard it.
bmermet
force-pushed
the
bmermet/force-transaction-sampling-p2
branch
from
90abcc8 to
f903d5f
Compare
| @@ -22,7 +22,8 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| samplingPriorityKey = "_sampling_priority_v1" | |||
| // SamplingPriorityKey is the key of the sampling priority value in the metrics map of the root span | |||
| SamplingPriorityKey = "_sampling_priority_v1" | |||
There was a problem hiding this comment.
Would it be better to use KeySamplingPriority? We could make it a convention for constants representing keys. It would be in the same style as the standard library http package constants (link)
There was a problem hiding this comment.
I don't know how much of a common practice it is in go. If you think it is the idiomatic way to do it, let's do that.
This PR makes sure that when a span from a trace with a sampling priority of 2 matches the criteria to be analyzed it is kept even if the rate based sampling policy would discard it.
Since these traces have been flagged as being of particular interest by the user, it makes sense to ensure the transactions too are always kept.