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.

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

chore(docs): custom and built-in event logging class at the same time #20098

Merged

Conversation

wiktor2200
Copy link
Contributor

SUMMARY

There was lack of documentation for using built-in DB based event logger along with custom JSON logger so I've added that.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue: Custom Logging Class not working #15548
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

@wiktor2200 wiktor2200 force-pushed the event-logging-class-builtin-and-custom branch from bfb41e5 to 28a30f2 Compare May 17, 2022 12:54
@wiktor2200
Copy link
Contributor Author

wiktor2200 commented Jun 7, 2022

Could someone verify and approve? @craig-rueda @dpgaspar @villebro @michael-s-molina @zhaoyongjie

@villebro
Copy link
Member

Thanks for this contribution @wiktor2200. In the interest of keeping the documentation as compact and to-the-point as possible, I'm unsure if we need to document how to extend the DBEventLogger (most Superset admins should know this). However, it could be a good idea to mention in the docs that when using a custom event logger, events will no longer be populated in the logs table, and that one will need to extend the DBEventLogger class to do this.

Ping @srinify for thoughts on this?

@wiktor2200
Copy link
Contributor Author

I totally agree, it's not rocket science to add it, but it's worth to mention that using custom will stop logging to logs table and info that it could be quite easily added.

@pull-request-size pull-request-size bot added size/XS and removed size/M labels Jul 6, 2022
@wiktor2200
Copy link
Contributor Author

Hi @villebro! I've updated this doc file as you suggested. Removed additional part, leaving only notice about running custom classes simultaneously. Please take a look.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for iterating on this!

@villebro villebro merged commit d4b59ff into apache:master Jul 12, 2022
akshatsri pushed a commit to charan1314/superset that referenced this pull request Jul 19, 2022
…apache#20098)

* chore: custom and built-in event logging class at the same time

* fix whitespaces

* Update Event Logging docs
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants