Stage 2 — Build, Dependency, and Logging
What this stage teaches
Stage 2 moves you from prose into the build system and the logging substrate — the two places where you can make a real, low-risk code change while the blast radius stays small. The skills:
- Read and fix Gradle build issues: a
precommitgate failing, a misconfigured task, aforbidden-APIsviolation, a missing license header, a Spotless formatting break. - Do a dependency version bump correctly: edit the version catalog
(
gradle/libs.versions.toml), regenerate the dependency report, update the SHA/license metadata, and prove nothing else moved. - Improve a log line: fix a misleading message, correct a wrong level, convert string
concatenation to Log4j2 parameterised logging, and pass
loggerUsageCheck.
You are still working in surgical PRs, but now they compile, run gates, and occasionally change a number that ripples through CI. The reward for staying in this stage is that you learn to read CI failures fluently, which every later stage needs.
Prerequisite: Stage 1. This stage assumes the fork → DCO → CHANGELOG →
precommit→ PR loop is muscle memory.
The build gates you will meet
./gradlew precommit is an umbrella over a dozen checks. Know what each one means so you
can read the failure:
| Gate | Gradle task | What trips it | How you fix it |
|---|---|---|---|
| Formatting | spotlessJavaCheck | wrong import order, whitespace, wildcard imports | ./gradlew spotlessApply |
| Style | checkstyleMain / checkstyleTest | line length, naming, missing braces | edit by hand |
| Forbidden APIs | forbiddenApisMain | System.out, new Date(), default-charset String.getBytes(), Math.random() | use the approved API |
| License headers | licenseHeaders | missing SPDX header on a new file | paste the SPDX block |
| Dependency licenses | dependencyLicenses | a jar whose LICENSE/NOTICE/SHA is absent | add files under */licenses/ |
| Third-party audit | thirdPartyAudit | a dependency pulls a disallowed transitive class | exclude or jarHell-fix it |
| Logger usage | loggerUsageCheck | logger.debug("..." + x) string concat, wrong arg count | use {} placeholders |
Run the umbrella, but when it fails, re-run just the failing task to iterate fast:
./gradlew precommit
./gradlew forbiddenApisMain -q # iterate on one gate
./gradlew :server:checkstyleMain -q
Finding Stage 2 issues
is:issue is:open label:"good first issue" no:assignee "dependency" in:title,body
is:issue is:open label:"good first issue" no:assignee "log" in:title,body
is:issue is:open label:"help wanted" no:assignee label:"dependencies"
is:issue is:open label:"enhancement" no:assignee "logging" in:title
Dependency bumps are also raised by Dependabot PRs; a good Stage 2 task is to finish a Dependabot bump that needs the license/CHANGELOG plumbing the bot cannot do, or to do the bump Dependabot skipped because the dependency is pinned in the version catalog.
Fallback grep for a logging candidate — find string-concatenated log calls that the logger check would flag if they were added today, and misleading levels:
# String concatenation inside a log call (parameterise these):
grep -rn 'logger\.\(info\|debug\|warn\|error\)("[^"]*"\s*+' server/src/main/java/ | head
# logger.info that looks like it should be debug (fires per-shard / per-request):
grep -rn 'logger.info' server/src/main/java/org/opensearch/index/ | head
Walked example 1 — a dependency version bump
Illustrative of the pattern. The exact catalog key and version vary by branch.
Symptom: an issue asks to bump a low-risk library (say a logging or compression dep) to pick up a CVE fix. OpenSearch centralises versions in a version catalog.
Locate the version
find . -name "libs.versions.toml"
grep -n "log4j\|jackson\|netty\|commons-" gradle/libs.versions.toml
The catalog looks like:
[versions]
log4j = "2.21.0"
jackson = "2.17.0"
netty = "4.1.110.Final"
[libraries]
log4japi = { group = "org.apache.logging.log4j", name = "log4j-api", version.ref = "log4j" }
Diff
--- a/gradle/libs.versions.toml
+++ b/gradle/libs.versions.toml
@@ [versions]
-log4j = "2.21.0"
+log4j = "2.21.1"
Prove the change is isolated
A version bump's risk is the transitive fan-out. Show the before/after dependency tree:
./gradlew :server:dependencies --configuration runtimeClasspath > /tmp/before.txt
# apply the bump, then:
./gradlew :server:dependencies --configuration runtimeClasspath > /tmp/after.txt
diff /tmp/before.txt /tmp/after.txt
If the diff shows only the lines you expected (the bumped artifact and nothing else), the bump is clean. If a transitive dependency also jumped, say so in the PR — reviewers care about the second-order moves.
Update license metadata and run the gates
When a jar's coordinates change, the dependencyLicenses gate needs the matching
LICENSE.txt/NOTICE.txt and SHA file under the module's licenses/ directory:
find . -path "*licenses*log4j*"
./gradlew :server:dependencyLicenses -q # tells you exactly which SHA is stale
./gradlew updateShas # regenerate the SHA-1 files if the task exists
./gradlew :server:thirdPartyAudit -q
./gradlew :server:precommit
CHANGELOG and PR
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ ### Changed
+- Bump `org.apache.logging.log4j:log4j-api` from 2.21.0 to 2.21.1 ([#NNNNN](https://github.com/opensearch-project/OpenSearch/pull/NNNNN))
git add gradle/libs.versions.toml server/licenses/ CHANGELOG.md
git commit -s -m "Bump log4j-api from 2.21.0 to 2.21.1"
git push origin deps/bump-log4j-api
gh pr create --repo opensearch-project/OpenSearch --fill
Walked example 2 — fixing a misleading / wrongly-levelled log line
Illustrative of the pattern. Run the grep to find the real site; class and line vary.
Symptom: a node logs at INFO on every shard relocation:
logger.info("relocating shard " + shardId + " from " + sourceNode + " to " + targetNode);
Two problems: (1) string concatenation instead of Log4j2 placeholders — fails
loggerUsageCheck if anyone touches it; (2) it is INFO, so a large rebalancing floods
the cluster-manager (formerly master) log with thousands of lines, drowning real events.
Operators have asked for it to be DEBUG.
Locate the code
grep -rn "relocating shard" server/src/main/java/org/opensearch/ | head
git log --oneline -n 5 -- <the file the grep printed>
git blame -L <start>,<end> <that file>
git blame tells you who last touched the line; consider mentioning them in the PR if the
level choice was deliberate.
Diff
--- a/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java
+++ b/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java
@@
- logger.info("relocating shard " + shardId + " from " + sourceNode + " to " + targetNode);
+ logger.debug("relocating shard [{}] from [{}] to [{}]", shardId, sourceNode, targetNode);
Three improvements in one diff:
- Parameterised logging.
{}placeholders mean the message string is only built ifDEBUGis enabled — no allocation on the hot path when the level is off. This is whatloggerUsageCheckenforces and the reason Log4j2 placeholders exist. - Correct level. Per-shard, high-frequency events are
DEBUG; cluster-level summaries (e.g. "rerouted N shards") stayINFO. Decide by frequency × operator value. - Bracketed identifiers. OpenSearch convention wraps IDs in
[...]so they are greppable in logs and so an empty value is visible as[]rather than vanishing.
Warning: Do not "fix" a level you are unsure about. If the line is
WARNand you think it is benign, the maintainer who wrote it may know it precedes a real failure. Argue the change in the issue first, with a sentence on how often it fires.
Validate
./gradlew :server:compileJava -q
./gradlew loggerUsageCheck -q # the gate that specifically checks log calls
./gradlew :server:precommit
There is usually no functional test for a log-line change. The reviewer signal is the diff plus, if you want to be thorough, a manual run:
./gradlew run & # single-node from source, REST on :9200
# create an index, add a node-less rebalance scenario, then:
grep "relocating shard" logs/opensearch.log | head
Document that grep in the PR description. Then CHANGELOG (Changed), git commit -s, PR.
Pitfalls
- Bumping a dependency without the license files.
dependencyLicenses/thirdPartyAuditwill fail. Always regenerate SHAs and addLICENSE/NOTICEfor new coordinates. - A "harmless" bump that drags a transitive major version. Always
diffthe:dependenciesoutput. A patch bump that pulls a new minor of a transitive jar is a different, riskier PR. - Changing a log message that a test or alert greps for. Some
*ITtests and external tooling match on log text.grep -rn "the old message" server/src/test qa/before you change it. - Lowering a level that gates a deprecation or security message.
WARN/ERRORlines are often load-bearing. Leave them unless the issue explicitly asks. - Reformatting the whole file. Run
spotlessApplyonly after your change; do not let it (or your IDE) reflow unrelated lines into your diff. Reviewers reject noise. - Forgetting the version catalog is the single source. Do not pin a version inline in a
build.gradle; editgradle/libs.versions.tomlso every module stays consistent.
Exit criteria — when you're ready for Stage 3
- You have at least two logging or build/dependency PRs merged with no
precommitre-asks from the reviewer. - You can read a red
gradle-checkand name the failing gate (forbidden-APIs vs spotless vs dependencyLicenses) from the log alone. - You did one dependency bump where you
diff-ed the:dependenciesoutput and stated the transitive impact in the PR. - You can explain, in one sentence each, why Log4j2 placeholders exist and when a log line
belongs at
DEBUGversusINFO.
Stage 2 keeps you in build-and-logging scaffolding. Stage 3 takes the same fluency into the behaviour of the system: the exception messages and validation that users actually see when something goes wrong.