Skip to content

fix: use SLF4J logger in circuit breaker callbacks (fixes missing state-transition logs)#107

Open
DominicBM wants to merge 1 commit intomainfrom
fix/circuit-breaker-callback-logging
Open

fix: use SLF4J logger in circuit breaker callbacks (fixes missing state-transition logs)#107
DominicBM wants to merge 1 commit intomainfrom
fix/circuit-breaker-callback-logging

Conversation

@DominicBM
Copy link
Copy Markdown
Contributor

@DominicBM DominicBM commented Apr 23, 2026

Root cause

CircuitBreaker.onOpen/onHalfOpen/onClose callbacks were logging via context.log (Akka Typed's ActorLogger). Those callbacks fire on a Future dispatcher thread — not the actor's message-processing thread. Akka Typed's ActorLogger is not thread-safe: it's guarded by an internal actor-thread check, and when accessed from outside the actor loop it silently drops the message (or throws an AssertionError that is swallowed by the Future chain).

This is why the ElasticSearchResponseHandler rejection warning was appearing (logged inside receiveMessage, safe) while the three state-transition lines from ElasticSearchClient were never appearing — despite 24k rejections proving the breaker was cycling.

Fix

Replace context.log in the three callbacks with the object-level log (LoggerFactory.getLogger(getClass), already declared at line 37 of ElasticSearchClient). A plain SLF4J Logger is thread-safe and works correctly from any thread.

// Before — context.log is unsafe off the actor thread
).onOpen(() => context.log.warn("ElasticSearch circuit breaker opened"))
 .onClose(() => context.log.info("ElasticSearch circuit breaker closed"))
 .onHalfOpen(() => context.log.info("ElasticSearch circuit breaker half-open, testing ES"))

// After — SLF4J logger is thread-safe from any thread
).onOpen(() => log.warn("ElasticSearch circuit breaker opened"))
 .onClose(() => log.info("ElasticSearch circuit breaker closed"))
 .onHalfOpen(() => log.info("ElasticSearch circuit breaker half-open, testing ES"))

Verification

After deploying, aws logs filter-log-events --log-group-name /ecs/api --filter-pattern '"circuit breaker opened"' should return results when the breaker trips.

🤖 Generated with Claude Code

Summary

This PR fixes missing circuit breaker state-transition logs in the ElasticSearch client by replacing thread-unsafe Akka actor logging with thread-safe SLF4J logging in circuit breaker callbacks.

Root Cause

The CircuitBreaker.onOpen(), onClose(), and onHalfOpen() callbacks were using Akka Typed's context.log (ActorLogger). Since these callbacks execute on Future dispatcher threads rather than the actor's message-processing thread, and ActorLogger is not thread-safe with built-in actor-thread checks, log messages were silently dropped or errors were swallowed by the Future chain.

Changes

Updated three circuit breaker event callbacks in ElasticSearchClient.scala (lines 142-144) to use the existing object-level SLF4J logger instance instead of context.log:

  • onOpen(): .onOpen(() => log.warn(...))
  • onClose(): .onClose(() => log.info(...))
  • onHalfOpen(): .onHalfOpen(() => log.info(...))

The log levels and message strings remain unchanged; only the logging mechanism was swapped. SLF4J's Logger is thread-safe and can be safely called from any thread.

Deployment

This is a standard code change with no operational requirements. It will be deployed through the normal CI/CD pipeline and does not require manual pipeline triggers, ECS redeployment commands, or infrastructure configuration changes.

Verification

After deployment, circuit breaker state transitions should now be visible in logs. CloudWatch Logs filter queries for "circuit breaker opened" will return results when the breaker trips.

….log

Akka Typed's context.log is only safe to call from within the actor's
message-processing thread. CircuitBreaker onOpen/onHalfOpen/onClose
callbacks fire on a Future dispatcher thread, so context.log accesses
the ActorLogger from the wrong thread — resulting in the state-transition
log lines never reaching CloudWatch.

Replace context.log with the object-level SLF4J logger (already present
as `log`), which is thread-safe and callable from any thread.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 01d01d58-3e0e-4bec-abbb-7ec13e8655ff

📥 Commits

Reviewing files that changed from the base of the PR and between a5feba5 and 96fb075.

📒 Files selected for processing (1)
  • src/main/scala/dpla/api/v2/search/ElasticSearchClient.scala

Walkthrough

Updated CircuitBreaker event logging callbacks in ElasticSearchClient to use a shared SLF4J log instance instead of Akka Typed's context.log for onOpen, onClose, and onHalfOpen messages.

Changes

Cohort / File(s) Summary
CircuitBreaker Logging
src/main/scala/dpla/api/v2/search/ElasticSearchClient.scala
Replaced context.log with shared SLF4J log instance in CircuitBreaker event callbacks. Log levels and message content remain unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing Akka Typed's context.log with SLF4J logger in circuit breaker callbacks to fix missing state-transition logs.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/circuit-breaker-callback-logging

Comment @coderabbitai help to get the list of available commands and usage tips.

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