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

[CT-1405] [Feature] Decouple event logging functionality #6139

Closed
3 tasks done
peterallenwebb opened this issue Oct 25, 2022 · 0 comments · Fixed by #6291
Closed
3 tasks done

[CT-1405] [Feature] Decouple event logging functionality #6139

peterallenwebb opened this issue Oct 25, 2022 · 0 comments · Fixed by #6291
Assignees
Labels
enhancement New feature or request logging
Milestone

Comments

@peterallenwebb
Copy link
Contributor

peterallenwebb commented Oct 25, 2022

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt functionality, rather than a Big Idea better suited to a discussion

Describe the feature

We should improve the encapsulation and flexibility of our event logging code, most of which is currently located in core/dbt/events/functions.py.

Areas needing improvement:

  • Event logging behavior is tied directly to global variables which are open to unpredictable access
  • Log redirection for testing (as implemented by capture_stdout_logs) and other aspects of the logging setup, are likely thread-unsafe. We run our integration tests multi-threaded, so that might already be a problem
  • We are using the python logging library, but few features of the library
  • The event logging code is hard to test because it is coded directly against stdout and file output rather than i/o streams

Design and implement a class which improves the situation by:

  • Relying on instance variables rather than global variables for configuration
  • Supporting any number of output configurations, each of which has its own type (text/json/protobuf), output stream (passed in rather than created internally), and level (info/warn/debug/etc)
  • (?) Bypassing python's logging module entirely
  • (?) Removing the EVENT_HISTORY global, either encapsulating it in this class, or replacing it with a subscription/callback mechanism

Current logging configuration, as peformed in setup_event_logger(), should be refactored to use the new class.

Describe alternatives you've considered

No response

Who will this benefit?

The improved testability, clarity, and encapsulation should increase the velocity and overall experience of any dbt engineer or community member who needs to work with this code, or perform tests which involve monitoring dbt's output.

Are you interested in contributing this feature?

Yes!

Anything else?

No response

@peterallenwebb peterallenwebb added enhancement New feature or request triage labels Oct 25, 2022
@github-actions github-actions bot changed the title [Feature] Decouple event logging functionality [CT-1405] [Feature] Decouple event logging functionality Oct 25, 2022
@jtcohen6 jtcohen6 added this to the v1.4 milestone Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request logging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants