aminghadersohi opened a new pull request, #42306: URL: https://github.com/apache/superset/pull/42306
### SUMMARY `AbstractEventLogger.log_with_context()` in `superset/utils/log.py` logs `WARNING:root:Failed to add user to db session: Class 'werkzeug.local.LocalProxy' is not mapped` whenever it tries to bind the current `flask.g.user` back into the DB session to resolve a `user_id`. `flask_appbuilder`'s `before_request` hook sets `g.user = current_user`, which is a `werkzeug.local.LocalProxy` wrapping the actual user object. For guest/anonymous users (e.g. embedded dashboards), the wrapped object is a `GuestUser`/`AnonymousUserMixin`, which is never a DB-mapped SQLAlchemy instance. `db.session.add()` on that proxy always raises `UnmappedInstanceError`, which is caught and logged at `WARNING` level — this is expected, permanent, and unrecoverable for guest/anonymous requests, not an actionable failure. In production this fires 1.3-1.8M times/day and accounts for ~63% of sampled WARN-level logs for the service. An earlier fix (#33025, closed #26020) added a `has_request_context()` guard and a clearer message, but didn't address the root cause: guest/anonymous users can never be added to the session, so the warning was structurally guaranteed to keep firing. This PR: - Checks `sqlalchemy.inspect(actual_user, raiseerr=False)` before attempting `db.session.add()`. If the user isn't a DB-mapped instance (the guest/anonymous case), the add and the log line are both skipped entirely — this is expected behavior, not an error. - For any other, genuinely unexpected exception, keeps a log line but at `debug` level via the module's own `logger`, instead of `WARNING` via the raw `logging` module. No other part of `log_with_context` or event-logging behavior is touched. Internal tracking: SC-114901 ### TESTING INSTRUCTIONS - Added `tests/integration_tests/event_logger_tests.py::test_log_with_context_guest_user_skips_warning`: sets `flask.g.user` to a `GuestUser` and asserts no `WARNING` is logged. - Added `tests/integration_tests/event_logger_tests.py::test_log_with_context_unexpected_add_failure_logs_debug`: forces `db.session.add()` to raise for a DB-mapped user and asserts the failure is logged at `DEBUG` (not `WARNING`). - `pytest tests/integration_tests/event_logger_tests.py tests/unit_tests/utils/log_tests.py` - `pre-commit run --files superset/utils/log.py tests/integration_tests/event_logger_tests.py` ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
