-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
feat: Add logging for ssh tunneling test_connection attempts #22625
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22625 +/- ##
==========================================
+ Coverage 67.07% 67.12% +0.05%
==========================================
Files 1869 1869
Lines 71591 72128 +537
Branches 7822 7822
==========================================
+ Hits 48018 48418 +400
- Misses 21544 21681 +137
Partials 2029 2029
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -105,7 +117,7 @@ def run(self) -> None: # pylint: disable=too-many-statements | |||
ssh_tunnel = SSHTunnel(**ssh_tunnel) | |||
|
|||
event_logger.log_with_context( | |||
action="test_connection_attempt", | |||
action=get_log_connection_action("test_connection_attempt", ssh_tunnel), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there tests for these?
engine=database.db_engine_spec.__name__, | ||
) | ||
# bubble up the exception to return a 408 | ||
raise ex | ||
except Exception as ex: | ||
event_logger.log_with_context( | ||
action=f"test_connection_error.{ex.__class__.__name__}", | ||
action=get_log_connection_action("test_connection_error", ssh_tunnel), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add param for exc
SUMMARY
With ssh tunneling being rolled out, we want to start instrument and verify these connections are successful. To do this i've created a new function to append context of the ssh tunneling to the
action
field so ETL can parse this accordingly.Another approach is creating a new column specifically to annotate whether this connection has ssh tunnel enabled.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION