Skip to content

replace Set<Integer> with BitSet for HTTP statuses #1496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
richardstartin merged 5 commits into from
May 27, 2020
Merged

replace Set<Integer> with BitSet for HTTP statuses #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0a12897
replace Set<Integer> with BitSet for HTTP statuses
richardstartin May 26, 2020
e37bb89
Merge branch 'master' into richardstartin/bitset-http-statuses
richardstartin May 27, 2020
b27ddbc
Merge branch 'master' into richardstartin/bitset-http-statuses
richardstartin May 27, 2020
8257868
remove unnecessary toSet() calls from ConfigTest
richardstartin May 27, 2020
44062a9
Merge branch 'richardstartin/bitset-http-statuses' of github.com:Data…
richardstartin May 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...natra-2.9/src/main/java/datadog/trace/instrumentation/finatra/FinatraInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public Listener(final AgentScope scope) {
@Override
public void onSuccess(final Response response) {
// Don't use DECORATE.onResponse because this is the controller span
if (Config.get().getHttpServerErrorStatuses().contains(DECORATE.status(response))) {
if (Config.get().getHttpServerErrorStatuses().get(DECORATE.status(response))) {
scope.span().setError(true);
}

Expand Down
43 changes: 21 additions & 22 deletions dd-trace-api/src/main/java/datadog/trace/api/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.BitSet;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -174,9 +174,9 @@ public class Config {

private static final boolean DEFAULT_PRIORITY_SAMPLING_ENABLED = true;
private static final boolean DEFAULT_TRACE_RESOLVER_ENABLED = true;
private static final Set<Integer> DEFAULT_HTTP_SERVER_ERROR_STATUSES =
private static final BitSet DEFAULT_HTTP_SERVER_ERROR_STATUSES =
parseIntegerRangeSet("500-599", "default");
private static final Set<Integer> DEFAULT_HTTP_CLIENT_ERROR_STATUSES =
private static final BitSet DEFAULT_HTTP_CLIENT_ERROR_STATUSES =
parseIntegerRangeSet("400-499", "default");
private static final boolean DEFAULT_HTTP_SERVER_TAG_QUERY_STRING = false;
private static final boolean DEFAULT_HTTP_CLIENT_TAG_QUERY_STRING = false;
Expand Down Expand Up @@ -273,8 +273,8 @@ private String profilingProxyPasswordMasker() {
private final Map<String, String> jmxTags;
@Getter private final List<String> excludedClasses;
@Getter private final Map<String, String> headerTags;
@Getter private final Set<Integer> httpServerErrorStatuses;
@Getter private final Set<Integer> httpClientErrorStatuses;
@Getter private final BitSet httpServerErrorStatuses;
@Getter private final BitSet httpClientErrorStatuses;
@Getter private final boolean httpServerTagQueryString;
@Getter private final boolean httpClientTagQueryString;
@Getter private final boolean httpClientSplitByDomain;
Expand Down Expand Up @@ -1071,8 +1071,8 @@ private static Set<PropagationStyle> getPropagationStyleSetSettingFromEnvironmen
return result;
}

private static Set<Integer> getIntegerRangeSettingFromEnvironment(
final String name, final Set<Integer> defaultValue) {
private static BitSet getIntegerRangeSettingFromEnvironment(
final String name, final BitSet defaultValue) {
final String value = getSettingFromEnvironment(name, null);
try {
return value == null ? defaultValue : parseIntegerRangeSet(value, name);
Expand Down Expand Up @@ -1178,8 +1178,8 @@ private static Set<PropagationStyle> getPropagationStyleSetFromPropertyValue(
return null;
}

private static Set<Integer> getPropertyIntegerRangeValue(
final Properties properties, final String name, final Set<Integer> defaultValue) {
private static BitSet getPropertyIntegerRangeValue(
final Properties properties, final String name, final BitSet defaultValue) {
final String value = properties.getProperty(name);
try {
return value == null ? defaultValue : parseIntegerRangeSet(value, name);
Expand Down Expand Up @@ -1220,7 +1220,7 @@ private static Map<String, String> parseMap(final String str, final String setti
}

@NonNull
private static Set<Integer> parseIntegerRangeSet(@NonNull String str, final String settingName)
private static BitSet parseIntegerRangeSet(@NonNull String str, final String settingName)
throws NumberFormatException {
str = str.replaceAll("\\s", "");
if (!str.matches("\\d{3}(?:-\\d{3})?(?:,\\d{3}(?:-\\d{3})?)*")) {
Expand All @@ -1231,24 +1231,23 @@ private static Set<Integer> parseIntegerRangeSet(@NonNull String str, final Stri
throw new NumberFormatException();
}

final int lastSeparator = Math.max(str.lastIndexOf(','), str.lastIndexOf('-'));
final int maxValue = Integer.parseInt(str.substring(lastSeparator + 1));
final BitSet set = new BitSet(maxValue);
final String[] tokens = str.split(",", -1);
final Set<Integer> set = new HashSet<>();

for (final String token : tokens) {
final String[] range = token.split("-", -1);
if (range.length == 1) {
set.add(Integer.parseInt(range[0]));
} else if (range.length == 2) {
final int left = Integer.parseInt(range[0]);
final int right = Integer.parseInt(range[1]);
final int separator = token.indexOf('-');
if (separator == -1) {
set.set(Integer.parseInt(token));
} else if (separator > 0) {
final int left = Integer.parseInt(token.substring(0, separator));
final int right = Integer.parseInt(token.substring(separator + 1));
final int min = Math.min(left, right);
final int max = Math.max(left, right);
for (int i = min; i <= max; i++) {
set.add(i);
}
set.set(min, max + 1);
}
}
return Collections.unmodifiableSet(set);
return set;
}

@NonNull
Expand Down
36 changes: 22 additions & 14 deletions dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ class ConfigTest extends DDSpecification {
config.mergedSpanTags == [:]
config.mergedJmxTags == [(RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName]
config.headerTags == [:]
config.httpServerErrorStatuses == (500..599).toSet()
config.httpClientErrorStatuses == (400..499).toSet()
config.httpServerErrorStatuses == toBitSet((500..599))
config.httpClientErrorStatuses == toBitSet((400..499))
config.httpClientSplitByDomain == false
config.dbClientSplitByInstance == false
config.splitByTags == [].toSet()
Expand Down Expand Up @@ -252,8 +252,8 @@ class ConfigTest extends DDSpecification {
config.mergedSpanTags == [b: "2", c: "3"]
config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName]
config.headerTags == [e: "5"]
config.httpServerErrorStatuses == (122..457).toSet()
config.httpClientErrorStatuses == (111..111).toSet()
config.httpServerErrorStatuses == toBitSet((122..457))
config.httpClientErrorStatuses == toBitSet((111..111))
config.httpClientSplitByDomain == true
config.dbClientSplitByInstance == true
config.splitByTags == ["some.tag1", "some.tag2"].toSet()
Expand Down Expand Up @@ -372,8 +372,8 @@ class ConfigTest extends DDSpecification {
config.mergedSpanTags == [b: "2", c: "3"]
config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName]
config.headerTags == [e: "5"]
config.httpServerErrorStatuses == (122..457).toSet()
config.httpClientErrorStatuses == (111..111).toSet()
config.httpServerErrorStatuses == toBitSet((122..457))
config.httpClientErrorStatuses == toBitSet((111..111))
config.httpClientSplitByDomain == true
config.dbClientSplitByInstance == true
config.splitByTags == ["some.tag3", "some.tag2", "some.tag1"].toSet()
Expand Down Expand Up @@ -495,8 +495,8 @@ class ConfigTest extends DDSpecification {
config.serviceMapping == [:]
config.mergedSpanTags == [:]
config.headerTags == [:]
config.httpServerErrorStatuses == (500..599).toSet()
config.httpClientErrorStatuses == (400..499).toSet()
config.httpServerErrorStatuses == toBitSet((500..599))
config.httpClientErrorStatuses == toBitSet((400..499))
config.httpClientSplitByDomain == false
config.dbClientSplitByInstance == false
config.splitByTags == [].toSet()
Expand Down Expand Up @@ -592,8 +592,8 @@ class ConfigTest extends DDSpecification {
config.mergedSpanTags == [b: "2", c: "3"]
config.mergedJmxTags == [b: "2", d: "4", (RUNTIME_ID_TAG): config.getRuntimeId(), (SERVICE_TAG): config.serviceName]
config.headerTags == [e: "5"]
config.httpServerErrorStatuses == (122..457).toSet()
config.httpClientErrorStatuses == (111..111).toSet()
config.httpServerErrorStatuses == toBitSet((122..457))
config.httpClientErrorStatuses == toBitSet((111..111))
config.httpClientSplitByDomain == true
config.dbClientSplitByInstance == true
config.splitByTags == [].toSet()
Expand Down Expand Up @@ -859,10 +859,10 @@ class ConfigTest extends DDSpecification {

then:
if (expected) {
assert config.httpServerErrorStatuses == expected.toSet()
assert config.httpClientErrorStatuses == expected.toSet()
assert propConfig.httpServerErrorStatuses == expected.toSet()
assert propConfig.httpClientErrorStatuses == expected.toSet()
assert config.httpServerErrorStatuses == toBitSet(expected)
assert config.httpClientErrorStatuses == toBitSet(expected)
assert propConfig.httpServerErrorStatuses == toBitSet(expected)
assert propConfig.httpClientErrorStatuses == toBitSet(expected)
} else {
assert config.httpServerErrorStatuses == Config.DEFAULT_HTTP_SERVER_ERROR_STATUSES
assert config.httpClientErrorStatuses == Config.DEFAULT_HTTP_CLIENT_ERROR_STATUSES
Expand Down Expand Up @@ -1575,4 +1575,12 @@ class ConfigTest extends DDSpecification {
throw new Throwable()
}
}

static BitSet toBitSet(Collection<Integer> set) {
BitSet bs = new BitSet()
for (Integer i : set) {
bs.set(i)
}
return bs
}
}
4 changes: 2 additions & 2 deletions dd-trace-core/src/main/java/datadog/trace/core/processor/rule/HttpStatusErrorRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ public void processSpan(
final int status =
value instanceof Integer ? (int) value : Integer.parseInt(value.toString());
if (DDSpanTypes.HTTP_SERVER.equals(span.getType())) {
if (Config.get().getHttpServerErrorStatuses().contains(status)) {
if (Config.get().getHttpServerErrorStatuses().get(status)) {
span.setError(true);
}
} else if (DDSpanTypes.HTTP_CLIENT.equals((span.getType()))) {
if (Config.get().getHttpClientErrorStatuses().contains(status)) {
if (Config.get().getHttpClientErrorStatuses().get(status)) {
span.setError(true);
}
}
Expand Down