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 precommit gate failing, a misconfigured task, a forbidden-APIs violation, 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:

GateGradle taskWhat trips itHow you fix it
FormattingspotlessJavaCheckwrong import order, whitespace, wildcard imports./gradlew spotlessApply
StylecheckstyleMain / checkstyleTestline length, naming, missing bracesedit by hand
Forbidden APIsforbiddenApisMainSystem.out, new Date(), default-charset String.getBytes(), Math.random()use the approved API
License headerslicenseHeadersmissing SPDX header on a new filepaste the SPDX block
Dependency licensesdependencyLicensesa jar whose LICENSE/NOTICE/SHA is absentadd files under */licenses/
Third-party auditthirdPartyAudita dependency pulls a disallowed transitive classexclude or jarHell-fix it
Logger usageloggerUsageChecklogger.debug("..." + x) string concat, wrong arg countuse {} 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:

  1. Parameterised logging. {} placeholders mean the message string is only built if DEBUG is enabled — no allocation on the hot path when the level is off. This is what loggerUsageCheck enforces and the reason Log4j2 placeholders exist.
  2. Correct level. Per-shard, high-frequency events are DEBUG; cluster-level summaries (e.g. "rerouted N shards") stay INFO. Decide by frequency × operator value.
  3. 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 WARN and 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 / thirdPartyAudit will fail. Always regenerate SHAs and add LICENSE/NOTICE for new coordinates.
  • A "harmless" bump that drags a transitive major version. Always diff the :dependencies output. 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 *IT tests 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/ERROR lines are often load-bearing. Leave them unless the issue explicitly asks.
  • Reformatting the whole file. Run spotlessApply only 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; edit gradle/libs.versions.toml so 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 precommit re-asks from the reviewer.
  • You can read a red gradle-check and name the failing gate (forbidden-APIs vs spotless vs dependencyLicenses) from the log alone.
  • You did one dependency bump where you diff-ed the :dependencies output 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 DEBUG versus INFO.

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.