Skip to content

BP-69: Convert stats and allocator modules to slog (phase 2)#4755

Open
merlimat wants to merge 4 commits intoapache:masterfrom
merlimat:bp-69-slog-stats
Open

BP-69: Convert stats and allocator modules to slog (phase 2)#4755
merlimat wants to merge 4 commits intoapache:masterfrom
merlimat:bp-69-slog-stats

Conversation

@merlimat
Copy link
Copy Markdown
Contributor

Part of the slog migration for BP-69. Tracking issue: #4750.

This PR stands alone — it does not depend on the bookkeeper-common PR (#4754) landing first. The 2 commits are:

  1. 526e9ac7ec BP-69 base: add slog dependency and LICENSE entries — minimal scaffolding:
    • Add io.github.merlimat.slog:slog:0.9.7 to pom.xml (alongside SLF4J; SLF4J stays as the rendering backend).
    • Add lombok.config so @CustomLog generates a slog Logger.
    • Register the slog jar in LICENSE-all.bin.txt, LICENSE-server.bin.txt, LICENSE-bkctl.bin.txt.
  2. db52607eb4 Convert stats and allocator modules from SLF4J to slog — the actual conversion (9 files):
    • stats/bookkeeper-stats-api: AlertStatsLogger, Stats, ThreadRegistry.
    • stats/bookkeeper-stats-providers: CodahaleMetricsProvider, OtelMetricsProvider, PrometheusMetricsProvider, ThreadScopedDataSketchesStatsLogger.
    • stats/utils/StatsDocGenerator.
    • bookkeeper-common-allocator/ByteBufAllocatorImpl.

If the bookkeeper-common PR (#4754) lands first, this PR will rebase cleanly onto it (the base commit here is a subset of what #4754 already contains, and will become empty on rebase).

Conventions applied

  • @Slf4j@CustomLog; LoggerFactory.getLogger(X.class)Logger.get(X.class); static = LOG, instance = log.
  • log.info("text {} {}", a, b)log.info().attr("a", a).attr("b", b).log("text").
  • log.error("msg", throwable)log.error().exception(throwable).log("msg").
  • if (log.isDebugEnabled()) guards removed.

Notes

  • CodahaleMetricsProvider retains one org.slf4j.LoggerFactory reference for the Dropwizard Slf4jReporter.outputTo() call (that API takes an SLF4J Logger); the class's own logs are on slog.
  • AlertStatsLogger.raise(String, Object...) keeps its vararg signature in this PR. The Map-based API change (raise(String, Map<String, Object>)) will land together with the stream/distributedlog conversion so the two sides stay in sync.

Not included

  • No wire protocol, binary format, metadata format, metric, or CLI change.
  • No module outside stats/* + bookkeeper-common-allocator is converted.

Test plan

  • mvn -pl stats/...,bookkeeper-common-allocator -am compile -DskipTests passes
  • Existing stats/* and bookkeeper-common-allocator unit tests still pass
  • Alert logs from AlertStatsLogger continue to render correctly under Logback/Log4j2

Minimal scaffolding commit for the BP-69 slog migration series:

- Add `io.github.merlimat.slog:slog:0.9.7` to the root pom.xml
  dependencyManagement and to the global compile classpath alongside
  the existing SLF4J API (which stays as the rendering backend).
- Add lombok.config at the repo root so `@CustomLog` generates a slog
  `Logger` instead of an SLF4J one.
- Register the slog jar in LICENSE-all.bin.txt, LICENSE-server.bin.txt
  and LICENSE-bkctl.bin.txt so the CI check-binary-license script
  finds it accounted for in the bundled-jars list.

No actual Java file is converted in this commit. Individual module
migrations stack on top of this one.
Part of BP-69. Converts the stats-api, codahale/otel/prometheus metrics
providers, stats-utils, and bookkeeper-common-allocator modules.
Pulls in the MDC propagation fix from merlimat/slog#6, which makes
log4j2 ThreadContext entries visible on slog events emitted via the
Log4j2Logger backend (so %X{key} layouts and appenders that read
event.getContextData().getValue(key) see the caller's MDC).
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.

2 participants