Clean commits that include non-ASCII characters - #12179
Conversation
2367191 to
6be21d4
Compare
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| // Gradle 4.10.3's native-platform crashes on Linux when restoring non-ASCII environment values, so remove all non-ASCII chars. | ||
| ["CI_COMMIT_TITLE", "CI_COMMIT_MESSAGE", "CI_COMMIT_DESCRIPTION"].each { name -> | ||
| String value = environment[name] | ||
| if (value != null) { | ||
| environment[name] = value.replaceAll("[^\\p{ASCII}]", "") | ||
| } | ||
| } |
There was a problem hiding this comment.
Just curious if this should be on test level?
Maybe similar to the fix I recently did to exclude noise env vars: in PR #11180
public static final Set<String> NOISY_ENVIRONMENT_VARIABLES = ImmutableSet.of('CI_COMMIT_MESSAGE', 'CI_COMMIT_DESCRIPTION')
We may probably exclude all CI_xxx env variables as noisy.
There was a problem hiding this comment.
I think it needs to be at this level to pre-date the Gradle daemon starting 🤔 but yes we could entirely remove the CI_* variables instead of just cleaning them!
There was a problem hiding this comment.
I think we start Gradle 4.10.3 from code. You can quickly check just by commenting your fix and excluding all CI_xxx vars in code.
There was a problem hiding this comment.
Agreed, I would rather do it in the test code to avoid messing with gradle task definition and their cache when possible 🙏
There was a problem hiding this comment.
Removing from just the tests fails: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-java/-/jobs/1941794160
The Gradle daemon is started (I think here) before the env configs are applied (here), and once the non-ASCII is passed to the daemon, it crashes. I'll revert back to the original change for now, but open to other suggestions (or perhaps there is a better place to remove the variables).
There was a problem hiding this comment.
Oh, got it.. we actually have 2 issues that looks similar and confused me...
- We have
smoke-appandCI_xxxvariables leaked into it's logs. - We have Gradle itself and
CI_xxxvariables also leaked into Gradle and affected it.
Just curious if we should delete CI_xxx variables instead of tweaking them? or maybe we can somehow prevent them at all? maybe on GitLab level?
There was a problem hiding this comment.
Yes we can delete all the variables too like I mentioned above. I don't particularly have a preference - I was just implementing a minimal fix here.
There was a problem hiding this comment.
More details
The configured Gradle Test environment strips emoji, accented Latin, CJK, and supplementary Unicode from all three commit metadata variables while preserving ASCII and unset values, including independent-variable handling. This directly exercises the environment passed to smoke-test workers and found no output-shape regression.
📊 Validated against 7 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 2ceeb08 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
🎯 Code Coverage (details) 🔗 Commit SHA: 66dfc3a | Docs | Datadog PR Page | Give us feedback! |
What Does This Do
Remove all non-ASCII characters from "CI_COMMIT_TITLE", "CI_COMMIT_MESSAGE", and "CI_COMMIT_DESCRIPTION" in Gradle smoke tests.
test_smoke: [8, 8/8]job failure when running against commit that starts with🍒and no fix: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-java/-/jobs/1938746622test_smoke: [8, 8/8]job failure when running against commit that starts with🍒and fix: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-java/-/jobs/1938934433Motivation
Gradle 4.10.3's native-platform crashes on Linux when restoring non-ASCII environment values for
test_smoke: [8, 8/8]job, as seen in failures in the two latest patch releases with🍒-prefixed commits:Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]