Add logging through slf4j (#6) - #7
Open
xbt-a4224j wants to merge 5 commits into
Open
xbt-a4224j wants to merge 5 commits into
xbt-a4224j wants to merge 5 commits into
Conversation
INFO is one line per request with its status and how long it took, plus a line for each retry and each connection failure. DEBUG adds the wire in both directions: method, url, headers and body. Nothing at WARN or above, because a failure is already thrown and the exception carries more than a log line would. The tiering follows the official Python and JavaScript SDKs. There is no TYPESAFE_LOG_LEVEL. slf4j has no library-side level setting, and configuring the logging environment is the application's job; the level is set on the io.github.premocloud.typesafe logger like any other library's. Redaction happens in one method, so no call site can leak a credential. The named header set is the union of what the two official SDKs cover, plus a substring check for token and secret, since Builder.header lets a caller add one this SDK never anticipated. The wire calls are guarded by isDebugEnabled so the redacted string is not built when the level is off. Adds five tests covering the level tiering, the wire in both directions, the retry line, silence above INFO, and that the key never appears in any message.
The capturing appender asserts on what was logged, but additivity also sent every event to logback's default console appender, so each run printed the request and response bodies.
Java's default root level is INFO and Spring Boot configures a console appender at it, so a per-request line at INFO is on in every application that never asked for one: a hundred requests, a hundred lines, no configuration. The reference SDKs put the summary at INFO, but a bare Python process has no handler on root and JS defaults its own level to warn, so INFO is off unless an application opts in. Sitting a tier lower reproduces that behaviour here. The wire moves to TRACE with it, which also separates 'show me the requests' from 'show me every body'.
…line Drop Logging.request: call sites use LOG.debug so the level is visible where it is logged. Logging keeps wire() as the only redaction point. Fix the class doc, which still described the INFO/DEBUG tiers.
xbt-a4224j
commented
Sep 20, 2026
|
|
||
| return attemptAsync(template, type, timeout, retryPolicy, 0); | ||
| return attemptAsync(template, type, timeout, retryPolicy, 0, | ||
| new Call("req-" + REQUESTS.incrementAndGet(), requestBody)); |
Contributor
Author
There was a problem hiding this comment.
@garretpremo Should we keep or remove the REQUESTS counter? It's in this PR to help distinguish logs of interleaved async calls, but maybe not worth.
Contributor
There was a problem hiding this comment.
@xbt-a4224j Adding an identifier to requests is super useful! Especially after adding async requests which could add interleaved logs that are indistinguishable from each other.
Suggested change: Generate the identifier as a short random hex id in the logging class rather than a JVM-global counter. This avoids collisions across restarts and replicas at a small cost to readability (which is worth it IMO for a debug line).
// Logging.java
/**
* A short opaque tag for one logical call, shared by every attempt it makes, so the lines of
* interleaved async calls can be told apart. Random rather than counted: no shared state, and no
* collisions across restarts or replicas in an aggregated log.
*/
static String tag() {
return String.format("req-%06x", ThreadLocalRandom.current().nextInt(1 << 24));
}Call lines:
Suggested change
| new Call("req-" + REQUESTS.incrementAndGet(), requestBody)); | |
| new Call(Logging.tag(), requestBody)); |
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
2 participants
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.
Motivation:
Make requests, statuses and retries visible at a log level the app chooses, following the Python reference: same log points as
transport.py:59-94, same redaction asSensitiveHeadersFilter, each log point one level below Python's so a stock Spring Boot app (root INFO) stays silent as a stock Python app (root WARNING) does.Changes:
Logging: slf4j loggerio.github.premocloud.typesafe;wire()is the one redaction point,"***"forSECRET_HEADERSplustoken/secretnamesTypeSafeClient: DEBUG line per request (status, elapsed, request id), per retry, per connection failure; TRACE for headers and bodies both waysbuild.gradle.kts:api(slf4j-api:2.0.16); logback-classic test-onlyLoggingTest: 5 tests on a captured appender, incl. key never logged and nothing emitted at INFOResult:
Stock Spring Boot (root INFO) sees no output.
logging.level.io.github.premocloud.typesafe=DEBUGgives one line per request; TRACE adds the wire withAuthorization=***. 44/44 tests pass.Closes: #6