-
Notifications
You must be signed in to change notification settings - Fork 17
Add trace logs from driver #29
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
Conversation
This reverts commit 3c0e1d7.
collector/config.go
Outdated
@@ -119,6 +119,7 @@ func (c Config) snowflakeConnectionString() (string, error) { | |||
Role: c.Role, | |||
Warehouse: c.Warehouse, | |||
Database: "SNOWFLAKE", | |||
Tracing: "trace", |
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.
This should be configurable, can we have this be confgured via a new configuration parameter to ensure the logs aren't too noisy when not debugging an issue?
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.
+1, having it be configurable would be ideal
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.
Yes, I'll update it this week.
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.
@dehaansa @Dasomeone Updated. I tested running the exporter with and without this flag set, it worked as expected.
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.
Looks fine to me, thanks Daniel!
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.
LGTM, thanks for updating the exporter with this! We'll have to get it into Alloy once it's merged.
@Dasomeone I cannot merge this, can you? |
Of course @kuiperda , apologies forgot you can't on this repo :) |
Add trace logging from the driver. Also add missing 'finished collection' message for deleted tables.