Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix a bug where the default HeadersSanitizer does not work in logging decorators #5519

Merged
merged 3 commits into from
Apr 2, 2024
Merged

Fix a bug where the default HeadersSanitizer does not work in logging decorators #5519

merged 3 commits into from
Apr 2, 2024

Commits on Mar 19, 2024

  1. Fix a bug where the default HeadersSanitizer in logging decorators 

    Motivation:

`HeadersSanitizer` was added in line#5188. If no `HeadersSanitizer` is set,
the default santizer masks `Authorization`, `Cookie` `Set-Cookie` and
`Proxy-Authorization` with `****`.

In addition to `HeadersSantizer`, other new features are added to
`LoggingDecoratorBuilder` and the code became messy to maintain
backward compatibility. `LoggingDecoratorBuilder`, `LogWriterBuilder`,
and `LogFormatterBuilder` each have their own properties, so it was not
easy to make the overall operation consistent.

As a result, while `LoggingClient.newDecorator()` santizes sensitive
headers, LoggingClient.builder().requestLogLevel(...).newDecorator()`
did not work. Neither code set a sanitizer, so a default sanitizer
should have been set, but it didn't.

Modifications:

- Refactor `LoggingDecoratorBuilder` to delegate all builder properties
  to either `LogWriterBuilder` or `LogFormatterBuilder`.
- Add missing `@Deprecated` annotation to `LoggingClientBuilder` and
  `LoggingRpcClientBuilder` and `LoggingServiceBuilder`.
  - `@Deprecated` in the super method aren't inspected by IntelliJ line#5395
- Breaking) Change the signature of `LogWriterBuilder.responseCauseFilter()`
  to have `RequestContext` as the first parameter.
  - It may be a bug or a mistake when adding the API. Our APIs
    usually provide `RequestContext` for predicates or handlers.

Result:

`LoggingClient` and `LoggingService` now correctly mask sensitive
headers by default.
    @ikhoon
    ikhoon committed Mar 19, 2024
    Configuration menu
    Copy the full SHA
    8bd8a4e View commit details
    Browse the repository at this point in the history

  2. remove shouldBuildLogWriter

    @ikhoon
    ikhoon committed Mar 19, 2024
    Configuration menu
    Copy the full SHA
    8423005 View commit details
    Browse the repository at this point in the history

  3. remove cruft

    @ikhoon
    ikhoon committed Mar 19, 2024
    Configuration menu
    Copy the full SHA
    cd0203c View commit details
    Browse the repository at this point in the history