Simplify logging

[dwreeves]

Description

This PR addresses #906 and fixes issues in the Cosmos logging once and for all.*

* Actually, there is another issue with logs being polluting with warnings about "source" nodes in 1.5.0, but that is a separate matter!

I have a long explanation of how the logging module in Python works, and the sort of idioms it expects of end users of the module, here: #906 (comment)

The choices I made, explained:

• Although I don't know that I entirely agree with adding (astronomer-cosmos) to all the logs, clearly at least one user, and possibly many more, want it, and I don't believe we should remove it. The objective of this PR was therefore to preserve the feature while future-proofing against future issues.
  • Why I can't say I'm a fan of it: It seems that adding (astronomer-cosmos) to logs is a symptom of other problems with the Cosmos library, specifically how it impacts performance when users do not set it up effectively. And the prefix was added as a way to assist people in diagnosing these issues. I think ultimately we want to move away from this. Other components of the Airflow ecosystem do not feel compelled to do things like this. Also, the module path is something that can be handled in the log_format if users really want it.
  • How I future-proofed: As per the long post I link above, basically the issue is that there should not be tons of StreamHandlers being created. The proper and typical use of the logging module, with few exceptions, is to allow for logs to propagate upwards to a root logger. The reason the Cosmos logs presented issues for so long was because it deviated a lot from this.
• I think default behavior being the "least astonishing" means making no modifications to the base logging behavior whatsoever. This is also less likely to morph into future issues if any further changes are made to the custom logging.
  • One thing I never mentioned: I found it odd that by default Cosmos did not "work out of the box" and that, despite using Astronomer's own Airflow platform (!), I had to set a config option that made Cosmos logging not be a nightmare (i.e. set propagate_logs = false). Previous logs referenced the Celery Executor as having issues, even though this is one of 2 of the most popular production ways to run Airflow. Something like this should just work out of the box for a majority of users!
• For task execution, Cosmos should make use of the more Airflow-idiomatic LoggingMixin class whenever appropriate. This can also be used in scheduler / webserver related logging contexts but I think it is less out-of-place there to use globally scoped loggers.
  • These will not use the get_logger() implementation. That is intentional and probably desirable. These logs do not need to be "enriched" because they are isolated in the task execution logs.

Oh also, I fixed an issue in the project.entry_points in the pyproject.toml while I was at it.

Breaking Change?

• Removes propagate_logging conf option, although removing this will not break users' builds. There is now a rich_logging conf option instead, which by default is disabled.

Checklist

• I have made corresponding changes to the documentation (if required)
• I have added tests that prove my fix is effective or that my feature works

[netlify]

✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.

Name	Link
🔨 Latest commit	3f8ddff
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/66be0e25e57ed30008e9b6be

[codecov]

Codecov Report

Attention: Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.

Project coverage is 96.36%. Comparing base (f7354e5) to head (3f8ddff).
Report is 1 commits behind head on main.

Files	Patch %	Lines
cosmos/operators/local.py	77.27%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1108      +/-   ##
==========================================
- Coverage   96.37%   96.36%   -0.01%     
==========================================
  Files          64       64              
  Lines        3452     3443       -9     
==========================================
- Hits         3327     3318       -9     
  Misses        125      125              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dwreeves]

So what's going on with the status of this PR is: I swapped the logger.info() for self.log.info() (same for error, warning etc.) inside the operators.

It's failing due to coverage for upload_to_cloud_storage(), which was previously not covered, but due to the fact I change how the logging works, it is now being marked as a coverage miss.

@tatiana I am wondering if we can make an exception for the coverage on that? I don't really want to have to write a unit test for code I don't even use right now 😭

The PR otherwise works, is documented, and has unit tests. So I think it should be fine and I removed the "Draft" from it.