Add rate limited sampling smoke tests for ingestion sampling #3731
Conversation
trask
requested review from
heyams,
jeanbisutti,
ramthi and
harsimar
as code owners
...LimitedSampling/src/smokeTest/resources/applicationinsights-ingestion-sampling-disabled.json
Show resolved
Hide resolved
trask
force-pushed
the
another-rate-limited-smoke-test
branch
from
b643f0e to
98a9486
Compare
| assertThat(rdEnvelopes.size()).isEqualTo(1000); | ||
|
|
||
| for (Envelope rdEnvelope : rdEnvelopes) { | ||
| assertThat(rdEnvelope.getSampleRate()).isEqualTo(99.99f); |
There was a problem hiding this comment.
When sampling is disabled shouldn't this be 100%?
There was a problem hiding this comment.
IIRC when it's 100 then ingestion sampling will take effect
but then again, I thought ingestion sampling only takes effect when we send no sample rate, so maybe it can be 100, I'll need to re-test this to confirm
There was a problem hiding this comment.
Looking at this part of code where sample rate is set to 99.99 when sampler returns 100:
. I do see node.js only setting sample rate attr if the rate returned from sampler isn't 100. Probably for the same reason - service side sampling applies at 100 or empty.There was a problem hiding this comment.
@trask you may want to put a comment about why you expect this to be 99.99 in the test so folks don't get confused
| assertThat(rdEnvelopes.size()).isEqualTo(1000); | ||
|
|
||
| for (Envelope rdEnvelope : rdEnvelopes) { | ||
| assertThat(rdEnvelope.getSampleRate()).isNull(); |
There was a problem hiding this comment.
Shouldn't this have a meaningful value when sampling is enabled? Am I missing something here?
There was a problem hiding this comment.
ah, this is about ingestion (server-side) sampling
There was a problem hiding this comment.
Can the name of this test be changed to make this more clear? To my understanding, this test asserts that when rate limited sampler is used, everything is sampled in at 100% & ingestion sampling enabled in json config, then we expect the rate to be null?
No description provided.