[GitHub] [incubator-superset] DiggidyDave opened a new pull request #8076: event logging changes

[GitBox <gi...@apache.org>]

DiggidyDave opened a new pull request #8076: event logging changes
URL: https://github.com/apache/incubator-superset/pull/8076
 
 
   - add std logger foundation to event_logger
   - log event for CSV export
   - change "log_this" to more self-documenting "log_requests"
   - add LoggingConfigurator type for user-definable log configuration
   
   ### CATEGORY
   
   Choose one
   
   - [ ] Bug Fix
   - [X] Enhancement (new features, refinement)
   - [ ] Refactor
   - [X] Add tests
   - [ ] Build / Development Environment
   - [ ] Documentation
   
   ### SUMMARY
   Current incarnation of "event logger" is an "abstract base class", but in practice its usefulness is tightly coupled to the DBEventLogger implementation, which in turn is tightly coupled to a mandatory db migration ('logs' table). The base class itself has a completely untyped interface (args and kwargs), but the db event logger relies on very specific, but not codified-via-interface fields to be in the incoming messages.  Those fields are put there by the `log_this` decorator which is scattered across the codebase, which means you can't actually use this AbstractEventLogger outside of the `log_this` decorator unless you know the specific schema that the DBEventLogger expects to find in the untyped args and kwargs data it receives.
   
   This change is a first step to move logging towards an actual standard logging based approach, and brings the following changes:
   - all incoming events are logged to a configured `superset_events` logger
   - `log_this` is renamed to `log_requests` to more clearly document what it is passing and persisting to the `DBEventLogger`, and now is logging an event through event logger prior to sending to `DBEventLogger`
   - `LoggingConfigurator` is added to allow local installations to configure a custom handler for events
   - query logger is now logging a `query` event to to event logger prior to calling the configured query logger
   - CSV export is writing a `csv_export` event to logs
   
   Future work:
   - move `log_requests` out of AbstactEventLogger base and into its own type which can log events and remove the existing call to `log` (leaving only `log_event` call)
   - turn `DBEventLogger` into a `logging.Handler` impl that looks for `endpoint_invocation` events and does the db persistence that it does today
   - migrate `AbstractEventLogger` to `EventLogger` after 
   - remove `QUERY_LOGGER`, that can be handled by registering logging.Handler impls that act upon incoming `query` event types.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   ### REVIEWERS
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org