Skip to content

Require explicit locales and charsets in tracer code - #13020

Open
bric3 wants to merge 5 commits into
masterfrom
bdu/explicit-locale-charset
Open

bric3 wants to merge 5 commits into
masterfrom
bdu/explicit-locale-charset

Conversation

@bric3

@bric3 bric3 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

What Does This Do

Require explicit locales and charsets for checked JDK APIs. Add Strings.format(...) in internal-api and use it for locale-independent formatting wherever that dependency already exists. Document intentional JVM defaults and the @SuppressForbidden escape hatch in the perf-review guide, with regression coverage for locale-independent configuration parsing and formatting.

Motivation

The tracer inherits the application's JVM defaults. Identifier case conversion, protocol data and machine-readable formatting need stable results across deployment locales and charsets.

Additional Notes

  • The signatures cover locale and charset defaults. The bundled jdk-unsafe rules also cover default time zones.
  • Module dependencies are unchanged. Lower-level modules keep explicit JDK overloads.
  • Local static-analysis validation excludes Maven instrumentation because the earlier run could not resolve org.sonatype.sisu:sisu-guice:3.1.0:no_aop.

Contributor Checklist

@bric3 bric3 added tag: ai generated Largely based on code generated by an AI or LLM comp: core Tracer core type: refactoring tag: no release notes Changes to exclude from release notes labels Oct 23, 2026 — with ChatGPT Codex Connector
@bric3
bric3 requested a review from dougqh October 23, 2026 16:21
@datadog-prod-us1-3

datadog-prod-us1-3 Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

🎯 Code Coverage (details)
• Patch Coverage: 67.35%
• Overall Coverage: 59.48% (+0.02%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 785aff6 | Docs | Give us feedback!

@pr-commenter

pr-commenter Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Debugger benchmarks

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
ci_job_date 1793106093 1793106419
end_time 2026-10-27T13:03:00 2026-10-27T13:08:27
git_branch master bdu/explicit-locale-charset
git_commit_sha 732a7fe 785aff6
start_time 2026-10-27T13:01:34 2026-10-27T13:07:00
See matching parameters
Baseline Candidate
ci_job_id 2180250000 2180250000
ci_pipeline_id 147797012 147797012
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
git_commit_date 1793105387 1793105387

Summary

Found 5 performance improvements and 0 performance regressions! Performance is the same for 5 metrics, 5 unstable metrics.

scenario Δ mean agg_http_req_duration_min Δ mean agg_http_req_duration_p50 Δ mean agg_http_req_duration_p75 Δ mean agg_http_req_duration_p99 Δ mean throughput
scenario:loop better
[-565.171µs; -469.509µs] or [-7.037%; -5.846%]
better
[-562.989µs; -466.053µs] or [-6.956%; -5.759%]
better
[-565.682µs; -468.505µs] or [-6.959%; -5.763%]
better
[-557.700µs; -419.849µs] or [-6.789%; -5.111%]
better
[+6.286op/s; +9.551op/s] or [+5.155%; +7.832%]
See unchanged results
scenario Δ mean agg_http_req_duration_min Δ mean agg_http_req_duration_p50 Δ mean agg_http_req_duration_p75 Δ mean agg_http_req_duration_p99 Δ mean throughput
scenario:noprobe unstable
[-24.543µs; +37.403µs] or [-7.822%; +11.921%]
unstable
[-33.204µs; +53.984µs] or [-9.563%; +15.548%]
unstable
[-42.139µs; +67.969µs] or [-11.657%; +18.802%]
unstable
[-104.206µs; +113.532µs] or [-8.770%; +9.555%]
same
scenario:basic same same same unstable
[-177.906µs; +65.677µs] or [-15.051%; +5.556%]
same
Request duration reports for reports
gantt
    title reports - request duration [CI 0.99] : candidate=None, baseline=None
    dateFormat X
    axisFormat %s
section baseline
noprobe (347.219 µs) : 313, 381
.   : milestone, 347,
basic (325.935 µs) : 320, 331
.   : milestone, 326,
loop (8.093 ms) : 8030, 8157
.   : milestone, 8093,
section candidate
noprobe (357.609 µs) : 310, 405
.   : milestone, 358,
basic (323.607 µs) : 318, 330
.   : milestone, 324,
loop (7.579 ms) : 7572, 7585
.   : milestone, 7579,
Loading
  • baseline results
Scenario Request median duration [CI 0.99]
noprobe 347.219 µs [313.474 µs, 380.963 µs]
basic 325.935 µs [320.385 µs, 331.486 µs]
loop 8.093 ms [8.03 ms, 8.157 ms]
  • candidate results
Scenario Request median duration [CI 0.99]
noprobe 357.609 µs [309.724 µs, 405.494 µs]
basic 323.607 µs [317.59 µs, 329.624 µs]
loop 7.579 ms [7.572 ms, 7.585 ms]

@dd-octo-sts

dd-octo-sts Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 13.98 s 14.01 s [-1.0%; +0.5%] (no difference)
startup:insecure-bank:tracing:Agent 12.90 s 12.95 s [-1.0%; +0.2%] (no difference)
startup:petclinic:appsec:Agent 17.18 s 17.08 s [-0.3%; +1.4%] (no difference)
startup:petclinic:iast:Agent 17.06 s 16.92 s [-0.1%; +1.8%] (no difference)
startup:petclinic:profiling:Agent 16.77 s 16.79 s [-1.3%; +1.1%] (no difference)
startup:petclinic:sca:Agent 17.12 s 16.42 s [-0.1%; +8.7%] (no difference)
startup:petclinic:tracing:Agent 16.15 s 16.17 s [-1.0%; +0.8%] (no difference)

Commit: 785aff60 · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@pr-commenter

pr-commenter Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Kafka / producer-benchmark

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master bdu/explicit-locale-charset
git_commit_date 1793033870 1793091770
git_commit_sha e7584c7 4840b5d
See matching parameters
Baseline Candidate
ci_job_date 1793093058 1793093058
ci_job_id 2179369988 2179369988
ci_pipeline_id 147738892 147738892
cpu_model Intel(R) Xeon(R) Platinum 8175M CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8175M CPU @ 2.50GHz
jdkVersion 11.0.31 11.0.31
jmhVersion 1.36 1.36
jvm /usr/lib/jvm/java-11-openjdk-amd64/bin/java /usr/lib/jvm/java-11-openjdk-amd64/bin/java
jvmArgs -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=15002 -Dhttps.proxyHost=127.0.0.1 -Dhttps.proxyPort=15002 -Dhttp.nonProxyHosts=localhost *.localhost
kernel_version Linux runner-zfyrx7zua-project-304-concurrent-0-m6xogzdg 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-zfyrx7zua-project-304-concurrent-0-m6xogzdg 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
vmName OpenJDK 64-Bit Server VM OpenJDK 64-Bit Server VM
vmVersion 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean throughput
scenario:not-instrumented/KafkaProduceBenchmark.benchProduce same
scenario:only-tracing-dsm-disabled-benchmarks/KafkaProduceBenchmark.benchProduce same
scenario:only-tracing-dsm-enabled-benchmarks/KafkaProduceBenchmark.benchProduce unsure
[+1032.641op/s; +7974.792op/s] or [+0.842%; +6.505%]

@pr-commenter

pr-commenter Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Kafka / consumer-benchmark

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master bdu/explicit-locale-charset
git_commit_date 1793033870 1793105387
git_commit_sha e7584c7 785aff6
See matching parameters
Baseline Candidate
ci_job_date 1793106649 1793106649
ci_job_id 2180249999 2180249999
ci_pipeline_id 147797012 147797012
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
jdkVersion 11.0.31 11.0.31
jmhVersion 1.36 1.36
jvm /usr/lib/jvm/java-11-openjdk-amd64/bin/java /usr/lib/jvm/java-11-openjdk-amd64/bin/java
jvmArgs -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=15002 -Dhttps.proxyHost=127.0.0.1 -Dhttps.proxyPort=15002 -Dhttp.nonProxyHosts=localhost *.localhost
kernel_version Linux runner-zfyrx7zua-project-304-concurrent-0-rwho6jd7 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-zfyrx7zua-project-304-concurrent-0-rwho6jd7 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
vmName OpenJDK 64-Bit Server VM OpenJDK 64-Bit Server VM
vmVersion 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean throughput
scenario:not-instrumented/KafkaConsumerBenchmark.benchConsume same
scenario:only-tracing-dsm-disabled-benchmarks/KafkaConsumerBenchmark.benchConsume same
scenario:only-tracing-dsm-enabled-benchmarks/KafkaConsumerBenchmark.benchConsume unsure
[-7184.985op/s; -188.067op/s] or [-3.813%; -0.100%]

@bric3
bric3 marked this pull request as ready for review October 27, 2026 14:33
@bric3
bric3 requested review from a team as code owners October 27, 2026 14:33
@bric3
bric3 removed the request for review from a team October 27, 2026 14:33
@bric3
bric3 requested review from ValentinZakharov, jandro996, tylfin and vpellan and removed request for a team October 27, 2026 14:33
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T14:36:58.615663Z 785aff6 Draft marked ready
🔒 Security Review ✅ Completed 2026-10-27T14:37:46.652524Z 785aff6 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@datadog-prod-us1-3 datadog-prod-us1-3 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

More details

The explicit locale and charset choices are consistent with their callers and build constraints. Confidence is limited to static analysis; tests were not run.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 785aff6

@jpbempel jpbempel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for debugger

@dougqh

dougqh commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

On the "keep the default" sites, the useful split seems to be who owns the bytes: us (→ UTF-8), a spec (→ spec charset), or an external party (→ their encoding). Since JDK 18, Charset.defaultCharset() is always UTF-8 and the OS encoding lives in native.encoding, so a couple of the new comments describe something the code doesn't do:

  • PrintStreamWrapper(PrintStream): super(ps) re-encodes with defaultCharset(), so it doesn't preserve the wrapped stream's encoding (on JDK 18+ with a non-UTF-8 console it writes UTF-8 into it). This was already the case, but the comment now says it's deliberate.
  • The log capture file feeds the flare, which we read, so UTF-8 seems like the right choice rather than platform for compatibility.
  • Git output: LocalFSGitInfoExtractor now decodes as UTF-8, but ShellCommandExecutor still uses defaultCharset() for git output. git's log output defaults to UTF-8, so the two readers can disagree on JDK ≤17 with a non-UTF-8 default.

Would a small nativeCharset() helper (native.encoding, falling back to the default) make sense for the genuinely OS-owned subprocess cases? It would also give developers who hit the new forbidden-API error an obvious choice. Also, the locale @defaultMessage could mention Strings.format so people find it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants