Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Endpoint tests leave a scheduler running after teardown and can miss unwanted entries in filtered streams.
2 open issues
What changed in this PR
Adds account and token filtering to the logger’s SSE endpoint for #163, with optional tenant scoping and validation before streaming.
Changes:
- Combines tenant, account, and token filters with AND.
- Fixes heartbeats before the first log entry and adds filtering tests.
- Updates the HTML demo and build dependencies.
| File | Description |
|---|---|
| pom.xml | Updates the parent version. |
| osgi-bundles/bundles/logger/src/test/java/org/killbill/billing/osgi/bundles/logger/TestSseParamsCriteria.java | Tests matching and combined criteria. |
| osgi-bundles/bundles/logger/src/test/java/org/killbill/billing/osgi/bundles/logger/TestLogsSseHandler.java | Tests filtering and heartbeat behavior. |
| osgi-bundles/bundles/logger/src/test/java/org/killbill/billing/osgi/bundles/logger/TestLogsSseFilter.java | Tests validation and record ID resolution. |
| osgi-bundles/bundles/logger/src/test/java/org/killbill/billing/osgi/bundles/logger/TestLogsSseEndpoint.java | Adds HTTP streaming integration tests. |
| osgi-bundles/bundles/logger/src/main/java/org/killbill/billing/osgi/bundles/logger/SseParamsCriteria.java | Implements combined matching criteria. |
| osgi-bundles/bundles/logger/src/main/java/org/killbill/billing/osgi/bundles/logger/LogsSseHandler.java | Applies filters and handles heartbeats without IDs. |
| osgi-bundles/bundles/logger/src/main/java/org/killbill/billing/osgi/bundles/logger/LogsSseFilter.java | Validates requests and resolves filter criteria. |
| osgi-bundles/bundles/logger/src/main/java/org/killbill/billing/osgi/bundles/logger/Activator.java | Registers request validation before streaming. |
| osgi-bundles/bundles/logger/pom.xml | Declares Jetty dependencies for endpoint tests. |
| osgi-bundles/bundles/logger/html-sse-demo.md | Documents filters and enables tenant headers. |
Review effort: Balanced
xsalefter
force-pushed
the
new-platform-163
branch
from
October 20, 2026 12:38
abc7abd to
a6b7033
Compare
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
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.
This PR will replace this draft PR: #203 .
Original ticket: #163 : Clients can now narrow the log stream with
accountIdanduserTokenquery parameters, and can optionally send their tenant API key and secret to see only their own tenant's entries. Filters combine with AND, and a malformed or missing combination gets a clear400,401or403before the stream opens.A well-formed request that simply matches nothing stays connected and sends heartbeats. Along the way we found and fixed a quiet bug: a heartbeat sent before any log entry used to throw and silently stop the heartbeat task. The PR also updates the HTML demo.