Skip to content

Add logging through slf4j (#6) - #7

Open
xbt-a4224j wants to merge 5 commits into
Premo-Cloud:mainfrom
xbt-a4224j:feat/logging
Open

xbt-a4224j wants to merge 5 commits into
Premo-Cloud:mainfrom
xbt-a4224j:feat/logging

Conversation

@xbt-a4224j

Copy link
Copy Markdown
Contributor

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 as SensitiveHeadersFilter, 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 logger io.github.premocloud.typesafe; wire() is the one redaction point, "***" for SECRET_HEADERS plus token/secret names
  • TypeSafeClient: DEBUG line per request (status, elapsed, request id), per retry, per connection failure; TRACE for headers and bodies both ways
  • build.gradle.kts: api(slf4j-api:2.0.16); logback-classic test-only
  • LoggingTest: 5 tests on a captured appender, incl. key never logged and nothing emitted at INFO
  • README Logging section, CHANGELOG entry

Result:
Stock Spring Boot (root INFO) sees no output. logging.level.io.github.premocloud.typesafe=DEBUG gives one line per request; TRACE adds the wire with Authorization=***. 44/44 tests pass.

Addresses: #6

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.

return attemptAsync(template, type, timeout, retryPolicy, 0);
return attemptAsync(template, type, timeout, retryPolicy, 0,
new Call("req-" + REQUESTS.incrementAndGet(), requestBody));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

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