Skip to content

Commit

Permalink
Fix wildcard dropping in DropTargetsSampler
Browse files Browse the repository at this point in the history
While /q/dev-ui/* would exclude:
- /q/dev-ui/
- /q/dev-ui/whatever
it wouldn't exclude /q/dev-ui/whatever/whenever/, which was problematic.

See additional tests.

Note that I think this should be improved with a proper matcher that
handles all our matching rules efficiently but for now, it will have to
do.
  • Loading branch information
gsmet committed Jan 17, 2025
1 parent 081c7b1 commit 7615750
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import static io.opentelemetry.semconv.UrlAttributes.URL_PATH;
import static io.opentelemetry.semconv.UrlAttributes.URL_QUERY;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.SpanKind;
Expand All @@ -15,12 +17,18 @@

public class DropTargetsSampler implements Sampler {
private final Sampler sampler;
private final Set<String> dropTargets;
private final Set<String> dropTargetsExact;
private final Set<String> dropTargetsWildcard;

public DropTargetsSampler(final Sampler sampler,
final Set<String> dropTargets) {
this.sampler = sampler;
this.dropTargets = dropTargets;
this.dropTargetsExact = dropTargets.stream().filter(s -> !s.endsWith("*"))
.collect(Collectors.toCollection(HashSet::new));
this.dropTargetsWildcard = dropTargets.stream()
.filter(s -> s.endsWith("*"))
.map(s -> s.substring(0, s.length() - 1))
.collect(Collectors.toCollection(HashSet::new));
}

@Override
Expand Down Expand Up @@ -49,26 +57,24 @@ private boolean shouldDrop(String target) {
if ((target == null) || target.isEmpty()) {
return false;
}
if (safeContains(target)) { // check exact match
if (containsExactly(target)) { // check exact match
return true;
}
if (target.charAt(target.length() - 1) == '/') { // check if the path without the ending slash matched
if (safeContains(target.substring(0, target.length() - 1))) {
if (containsExactly(target.substring(0, target.length() - 1))) {
return true;
}
}
int lastSlashIndex = target.lastIndexOf('/');
if (lastSlashIndex != -1) {
if (safeContains(target.substring(0, lastSlashIndex) + "*")
|| safeContains(target.substring(0, lastSlashIndex) + "/*")) { // check if a wildcard matches
for (String dropTargetsWildcard : dropTargetsWildcard) {
if (target.startsWith(dropTargetsWildcard)) {
return true;
}
}
return false;
}

private boolean safeContains(String target) {
return dropTargets.contains(target);
private boolean containsExactly(String target) {
return dropTargetsExact.contains(target);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,33 @@ void testDropTargets() {
assertEquals(2, countingSampler.count.get());
}

@Test
void testDropTargetsWildcards() {
CountingSampler countingSampler = new CountingSampler();
var sut = new DropTargetsSampler(countingSampler, Set.of("/q/dev-ui", "/q/dev-ui/*"));

assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/other"));
assertEquals(1, countingSampler.count.get());

assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/q/dev-ui-test"));
assertEquals(2, countingSampler.count.get());

assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui"));
assertEquals(2, countingSampler.count.get());

assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui/"));
assertEquals(2, countingSampler.count.get());

assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui/whatever"));
assertEquals(2, countingSampler.count.get());

assertEquals(SamplingResult.drop(), getShouldSample(sut, "/q/dev-ui/whatever/wherever/whenever"));
assertEquals(2, countingSampler.count.get());

assertEquals(SamplingResult.recordAndSample(), getShouldSample(sut, "/q/test"));
assertEquals(3, countingSampler.count.get());
}

private static SamplingResult getShouldSample(DropTargetsSampler sut, String target) {
return sut.shouldSample(null, null, null, SpanKind.SERVER,
Attributes.of(URL_PATH, target), null);
Expand Down

0 comments on commit 7615750

Please sign in to comment.