Sampling Rule and Rate Limiter Standardization #598
Conversation
| maxTracesPerSecond = 100; // default | ||
| } | ||
|
|
||
| MaxTracesSubmittedPerSecond = maxTracesPerSecond.Value; |
There was a problem hiding this comment.
If I am looking at it right I think this is perfect, but just wanted to drop a message here to double check.
We are doing RuleBasedSampler.OptInTracingWithoutLimits(); here (if the parameter that controls traces per second is set) and when a new a new rule is registered (if the user register at least one rule).
Some languages let users opt-in also using of a global configuration env variable, DD_TRACE_SAMPLE_RATE=0.7 which means, not surprisingly...sample 70% of the traces, no matter what. In this case, we need to enforce the traces per second limit as well.
I could not find evidence of this in the possibility in this tracer - I looked here - , but just wanted to double check. If you haven't this configuration option then no action is required, otherwise I'd ask you to make sure the if that env is set we comply with max traces per second.
There was a problem hiding this comment.
We don't have such a configuration option no, currently that would be accomplished with a single rule with no filter on service or operation.
There was a problem hiding this comment.
The name of this env variable should be changed from DD_CUSTOM_SAMPLING_RULES to DD_TRACE_SAMPLING_RULES.
|
|
||
| public CustomSamplingRule( | ||
| float rate, | ||
| string ruleName, |
There was a problem hiding this comment.
what is ruleName used for?
There was a problem hiding this comment.
Was for testing and more easily identifying rules.
When logging, the name is logged with the rule.
If it is not specified, it is given a name based on it's index in configuration.
return new CustomSamplingRule(r.SampleRate, r.RuleName ?? $"config-rule-{index}", r.Service, r.OperationName)
First rule is "config-rule-1", second rule is "config-rule-2", etc.
There was a problem hiding this comment.
Then it makes it easier to look at config and know which rule a trace is matched on.
| } | ||
|
|
||
| public void SetDefaultSampleRates(IEnumerable<KeyValuePair<string, float>> sampleRates) | ||
| public static void OptInTracingWithoutLimits() |
There was a problem hiding this comment.
(Too late since this is already merged)
Note that this enables TwL for all RuleBasedSampler instances, even through each Tracer instance can have different settings (some with custom rules and some without).
|
|
||
| Scope root; | ||
|
|
||
| using (root = Tracer.Instance.StartActive(operationName: operationName, serviceName: serviceName)) |
There was a problem hiding this comment.
Tests that only use manual instrumentation don't need an additional project, since we don't need to attach the profiler.
|
|
||
| Scope root; | ||
|
|
||
| using (root = Tracer.Instance.StartActive(operationName: operationName, serviceName: serviceName)) |
There was a problem hiding this comment.
Tests that only use manual instrumentation don't need an additional project, since we don't need to attach the profiler.
lucaspimentel
added
the
area:tracer
* Standardize tracing without limits features
Changes proposed in this pull request:
Standardize configuration of trace sampling rules and rate limiting for tracing without limits.
@DataDog/apm-dotnet