diff --git a/.github/ISSUE_TEMPLATE/01-bug.yml b/.github/ISSUE_TEMPLATE/01-bug.yml index 3fe9c7b8931..5cef2c8af39 100644 --- a/.github/ISSUE_TEMPLATE/01-bug.yml +++ b/.github/ISSUE_TEMPLATE/01-bug.yml @@ -249,11 +249,8 @@ body: attributes: label: Logs description: | - Please provide a link to a [GitHub Gist](https://gist.github.com) containing log output. [Terraform Debugging Documentation](https://www.terraform.io/internals/debugging) - - Here's a list of environment variables you can set for logging additional information: - - *TF_LOG=DEBUG* - this one is a must-have as it's printing executed SQLs which helps us to see what is happening underneath. - - *SF_TF_GOSNOWFLAKE_LOG_LEVEL=debug* - if you feel the problem may be connected with the Snowflake driver, please set this one for additional driver logs. + Please provide a link to a [GitHub Gist](https://gist.github.com) containing log output. + Read more about logging in the provider [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/dev/CREATING_ISSUES.md#how-can-i-turn-on-logs). placeholder: https://gist.github.com/example/12345678 - id: additional-information diff --git a/.github/ISSUE_TEMPLATE/02-general-usage.yml b/.github/ISSUE_TEMPLATE/02-general-usage.yml index 9591da908c3..e5ce5dfd55c 100644 --- a/.github/ISSUE_TEMPLATE/02-general-usage.yml +++ b/.github/ISSUE_TEMPLATE/02-general-usage.yml @@ -247,11 +247,8 @@ body: attributes: label: Logs description: | - Please provide a link to a [GitHub Gist](https://gist.github.com) containing log output. [Terraform Debugging Documentation](https://www.terraform.io/internals/debugging) - - Here's a list of environment variables you can set for logging additional information: - - *TF_LOG=DEBUG* - this one is a must-have as it's printing executed SQLs which helps us to see what is happening underneath. - - *SF_TF_GOSNOWFLAKE_LOG_LEVEL=debug* - if you feel the problem may be connected with the Snowflake driver, please set this one for additional driver logs. + Please provide a link to a [GitHub Gist](https://gist.github.com) containing log output. + Read more about logging in the provider [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/dev/CREATING_ISSUES.md#how-can-i-turn-on-logs). placeholder: https://gist.github.com/example/12345678 - id: additional-information diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0882eefb599..c282c602c94 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ To run the tests we have three different commands: - `make test-acceptance` run acceptance tests - `make test-integration` run integration tests -You can run the particular tests form inside your chosen IDE but remember that you have to set `TF_ACC=1` environment variable to run any acceptance tests (the above commands set it for you). It is also worth adding the `TF_LOG=DEBUG` environment variable too, because the output of the execution is much more verbose. +You can run the particular tests form inside your chosen IDE but remember that you have to set `TF_ACC=1` environment variable to run any acceptance tests (the above commands set it for you). It is also worth setting up more verbose logging (check [this section](CREATING_ISSUES.md#how-can-i-turn-on-logs) for more details). ## Making a contribution diff --git a/CREATING_ISSUES.md b/CREATING_ISSUES.md index 5b81ede61d2..2f59d9d75eb 100644 --- a/CREATING_ISSUES.md +++ b/CREATING_ISSUES.md @@ -93,9 +93,19 @@ If you would like to contribute to the project, please follow our [contribution ### How can I debug the issue myself? The provider is simply an abstraction issuing SQL commands through the Go Snowflake driver, so most of the errors will be connected to incorrectly built or executed SQL statements. -To see what SQLs are being run you have to set the `TF_LOG=DEBUG` environment variable. +To see what SQLs are being run you have to set more verbose logging check the [section below](#how-can-i-turn-on-logs). To confirm the correctness of the SQLs, refer to the [official Snowflake documentation](https://docs.snowflake.com/). +### How can I turn on logs? +The provider offers two main types of logging: +- Terraform execution (check [Terraform Debugging Documentation](https://www.terraform.io/internals/debugging)) - you can set it through the `TF_LOG` environment variable, e.g.: `TF_LOG=DEBUG`; it will make output of the Terraform execution more verbose. +- Snowflake communication (using the logs from the underlying [Go Snowflake driver](https://github.com/snowflakedb/gosnowflake)) - you can set it directly in the provider config ([`driver_tracing`](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/1.0.3/docs#driver_tracing-3) attribute), by `SNOWFLAKE_DRIVER_TRACING` environmental variable (e.g. `SNOWFLAKE_DRIVER_TRACING=info`), or by `drivertracing` field in the TOML file. To see the communication with Snowflake (including the SQL commands run) we recommend setting it to `info`. + +As driver logs may seem cluttered, to locate the SQL commands run, search for: +- (preferred) `--terraform_provider_usage_tracking` +- `msg="Query:` +- `msg="Exec:` + ### How can I import already existing Snowflake infrastructure into Terraform? Please refer to [this document](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/guides/resource_migration.md#3-two-options-from-here) as it describes different approaches of importing the existing Snowflake infrastructure into Terraform as configuration. diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 6307341550c..96c87c941a9 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -14,6 +14,13 @@ From this version, the snowflake_oauth_integration_for_partner_applications reso detect changes on the Snowflake side and apply appropriate action from the provider level. This may produce changes after running `terraform plan`, as before the configuration could contain different value than on the Snowflake side. +### Removal of instrumentation library +We decided to remove the instrumentation around the [Go Snowflake driver](https://github.com/snowflakedb/gosnowflake). It does not introduce any functional changes, however, it changes the way the Snowflake communication logs are turned on and how they are printed. Check [this section](CREATING_ISSUES.md#how-can-i-turn-on-logs) for more details. + +`SF_TF_NO_INSTRUMENTED_SQL`, used to turn the instrumentation off, was removed because it is no longer needed. + +These changes should not affect any existing workflows (unless you have custom logic based on the old logs output). + ## v1.0.3 ➞ v1.0.4 ### Fixed external_function VARCHAR return_type diff --git a/Makefile b/Makefile index 2f1c3a8c3c9..ac2b522b136 100644 --- a/Makefile +++ b/Makefile @@ -61,7 +61,7 @@ sweep: ## destroy the whole architecture; USE ONLY FOR DEVELOPMENT ACCOUNTS else echo "Aborting..."; \ fi; -test: test-client ## run unit and integration tests +test: ## run unit and integration tests go test -v -cover -timeout=45m ./... test-acceptance: ## run acceptance tests @@ -73,14 +73,11 @@ test-integration: ## run SDK integration tests test-architecture: ## check architecture constraints between packages go test ./pkg/architests/... -v -test-client: ## runs test that checks sdk.Client without instrumentedsql - SF_TF_NO_INSTRUMENTED_SQL=1 go test ./pkg/sdk/internal/client/... -v - test-object-renaming: ## runs tests in object_renaming_acceptance_test.go TEST_SF_TF_ENABLE_OBJECT_RENAMING=1 go test ./pkg/resources/object_renaming_acceptace_test.go -v test-acceptance-%: ## run acceptance tests for the given resource only, e.g. test-acceptance-Warehouse - TF_ACC=1 TF_LOG=DEBUG SF_TF_ACC_TEST_CONFIGURE_CLIENT_ONCE=true SF_TF_ACC_TEST_ENABLE_ALL_PREVIEW_FEATURES=true go test -run ^TestAcc_$*_ -v -timeout=20m ./pkg/resources + TF_ACC=1 TF_LOG=DEBUG SNOWFLAKE_DRIVER_TRACING=debug SF_TF_ACC_TEST_CONFIGURE_CLIENT_ONCE=true SF_TF_ACC_TEST_ENABLE_ALL_PREVIEW_FEATURES=true go test -run ^TestAcc_$*_ -v -timeout=20m ./pkg/resources build-local: ## build the binary locally go build -o $(BASE_BINARY_NAME) . diff --git a/README.md b/README.md index 7d23a32d474..2368d6631e4 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ Set environment variable `SF_TF_ADDITIONAL_DEBUG_LOGGING` to a non-empty value. ``` ## Additional SQL Client configuration -Currently underlying sql [gosnowflake](https://github.com/snowflakedb/gosnowflake) driver is wrapped with [instrumentedsql](https://github.com/luna-duclos/instrumentedsql). In order to use raw [gosnowflake](https://github.com/snowflakedb/gosnowflake) driver, set environment variable `SF_TF_NO_INSTRUMENTED_SQL` to a non-empty value. +The provider uses the underlying [gosnowflake](https://github.com/snowflakedb/gosnowflake) driver to send SQL commands to Snowflake. By default, the underlying driver is set to error level logging. It can be changed by setting `driver_tracing` field in the configuration to one of (from most to least verbose): - `trace` diff --git a/go.mod b/go.mod index ccf3a85fa32..737a78d6317 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,6 @@ require ( github.com/hashicorp/terraform-plugin-sdk/v2 v2.36.0 github.com/hashicorp/terraform-plugin-testing v1.11.0 github.com/jmoiron/sqlx v1.4.0 - github.com/luna-duclos/instrumentedsql v1.1.3 github.com/pelletier/go-toml/v2 v2.2.3 github.com/snowflakedb/gosnowflake v1.13.0 github.com/stretchr/testify v1.10.0 diff --git a/go.sum b/go.sum index be2a7a647bb..be5d385176b 100644 --- a/go.sum +++ b/go.sum @@ -221,8 +221,6 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/luna-duclos/instrumentedsql v1.1.3 h1:t7mvC0z1jUt5A0UQ6I/0H31ryymuQRnJcWCiqV3lSAA= -github.com/luna-duclos/instrumentedsql v1.1.3/go.mod h1:9J1njvFds+zN7y85EDhN9XNQLANWwZt2ULeIC8yMNYs= github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= diff --git a/pkg/internal/snowflakeenvs/snowflake_environment_variables.go b/pkg/internal/snowflakeenvs/snowflake_environment_variables.go index 8440f0092cd..05553babcd7 100644 --- a/pkg/internal/snowflakeenvs/snowflake_environment_variables.go +++ b/pkg/internal/snowflakeenvs/snowflake_environment_variables.go @@ -47,6 +47,4 @@ const ( DisableConsoleLogin = "SNOWFLAKE_DISABLE_CONSOLE_LOGIN" ConfigPath = "SNOWFLAKE_CONFIG_PATH" - - NoInstrumentedSql = "SF_TF_NO_INSTRUMENTED_SQL" ) diff --git a/pkg/resources/execute_acceptance_test.go b/pkg/resources/execute_acceptance_test.go index c9e593c167a..cccb90a5381 100644 --- a/pkg/resources/execute_acceptance_test.go +++ b/pkg/resources/execute_acceptance_test.go @@ -572,7 +572,7 @@ func TestAcc_Execute_grants(t *testing.T) { // // Quick search unveiled this issue: https://github.com/hashicorp/terraform-plugin-sdk/issues/536. // -// It also seems that it is working correctly underneath; with TF_LOG set to DEBUG we have: +// It also seems that it is working correctly underneath: // // 2023/11/26 17:16:03 [DEBUG] SQL "GRANT CREATE SCHEMA,MODIFY ON DATABASE EXECUTE_TEST_DATABASE_4397 TO ROLE EXECUTE_TEST_ROLE_1145" applied successfully // 2023/11/26 17:16:03 [DEBUG] SQL "GRANT MODIFY,USAGE ON DATABASE EXECUTE_TEST_DATABASE_3740 TO ROLE EXECUTE_TEST_ROLE_3008" applied successfully diff --git a/pkg/sdk/client.go b/pkg/sdk/client.go index 240162c2f1a..9da4fd984c9 100644 --- a/pkg/sdk/client.go +++ b/pkg/sdk/client.go @@ -5,23 +5,12 @@ import ( "database/sql" "fmt" "log" - "os" - "slices" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/tracking" - - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeenvs" "github.com/jmoiron/sqlx" - "github.com/luna-duclos/instrumentedsql" "github.com/snowflakedb/gosnowflake" ) -var instrumentedSQL bool - -func init() { - instrumentedSQL = os.Getenv(snowflakeenvs.NoInstrumentedSql) == "" -} - type Client struct { config *gosnowflake.Config db *sqlx.DB @@ -124,37 +113,17 @@ func NewClient(cfg *gosnowflake.Config) (*Client, error) { cfg = DefaultConfig() } - var client *Client - // register the snowflake driver if it hasn't been registered yet - - driverName := "snowflake" - if instrumentedSQL { - if !slices.Contains(sql.Drivers(), "snowflake-instrumented") { - log.Println("[DEBUG] Registering snowflake-instrumented driver") - logger := instrumentedsql.LoggerFunc(func(ctx context.Context, s string, kv ...interface{}) { - switch s { - case "sql-conn-query", "sql-conn-exec": - log.Printf("[DEBUG] %s: %v (%s)\n", s, kv, ctx.Value(snowflakeAccountLocatorContextKey)) - default: - return - } - }) - sql.Register("snowflake-instrumented", instrumentedsql.WrapDriver(new(gosnowflake.SnowflakeDriver), instrumentedsql.WithLogger(logger))) - } - driverName = "snowflake-instrumented" - } - dsn, err := gosnowflake.DSN(cfg) if err != nil { return nil, err } - db, err := sqlx.Connect(driverName, dsn) + db, err := sqlx.Connect("snowflake", dsn) if err != nil { return nil, fmt.Errorf("open snowflake connection: %w", err) } - client = &Client{ + client := &Client{ // snowflake does not adhere to the normal sql driver interface, so we have to use unsafe db: db.Unsafe(), config: cfg, @@ -182,15 +151,6 @@ func NewClient(cfg *gosnowflake.Config) (*Client, error) { return client, nil } -func NewClientFromDB(db *sql.DB) *Client { - dbx := sqlx.NewDb(db, "snowflake") - client := &Client{ - db: dbx.Unsafe(), - } - client.initialize() - return client -} - func (c *Client) initialize() { c.Accounts = &accounts{client: c} c.Alerts = &alerts{client: c} diff --git a/pkg/sdk/internal/client/client_test.go b/pkg/sdk/internal/client/client_test.go deleted file mode 100644 index c410012a275..00000000000 --- a/pkg/sdk/internal/client/client_test.go +++ /dev/null @@ -1,31 +0,0 @@ -package client - -import ( - "database/sql" - "os" - "testing" - - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeenvs" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// TestNewClientWithoutInstrumentedSQL checks if the client is initialized with the different driver implementation. -// This is dependent on the SF_TF_NO_INSTRUMENTED_SQL env variable setting. That's why it was extracted to another file. -// To run this test use: `make test-client` command. -func TestNewClientWithoutInstrumentedSQL(t *testing.T) { - if os.Getenv(snowflakeenvs.NoInstrumentedSql) == "" { - t.Skipf("Skipping TestNewClientWithoutInstrumentedSQL, because %s is not set", snowflakeenvs.NoInstrumentedSql) - } - - t.Run("registers snowflake-not-instrumented driver", func(t *testing.T) { - config := sdk.DefaultConfig() - config.Tracing = string(sdk.DriverLogLevelDebug) - _, err := sdk.NewClient(config) - require.NoError(t, err) - - assert.NotContains(t, sql.Drivers(), "snowflake-instrumented") - assert.Contains(t, sql.Drivers(), "snowflake") - }) -} diff --git a/pkg/sdk/testint/client_integration_test.go b/pkg/sdk/testint/client_integration_test.go index cbf8a523911..b05e571c800 100644 --- a/pkg/sdk/testint/client_integration_test.go +++ b/pkg/sdk/testint/client_integration_test.go @@ -68,12 +68,12 @@ func TestInt_Client_NewClient(t *testing.T) { require.ErrorContains(t, err, "260000: account is empty") }) - t.Run("registers snowflake-instrumented driver", func(t *testing.T) { + t.Run("registers snowflake driver", func(t *testing.T) { config := sdk.DefaultConfig() _, err := sdk.NewClient(config) require.NoError(t, err) - assert.ElementsMatch(t, sql.Drivers(), []string{"snowflake-instrumented", "snowflake"}) + assert.ElementsMatch(t, sql.Drivers(), []string{"snowflake"}) }) }