apache / apache/gravitino

[Improvement] Review stack trace logging for client-driven 403s in GravitinoInterceptionService

Open
#13,109 1 comment 0 reactions 1 assignee Claimed by @nevzheng View on GitHub
improvement
Dominant language
Java
Stars
3.2k
Forks
935
Avg merge
1d 17h
Merged PRs (30d)
339

Description

### What would you like to be improved?

`GravitinoInterceptionService` logs a full stack trace at WARN every time a caller is rejected because they are not a member of the metalake (`ForbiddenException` from `AuthorizationUtils.checkCurrentUser`). This is a client-driven 403, so a busy deployment can write a stack trace per rejected request.

The throwable was added to that WARN in #13057 so the cause stays in the server log when stack traces are omitted from responses. The same PR does not log other client errors on the authentication path (401/403/400), so the two paths now treat client errors differently.

This needs a closer look, and was left out of #13057 to keep that PR's scope tight.

The desired outcome is to confirm whether logging the full stack for client-driven 403s is the right trade-off. It satisfies the #12728 condition to log where the exception is mapped, but may be noisy for routine denials.

### How should we improve?

Open questions:

- Should client-driven 403s on the authorization path log the stack trace, the message only, or log the stack at DEBUG?
- Should the authentication and authorization paths follow one rule for which failures are logged with a stack trace?

Location: `server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java`, `catch (ForbiddenException ex)` in the metadata authorization interceptor.

Related: #12728, #13057.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.