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

Logging improvements #172

Merged
merged 5 commits into from
Oct 12, 2023
Merged

Conversation

rcypher-databricks
Copy link
Contributor

Differentiated some cases where the logged messages were ambiguous as to which code path they came from.
Updated client to be more consistent in what context information is included in logging messages. Also updated client methods to produce a duration log message even if there is an error. Updated error handling to log the wrapped error message.

Differentiated some cases where the logged messages were ambiguous as to which code path they came from.
Updated client to be more consistent in what context information is included in logging messages.

Signed-off-by: Raymond Cypher <raymond.cypher@databricks.com>
Logging status/state returned by executing a statement.
The logic for handling context cancel/timeout was not working correctly in the case where there are no direct results returned from execute statement.
Signed-off-by: Raymond Cypher <raymond.cypher@databricks.com>
Signed-off-by: Raymond Cypher <raymond.cypher@databricks.com>
Signed-off-by: Raymond Cypher <raymond.cypher@databricks.com>
logDisplayMessage(resp, log)
logExecStatementState(resp, log)

defer log.Duration(msg, start)

Choose a reason for hiding this comment

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

should we move this to before line 170?

Copy link

@susodapop susodapop left a comment

Choose a reason for hiding this comment

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

LGTM

internal/client/client.go Outdated Show resolved Hide resolved
Co-authored-by: Jesse <jwhitehouse@airpost.net>
Signed-off-by: Raymond Cypher <raymond.cypher@databricks.com>
@rcypher-databricks rcypher-databricks merged commit a9448d6 into databricks:main Oct 12, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants