Skip to content

Optimize AppSecRequestContext numeric conversion to eliminate exceptions on hot path #10520

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
jandro996 merged 6 commits into from
Feb 6, 2026
Merged

Optimize AppSecRequestContext numeric conversion to eliminate exceptions on hot path #10520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49d9f79
First Version
jandro996 Feb 4, 2026
22a5723
Support scientific notation
jandro996 Feb 4, 2026
fb3d48b
spotless
jandro996 Feb 4, 2026
fce183a
Merge branch 'master' into alejandro.gonzalez/issue-10494
jandro996 Feb 5, 2026
537a668
Move to static utility
jandro996 Feb 6, 2026
4004a3a
Merge branch 'master' into alejandro.gonzalez/issue-10494
jandro996 Feb 6, 2026
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
221 changes: 221 additions & 0 deletions dd-java-agent/appsec/src/jmh/java/datadog/appsec/benchmark/NumericConversionBenchmark.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
package datadog.appsec.benchmark;

import static java.util.Collections.singletonMap;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;

import com.datadog.appsec.gateway.AppSecRequestContext;
import datadog.trace.api.internal.TraceSegment;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Level;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;

/**
* Benchmark for numeric attribute conversion performance (issue #10494).
*
* <p>Tests the optimization that replaces exception-driven parsing with fast-path validation to
* avoid NumberFormatException overhead on invalid inputs.
*
* <p>Expected results: - Valid numeric inputs: no regression (~100-200ns/op) - Invalid inputs:
* 10-100x improvement (from ~1000ns+ with exceptions to <100ns without) - Empty/whitespace: fast
* rejection (<50ns/op)
*/
@State(Scope.Benchmark)
@Warmup(iterations = 4, time = 2, timeUnit = SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = SECONDS)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(NANOSECONDS)
@Fork(value = 3)
public class NumericConversionBenchmark {

static {
BenchmarkUtil.disableLogging();
}

private AppSecRequestContext context;
private TraceSegment mockTraceSegment;

@Setup(Level.Iteration)
public void setUp() {
context = new AppSecRequestContext();
// Use NoOp TraceSegment to minimize overhead
mockTraceSegment = TraceSegment.NoOp.INSTANCE;
}

@Benchmark
public void validInteger(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "42")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void validIntegerNegative(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "-12345")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void validIntegerWithSign(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "+999")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void validDecimal(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "3.14")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void validDecimalNegative(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "-99.5")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void validLargeNumber(Blackhole blackhole) {
context.reportDerivatives(
singletonMap("test_attr", singletonMap("value", "9223372036854775807")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

// Invalid inputs - these should show the biggest improvement (no exceptions thrown)
@Benchmark
public void invalidAlphabetic(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "not_a_number")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void invalidAlphanumeric(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "12x34")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void invalidMultipleDecimals(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "3.14.15")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void invalidHexFormat(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "0x10")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void invalidScientific(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "1e10")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void invalidSpecialChars(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "$100")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

// Whitespace handling - now works correctly after optimization (issue #10494)
@Benchmark
public void whitespaceLeading(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", " 42")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void whitespaceTrailing(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "42 ")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void whitespaceBoth(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", " 42 ")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void whitespaceTabNewline(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "\t100\n")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

// Empty/null - fast rejection
@Benchmark
public void emptyString(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", "")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void whitespaceOnly(Blackhole blackhole) {
context.reportDerivatives(singletonMap("test_attr", singletonMap("value", " ")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

// Overflow cases - should handle gracefully without exceptions
@Benchmark
public void overflowLongMax(Blackhole blackhole) {
context.reportDerivatives(
singletonMap("test_attr", singletonMap("value", "9223372036854775808")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

@Benchmark
public void overflowVeryLarge(Blackhole blackhole) {
context.reportDerivatives(
singletonMap("test_attr", singletonMap("value", "99999999999999999999999")));
boolean result = context.commitDerivatives(mockTraceSegment);
blackhole.consume(result);
}

// Mixed workload - realistic scenario with 80% invalid, 20% valid
@Benchmark
public void mixedWorkload(Blackhole blackhole) {
// Invalid (80%)
context.reportDerivatives(singletonMap("attr1", singletonMap("value", "invalid")));
blackhole.consume(context.commitDerivatives(mockTraceSegment));

context.reportDerivatives(singletonMap("attr2", singletonMap("value", "abc123")));
blackhole.consume(context.commitDerivatives(mockTraceSegment));

context.reportDerivatives(singletonMap("attr3", singletonMap("value", "0x10")));
blackhole.consume(context.commitDerivatives(mockTraceSegment));

context.reportDerivatives(singletonMap("attr4", singletonMap("value", "")));
blackhole.consume(context.commitDerivatives(mockTraceSegment));

// Valid (20%)
context.reportDerivatives(singletonMap("attr5", singletonMap("value", "42")));
blackhole.consume(context.commitDerivatives(mockTraceSegment));
}
}
24 changes: 2 additions & 22 deletions dd-java-agent/appsec/src/main/java/com/datadog/appsec/gateway/AppSecRequestContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import datadog.trace.api.Config;
import datadog.trace.api.http.StoredBodySupplier;
import datadog.trace.api.internal.TraceSegment;
import datadog.trace.util.Numbers;
import datadog.trace.util.stacktrace.StackTraceEvent;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.Closeable;
Expand Down Expand Up @@ -726,27 +727,6 @@ List<StackTraceEvent> getStackTraces() {
return stackTraces;
}

/**
* Attempts to parse a string value as a number. Returns the parsed number if successful, null
* otherwise. Tries to parse as integer first, then as double if it contains a decimal point.
*/
private static Number convertToNumericAttribute(String value) {
if (value == null || value.isEmpty()) {
return null;
}
try {
// Check if it contains a decimal point to determine if it's a double
if (value.contains(".")) {
return Double.parseDouble(value);
} else {
// Try to parse as integer first
return Long.parseLong(value);
}
} catch (NumberFormatException e) {
return null;
}
}

public void reportDerivatives(Map<String, Object> data) {
log.debug("Reporting derivatives: {}", data);
if (data == null || data.isEmpty()) return;
Expand Down Expand Up @@ -968,7 +948,7 @@ public boolean commitDerivatives(TraceSegment traceSegment) {
traceSegment.setTagTop(key, (Number) value);
} else if (value instanceof String) {
// Try to parse as numeric, otherwise use as string
Number parsedNumber = convertToNumericAttribute((String) value);
Number parsedNumber = Numbers.parseNumber((String) value);
if (parsedNumber != null) {
traceSegment.setTagTop(key, parsedNumber);
} else {
Expand Down
102 changes: 102 additions & 0 deletions ...ppsec/src/test/groovy/com/datadog/appsec/gateway/AppSecRequestContextSpecification.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,108 @@ class AppSecRequestContextSpecification extends DDSpecification {
}
}

def "test numeric conversion with edge cases: #description"() {
given:
def context = new AppSecRequestContext()
def mockTraceSegment = Mock(datadog.trace.api.internal.TraceSegment)
def committedTags = [:]

mockTraceSegment.setTagTop(_ as String, _ as Object) >> { String key, Object value ->
committedTags[key] = value
}

when:
context.reportDerivatives(['test_attr': ['value': inputValue]])
context.commitDerivatives(mockTraceSegment)

then:
// When conversion fails (expectedValue is null), the original string value should be preserved
// When conversion succeeds, the converted numeric value should be used
committedTags['test_attr'] == (expectedValue != null ? expectedValue : inputValue)

where:
description | inputValue | expectedValue
// Valid integers
'zero' | '0' | 0L
'positive integer' | '42' | 42L
'negative integer' | '-100' | -100L
'integer with plus sign' | '+999' | 999L
'large valid long' | '9223372036854775807' | 9223372036854775807L
'large negative long' | '-9223372036854775808' | -9223372036854775808L

// Valid decimals
'simple decimal' | '3.14' | 3.14d
'negative decimal' | '-0.5' | -0.5d
'decimal with plus sign' | '+99.99' | 99.99d
'zero decimal' | '0.0' | 0.0d
'decimal with many digits' | '123.456789' | 123.456789d

// Whitespace handling (should now parse after trim - issue #10494 fix)
'leading whitespace integer' | ' 42' | 42L
'trailing whitespace integer' | '42 ' | 42L
'both whitespace integer' | ' 42 ' | 42L
'tab and newline whitespace' | '\t100\n' | 100L
'multiple spaces decimal' | ' -3.14 ' | -3.14d

// Empty and null
'null value' | null | null
'empty string' | '' | null
'whitespace only' | ' ' | null
'tab only' | '\t' | null

// Invalid formats (should return null, original string preserved)
'alphabetic string' | 'abc' | null
'alphanumeric string' | '12x34' | null
'multiple decimals' | '3.14.15' | null
'multiple signs' | '+-5' | null
'sign in middle' | '12-34' | null

// Sign-only strings
'plus sign only' | '+' | null
'minus sign only' | '-' | null
'plus with whitespace' | ' + ' | null

// Overflow cases (should return null gracefully)
'long overflow positive' | '9223372036854775808' | null
'long overflow negative' | '-9223372036854775809' | null
'very large number' | '99999999999999999999999' | null

// Scientific notation (now supported for backward compatibility)
'scientific notation lowercase' | '1e10' | 1.0e10d
'scientific notation uppercase' | '1E10' | 1.0E10d
'scientific with decimal' | '1.5e10' | 1.5e10d
'scientific negative exponent' | '3.5E-7' | 3.5E-7d
'scientific with sign' | '-2.5e+3' | -2.5e+3d
'scientific integer base' | '5e3' | 5000.0d

// Exotic number formats (not supported)
'hexadecimal' | '0x10' | null
'binary' | '0b1010' | null
'octal' | '0777' | 777L

// Edge decimal cases (note: .5 and 5. are valid Java double literals)
'decimal starts with dot' | '.5' | 0.5d
'decimal ends with dot' | '5.' | 5.0d
'dot only' | '.' | null
'multiple dots' | '...' | null

// Regression - ensure existing valid formats still work
'regression valid integer' | '100' | 100L
'regression valid decimal' | '99.5' | 99.5d
'regression negative' | '-50' | -50L

// Special characters
'comma separator' | '1,000' | null
'underscore separator' | '1_000' | null
'currency symbol' | '$100' | null
'percentage' | '50%' | null

// Edge cases with zeros
'zero with plus' | '+0' | 0L
'zero with minus' | '-0' | 0L
'decimal zero variations' | '0.00' | 0.0d
}

def "test getDerivativeKeys with empty derivatives"() {
given:
def context = new AppSecRequestContext()
Expand Down
Loading