Skip to content

BP-69: Convert remaining SLF4J usages in non-test code to slog#4765

Open
merlimat wants to merge 4 commits intoapache:masterfrom
merlimat:bp-69-remove-slf4j-main
Open

BP-69: Convert remaining SLF4J usages in non-test code to slog#4765
merlimat wants to merge 4 commits intoapache:masterfrom
merlimat:bp-69-remove-slf4j-main

Conversation

@merlimat
Copy link
Copy Markdown
Contributor

Summary

Final pass to retire SLF4J from production-style code paths. After this, the only remaining LoggerFactory.getLogger() in non-test code is the intentional Codahale Slf4jReporter integration in CodahaleMetricsProvider, which forwards stats to a named SLF4J logger and must stay on the SLF4J API.

Conversion patterns (same as earlier BP-69 phases)

  • LoggerFactory.getLogger(Foo.class) / @Slf4j → Lombok @CustomLog (the top-level lombok.config already maps it to a slog Logger).
  • LOG.info("text {} {}", a, b)log.info().attr("nameA", a).attr("nameB", b).log("text") — values become typed structured attributes with meaningful names.
  • LOG.error("msg", exception)log.error().exception(exception).log("msg"); .exceptionMessage(e) where appropriate.
  • if (LOG.isDebugEnabled()) LOG.debug("...{}", expr)log.debug().attr("name", () -> expr).log("...") — using the lazy-supplier overload so eager evaluation is avoided when the level is disabled.

No log output format changes — slog with the SLF4J/log4j2 backend produces equivalent output.

Scope

48 files across:

Module Files
bookkeeper-common-allocator 2
bookkeeper-common 8
bookkeeper-server 23
circe-checksum 1
cpu-affinity 2
tests/integration-tests-utils 5
tests/integration-tests-topologies 5

IOUtils SLF4J overload removal

IOUtils.close() had four overloads — two slog and two SLF4J ("kept for transition during the slog migration" per the doc comment). The transition is now complete; no remaining callers in non-test code use the SLF4J overloads. They are removed in this PR.

Test plan

  • mvn compile checkstyle:check clean across the 7 touched modules
  • Local sample test runs (62 tests across Test*Configuration*, *DataIntegrity*, FutureUtilsTest) pass
  • Full CI matrix green

Final pass to retire SLF4J from production-style code paths. After this,
the only remaining LoggerFactory.getLogger() in non-test code is the
intentional Codahale Slf4jReporter integration in CodahaleMetricsProvider,
which forwards stats to a named SLF4J logger and must stay on the SLF4J
API.

Conversion patterns (same as earlier BP-69 phases):
- LoggerFactory.getLogger(Foo.class) / @slf4j → @CustomLog
- LOG.info("text {} {}", a, b) → log.info().attr("name1", a).attr("name2", b).log("text")
- LOG.error("msg", exception) → log.error().exception(exception).log("msg")
- if (LOG.isDebugEnabled()) LOG.debug("...{}", expr)
    → log.debug().attr("name", () -> expr).log("...")
  using the lazy-supplier overload to preserve zero-overhead-when-disabled
  semantics on attrs whose value is a method call.

Files: 48 across bookkeeper-common-allocator, bookkeeper-common,
bookkeeper-server, circe-checksum, cpu-affinity,
tests/integration-tests-utils, tests/integration-tests-topologies.

IOUtils: removed the two SLF4J overloads of close() (kept "for transition
during the slog migration" per the doc comment). The transition is
complete — no remaining callers in non-test code.
The `cube` and `dockerClientExecutor` fields are populated by Arquillian
via reflection. Removing the SLF4J `LOG` field as part of the slog
conversion left those fields as the only declared instance state, which
SpotBugs now flags as UWF_UNWRITTEN_FIELD / NP_UNWRITTEN_FIELD.
Annotate the class to suppress these warnings.
The slog conversion in this PR changed the exception-handler log message
from "Triggered exceptionHandler of Component: {}" (with placeholder)
to "Triggered exceptionHandler of Component" (component name moved to
a structured attr). Drop the trailing colon from the test's substring
match so it matches the new literal message.
Slog attributes are forwarded to log4j2 as ContextData (the same channel
SLF4J's MDC uses). Add %X to every PatternLayout in the user-facing
log4j2 configs so the structured attributes attached via .attr(...)
appear in log output by default. Drop the now-redundant *MDC variant
appenders and the directentrylogger override that targeted them.

Also add conf/log4j2.otel-json.xml plus conf/otel-log-template.json as
an example layout that emits each event as a JSON object aligned with
the OpenTelemetry log data model (severity_text, severity_number, body,
attributes, resource, instrumentation_scope, trace_id/span_id from MDC).
The example uses log4j-layout-template-json, which must be added to the
classpath alongside log4j-core to enable JsonTemplateLayout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant