feat(java): observability contrib middleware#346
Merged
Conversation
TaskitoObservation wraps each task in a Micrometer Observation (metrics + trace span, OTel-pluggable); SentryMiddleware reports failures and dead-letters. Both deps are compileOnly so consumers opt in.
Observation success/error/filter via TestObservationRegistry; Sentry no-op safety + filter.
📝 WalkthroughWalkthroughThis PR adds an optional ChangesObservability middleware integrations
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant TaskExecutor
participant TaskitoObservation
participant ObservationRegistry
participant SentryMiddleware
participant Sentry
TaskExecutor->>TaskitoObservation: before(context)
TaskitoObservation->>ObservationRegistry: create & start Observation
TaskExecutor->>TaskExecutor: run task
alt task succeeds
TaskExecutor->>TaskitoObservation: after(context, result)
TaskitoObservation->>ObservationRegistry: stop Observation
else task fails
TaskExecutor->>TaskitoObservation: onError(context, error)
TaskitoObservation->>ObservationRegistry: mark error & stop
TaskExecutor->>SentryMiddleware: onError(context, error)
SentryMiddleware->>Sentry: captureException (tagged)
end
Related Issues: Not specified in the provided diff. Related PRs: Not specified in the provided diff. Suggested labels: enhancement, java-sdk, observability Suggested reviewers: Not specified in the provided diff. 🐰 A rabbit hops through Sentry's glow, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
sdks/java/src/main/java/org/byteveda/taskito/contrib/SentryMiddleware.java (2)
24-26: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against null
taskFilter.If a caller passes
null,taskFilter.test(...)inonError/onDeadLetterwill NPE on every task, silently breaking failure reporting for the whole application.🛡️ Proposed fix
+import java.util.Objects; + public SentryMiddleware(Predicate<String> taskFilter) { - this.taskFilter = taskFilter; + this.taskFilter = Objects.requireNonNull(taskFilter, "taskFilter"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/contrib/SentryMiddleware.java` around lines 24 - 26, The SentryMiddleware constructor currently accepts a nullable taskFilter, which later causes taskFilter.test(...) in onError/onDeadLetter to NPE and break failure reporting. Update SentryMiddleware to reject null at construction time or fall back to a safe default Predicate in the constructor, and keep the null-safety consistent wherever taskFilter is used.
41-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winonDeadLetter drops richer failure context.
Only the message string is forwarded to Sentry; retry count/
willRetryfromOutcomeEventaren't attached, limiting debuggability of dead-lettered events in Sentry compared to what's available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/contrib/SentryMiddleware.java` around lines 41 - 51, The onDeadLetter handler in SentryMiddleware only sends a message string to Sentry and drops useful OutcomeEvent context. Update the Sentry.withScope block in onDeadLetter to attach retry-related fields such as event.retryCount and event.willRetry as tags or extras alongside the existing task/job tags, and keep the captured message focused on the dead-letter failure so Sentry shows the full event context for debugging.sdks/java/src/test/java/org/byteveda/taskito/contrib/SentryMiddlewareTest.java (1)
20-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd symmetric filter test for
onDeadLetter.Only
onError's filter-skip is tested;onDeadLetteruses the same filter but isn't covered.✅ Suggested addition
+ `@Test` + void filterSkipsUnreportedDeadLetterTasks() { + SentryMiddleware middleware = new SentryMiddleware(task -> task.equals("included")); + assertDoesNotThrow(() -> middleware.onDeadLetter( + new OutcomeEvent(EventName.DEAD, "j", "excluded", "err", 0, false))); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/test/java/org/byteveda/taskito/contrib/SentryMiddlewareTest.java` around lines 20 - 24, Add a symmetric filter-skip test for SentryMiddleware.onDeadLetter, since SentryMiddlewareTest.filterSkipsUnreportedTasks only covers onError. Reuse the same predicate setup used in filterSkipsUnreportedTasks and verify that calling onDeadLetter with an excluded task does not throw, mirroring the existing onError assertion so both code paths using the filter are covered.sdks/java/src/main/java/org/byteveda/taskito/contrib/TaskitoObservation.java (1)
59-72: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap
stop()body in try/finally to avoid a leaked observation.If
scope.close()throws,observation.stop()is skipped, leaving the observation's timer running / span unclosed indefinitely.🔧 Proposed fix
private void stop(TaskContext context, Throwable error) { Observation.Scope scope = (Observation.Scope) context.attributes().remove(SCOPE); - if (scope != null) { - scope.close(); - } Observation observation = (Observation) context.attributes().remove(OBSERVATION); - if (observation == null) { - return; - } - if (error != null) { - observation.error(error); + try { + if (scope != null) { + scope.close(); + } + } finally { + if (observation != null) { + if (error != null) { + observation.error(error); + } + observation.stop(); + } } - observation.stop(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/contrib/TaskitoObservation.java` around lines 59 - 72, Wrap the body of TaskitoObservation.stop in a try/finally so the observation is always stopped even if Observation.Scope.close throws. Keep the current lookup/removal of SCOPE and OBSERVATION in stop(TaskContext, Throwable), but move observation.error(error) and observation.stop() into the finally path, guarding observation being null, so a failure while closing the scope cannot prevent cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sdks/java/src/main/java/org/byteveda/taskito/contrib/SentryMiddleware.java`:
- Around line 24-26: The SentryMiddleware constructor currently accepts a
nullable taskFilter, which later causes taskFilter.test(...) in
onError/onDeadLetter to NPE and break failure reporting. Update SentryMiddleware
to reject null at construction time or fall back to a safe default Predicate in
the constructor, and keep the null-safety consistent wherever taskFilter is
used.
- Around line 41-51: The onDeadLetter handler in SentryMiddleware only sends a
message string to Sentry and drops useful OutcomeEvent context. Update the
Sentry.withScope block in onDeadLetter to attach retry-related fields such as
event.retryCount and event.willRetry as tags or extras alongside the existing
task/job tags, and keep the captured message focused on the dead-letter failure
so Sentry shows the full event context for debugging.
In
`@sdks/java/src/main/java/org/byteveda/taskito/contrib/TaskitoObservation.java`:
- Around line 59-72: Wrap the body of TaskitoObservation.stop in a try/finally
so the observation is always stopped even if Observation.Scope.close throws.
Keep the current lookup/removal of SCOPE and OBSERVATION in stop(TaskContext,
Throwable), but move observation.error(error) and observation.stop() into the
finally path, guarding observation being null, so a failure while closing the
scope cannot prevent cleanup.
In
`@sdks/java/src/test/java/org/byteveda/taskito/contrib/SentryMiddlewareTest.java`:
- Around line 20-24: Add a symmetric filter-skip test for
SentryMiddleware.onDeadLetter, since
SentryMiddlewareTest.filterSkipsUnreportedTasks only covers onError. Reuse the
same predicate setup used in filterSkipsUnreportedTasks and verify that calling
onDeadLetter with an excluded task does not throw, mirroring the existing
onError assertion so both code paths using the filter are covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f45ec2da-8791-44f6-9248-5255d8ea6425
📒 Files selected for processing (6)
sdks/java/build.gradle.ktssdks/java/src/main/java/org/byteveda/taskito/contrib/SentryMiddleware.javasdks/java/src/main/java/org/byteveda/taskito/contrib/TaskitoObservation.javasdks/java/src/main/java/org/byteveda/taskito/contrib/package-info.javasdks/java/src/test/java/org/byteveda/taskito/contrib/ObservationMiddlewareTest.javasdks/java/src/test/java/org/byteveda/taskito/contrib/SentryMiddlewareTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds two optional observability middlewares under
org.byteveda.taskito.contrib. Both peer deps arecompileOnly— out of the default fat JAR and GraalVM path; consumers opt in.TaskitoObservation— wraps each task in a MicrometerObservation: one instrumentation yielding both a metric (timer) and a trace span. Backend-agnostic — plug in OpenTelemetry (or any) by configuring theObservationRegistrythe app passes in.before, scope opened for the handler, stopped inafter/onError(records the error). State carried on the per-taskTaskContext.attributes().name+taskFilterto scope which tasks are instrumented.SentryMiddleware— thin error-capture: reports a task failure to Sentry inonErrorwith task context. Not a parallel metrics/tracing stack — just error capture.Test
ObservationMiddlewareTest— usesmicrometer-observation-testto assert an observation starts/stops per task and records errors.SentryMiddlewareTest— asserts a failure is captured../gradlew buildgreen (JDK 17 build leg + 21/25 test legs).Summary by CodeRabbit
New Features
Documentation
Tests