From ecf0019895846b4ac6f227dcfbc6efc224d42961 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Fri, 30 Sep 2022 19:23:25 +0530 Subject: [PATCH 1/4] go-sql refactoring --- go/go-sql/README.md | 46 ++++++----------- go/go-sql/go-sql.go | 103 +++++---------------------------------- go/go-sql/go-sql_test.go | 54 ++++++++++++-------- go/go-sql/go.mod | 8 +-- go/go-sql/go.sum | 6 ++- 5 files changed, 70 insertions(+), 147 deletions(-) diff --git a/go/go-sql/README.md b/go/go-sql/README.md index d19afe0c..f8aeb1e2 100644 --- a/go/go-sql/README.md +++ b/go/go-sql/README.md @@ -1,4 +1,4 @@ -# Sqlcommenter [In development] +# go-sql-driver [In development] SQLcommenter is a plugin/middleware/wrapper to augment application related information/tags with SQL Statements that can be used later to correlate user code with SQL statements. @@ -7,28 +7,29 @@ SQLcommenter is a plugin/middleware/wrapper to augment application related infor ### Install from source * Clone the source -* In terminal go inside the client folder location where we need to import google-sqlcommenter package and enter the below commands +* In terminal go inside the client folder location where we need to import sqlcommenter go-sql module and enter the below commands ```shell -go mod edit -replace google.com/sqlcommenter=path/to/google/sqlcommenter/go +go mod edit -replace google.com/sqlcommenter=path/to/google/sqlcommenter/go-sql go mod tiny + +go get google.com/sqlcommenter/gosql ``` ### Install from github [To be added] -## Usages +## Usage -### go-sql-driver -Please use the sqlcommenter's default database driver to execute statements. \ +Please use the sqlcommenter's default go-sql database driver to execute statements. Due to inherent nature of Go, the safer way to pass information from framework to database driver is via `context`. So, it is recommended to use the context based methods of `DB` interface like `QueryContext`, `ExecContext` and `PrepareContext`. ```go -db, err := sqlcommenter.Open("", "", sqlcommenter.CommenterOptions{:}) +db, err := gosql.Open("", "", sqlcommenter.CommenterOptions{:}) ``` -#### Configuration +### Configuration -Users are given control over what tags they want to append by using `sqlcommenter.CommenterOptions` struct. +Users are given control over what tags they want to append by using `core.CommenterOptions` struct. ```go type CommenterOptions struct { @@ -41,32 +42,15 @@ type CommenterOptions struct { } ``` -### net/http -Populate the request context with sqlcommenter.AddHttpRouterTags(r) function in a custom middleware. -#### Note -* We only support the `database/sql` driver and have provided an implementation for that. -* ORM related tags are added to the driver only when the tags are enabled in the commenter's driver's config and also the request context should passed to the querying functions +### Framework Supported +* [http/net](.../../../http-net/README.md) -#### Example -```go -// middleware is used to intercept incoming HTTP calls and populate request context with commenter tags. -func middleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := sqlcommenter.AddHttpRouterTags(r, next) - next.ServeHTTP(w, r.WithContext(ctx)) - }) -} -``` ## Options With Go SqlCommenter, we have configuration to choose which tags to be appended to the comment. -| Options | Included by default? | go-sql-orm | net/http | Notes | -| --------------- | :------------------: | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :---: | -| `DBDriver` | | [ go-sql-driver](https://pkg.go.dev/database/sql/driver) | | -| `Action` | | | [net/http handle](https://pkg.go.dev/net/http#Handle) | | -| `Route` | | | [net/http routing path](https://pkg.go.dev/github.com/gorilla/mux#Route.URLPath) | | -| `Framework` | | | [net/http](https://pkg.go.dev/net/http) | | -| `Opentelemetry` | | [W3C TraceContext.Traceparent](https://www.w3.org/TR/trace-context/#traceparent-field), [W3C TraceContext.Tracestate](https://www.w3.org/TR/trace-context/#tracestate-field) | [W3C TraceContext.Traceparent](https://www.w3.org/TR/trace-context/#traceparent-field), [W3C TraceContext.Tracestate](https://www.w3.org/TR/trace-context/#tracestate-field) | | +| Options | Included by default? | go-sql-driver | +| ---------- | -------------------- | -------------------------------------------------------- | +| `DBDriver` | | [ go-sql-driver](https://pkg.go.dev/database/sql/driver) | diff --git a/go/go-sql/go-sql.go b/go/go-sql/go-sql.go index 05ddf6ca..f4aae1b9 100644 --- a/go/go-sql/go-sql.go +++ b/go/go-sql/go-sql.go @@ -18,40 +18,17 @@ import ( "context" "database/sql" "fmt" - "net/http" - "net/url" - "reflect" - "runtime" - "sort" "strings" - "go.opentelemetry.io/otel/propagation" -) - -const ( - route string = "route" - controller string = "controller" - action string = "action" - framework string = "framework" - driver string = "driver" - traceparent string = "traceparent" + "github.com/google/sqlcommenter/go/core" ) type DB struct { *sql.DB - options CommenterOptions -} - -type CommenterOptions struct { - EnableDBDriver bool - EnableRoute bool - EnableFramework bool - EnableController bool - EnableAction bool - EnableTraceparent bool + options core.CommenterOptions } -func Open(driverName string, dataSourceName string, options CommenterOptions) (*DB, error) { +func Open(driverName string, dataSourceName string, options core.CommenterOptions) (*DB, error) { db, err := sql.Open(driverName, dataSourceName) return &DB{DB: db, options: options}, err } @@ -88,18 +65,6 @@ func (db *DB) PrepareContext(ctx context.Context, query string) (*sql.Stmt, erro // ***** Query Functions ***** -// ***** Framework Functions ***** - -func AddHttpRouterTags(r *http.Request, next any) context.Context { // any type is set because we need to refrain from importing http-router package - ctx := context.Background() - ctx = context.WithValue(ctx, route, r.URL.Path) - ctx = context.WithValue(ctx, action, getFunctionName(next)) - ctx = context.WithValue(ctx, framework, "net/http") - return ctx -} - -// ***** Framework Functions ***** - // ***** Commenter Functions ***** func (db *DB) withComment(ctx context.Context, query string) string { @@ -107,34 +72,34 @@ func (db *DB) withComment(ctx context.Context, query string) string { query = strings.TrimSpace(query) // Sorted alphabetically - if db.options.EnableAction && (ctx.Value(action) != nil) { - commentsMap[action] = ctx.Value(action).(string) + if db.options.EnableAction && (ctx.Value(core.Action) != nil) { + commentsMap[core.Action] = ctx.Value(core.Action).(string) } // `driver` information should not be coming from framework. // So, explicitly adding that here. if db.options.EnableDBDriver { - commentsMap[driver] = "database/sql" + commentsMap[core.Driver] = "database/sql" } - if db.options.EnableFramework && (ctx.Value(framework) != nil) { - commentsMap[framework] = ctx.Value(framework).(string) + if db.options.EnableFramework && (ctx.Value(core.Framework) != nil) { + commentsMap[core.Framework] = ctx.Value(core.Framework).(string) } - if db.options.EnableRoute && (ctx.Value(route) != nil) { - commentsMap[route] = ctx.Value(route).(string) + if db.options.EnableRoute && (ctx.Value(core.Route) != nil) { + commentsMap[core.Route] = ctx.Value(core.Route).(string) } if db.options.EnableTraceparent { - carrier := extractTraceparent(ctx) + carrier := core.ExtractTraceparent(ctx) if val, ok := carrier["traceparent"]; ok { - commentsMap[traceparent] = val + commentsMap[core.Traceparent] = val } } var commentsString string = "" if len(commentsMap) > 0 { // Converts comments map to string and appends it to query - commentsString = fmt.Sprintf("/*%s*/", convertMapToComment(commentsMap)) + commentsString = fmt.Sprintf("/*%s*/", core.ConvertMapToComment(commentsMap)) } // A semicolon at the end of the SQL statement means the query ends there. @@ -146,45 +111,3 @@ func (db *DB) withComment(ctx context.Context, query string) string { } // ***** Commenter Functions ***** - -// ***** Util Functions ***** - -func encodeURL(k string) string { - return url.QueryEscape(string(k)) -} - -func getFunctionName(i interface{}) string { - return runtime.FuncForPC(reflect.ValueOf(i).Pointer()).Name() -} - -func convertMapToComment(tags map[string]string) string { - var sb strings.Builder - i, sz := 0, len(tags) - - //sort by keys - sortedKeys := make([]string, 0, len(tags)) - for k := range tags { - sortedKeys = append(sortedKeys, k) - } - sort.Strings(sortedKeys) - - for _, key := range sortedKeys { - if i == sz-1 { - sb.WriteString(fmt.Sprintf("%s=%v", encodeURL(key), encodeURL(tags[key]))) - } else { - sb.WriteString(fmt.Sprintf("%s=%v,", encodeURL(key), encodeURL(tags[key]))) - } - i++ - } - return sb.String() -} - -func extractTraceparent(ctx context.Context) propagation.MapCarrier { - // Serialize the context into carrier - propgator := propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}) - carrier := propagation.MapCarrier{} - propgator.Inject(ctx, carrier) - return carrier -} - -// ***** Util Functions ***** diff --git a/go/go-sql/go-sql_test.go b/go/go-sql/go-sql_test.go index be03518a..fedbb7f6 100644 --- a/go/go-sql/go-sql_test.go +++ b/go/go-sql/go-sql_test.go @@ -1,3 +1,17 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package gosql import ( @@ -7,20 +21,18 @@ import ( "testing" "github.com/DATA-DOG/go-sqlmock" + "github.com/google/sqlcommenter/go/core" + httpnet "github.com/google/sqlcommenter/go/net/http" "go.opentelemetry.io/otel/exporters/stdout/stdouttrace" sdktrace "go.opentelemetry.io/otel/sdk/trace" ) -var engine, connectionParams = "mysql", "root:root@/gotest" - func TestDisabled(t *testing.T) { mockDB, _, err := sqlmock.New() - - db := DB{DB: mockDB, options: CommenterOptions{}} if err != nil { t.Fatalf("MockSQL failed with unexpected error: %s", err) } - + db := DB{DB: mockDB, options: core.CommenterOptions{}} query := "SELECT 2" if got, want := db.withComment(context.Background(), query), query; got != want { t.Errorf("db.withComment(context.Background(), %q) = %q, want = %q", query, got, want) @@ -29,59 +41,61 @@ func TestDisabled(t *testing.T) { func TestHTTP_Net(t *testing.T) { mockDB, _, err := sqlmock.New() - - db := DB{DB: mockDB, options: CommenterOptions{EnableDBDriver: true, EnableRoute: true, EnableFramework: true}} if err != nil { t.Fatalf("MockSQL failed with unexpected error: %s", err) } - r, _ := http.NewRequest("GET", "hello/1", nil) - ctx := AddHttpRouterTags(r, context.Background()) + db := DB{DB: mockDB, options: core.CommenterOptions{EnableDBDriver: true, EnableRoute: true, EnableFramework: true}} + r, err := http.NewRequest("GET", "hello/1", nil) + if err != nil { + t.Errorf("http.NewRequest('GET', 'hello/1', nil) returned unexpected error: %v", err) + } + ctx := core.ContextInject(r.Context(), httpnet.NewHTTPRequestExtractor(r, nil)) got := db.withComment(ctx, "Select 1") want := "Select 1/*driver=database%2Fsql,framework=net%2Fhttp,route=hello%2F1*/" - if got != want { - t.Errorf("got %q, wanted %q", got, want) + t.Errorf("db.withComment(ctx, 'Select 1') got %q, wanted %q", got, want) } } func TestQueryWithSemicolon(t *testing.T) { mockDB, _, err := sqlmock.New() - - db := DB{DB: mockDB, options: CommenterOptions{EnableDBDriver: true}} if err != nil { t.Fatalf("MockSQL failed with unexpected error: %s", err) } + + db := DB{DB: mockDB, options: core.CommenterOptions{EnableDBDriver: true}} got := db.withComment(context.Background(), "Select 1;") want := "Select 1/*driver=database%2Fsql*/;" - if got != want { - t.Errorf("got %q, wanted %q", got, want) + t.Errorf("db.withComment(context.Background(), 'Select 1;') got %q, wanted %q", got, want) } } func TestOtelIntegration(t *testing.T) { mockDB, _, err := sqlmock.New() - - db := DB{DB: mockDB, options: CommenterOptions{EnableTraceparent: true}} if err != nil { t.Fatalf("MockSQL failed with unexpected error: %s", err) } + db := DB{DB: mockDB, options: core.CommenterOptions{EnableTraceparent: true}} exp, _ := stdouttrace.New(stdouttrace.WithPrettyPrint()) bsp := sdktrace.NewSimpleSpanProcessor(exp) // You should use batch span processor in prod tp := sdktrace.NewTracerProvider( sdktrace.WithSampler(sdktrace.AlwaysSample()), sdktrace.WithSpanProcessor(bsp), ) - ctx, _ := tp.Tracer("").Start(context.Background(), "parent-span-name") got := db.withComment(ctx, "Select 1;") - r, _ := regexp.Compile("Select 1/\\*traceparent=\\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\\d{1,2}\\*/;") + wantRegex := "Select 1/\\*traceparent=\\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\\d{1,2}\\*/;" + r, err := regexp.Compile(wantRegex) + if err != nil { + t.Errorf("regex.Compile() failed with error: %v", err) + } if !r.MatchString(got) { - t.Errorf("got %q", got) + t.Errorf("%q does not match the given regex %q", got, wantRegex) } } diff --git a/go/go-sql/go.mod b/go/go-sql/go.mod index 201245cb..060407c1 100644 --- a/go/go-sql/go.mod +++ b/go/go-sql/go.mod @@ -2,19 +2,19 @@ module google.com/sqlcommenter/gosql go 1.19 -require ( - go.opentelemetry.io/otel v1.10.0 - go.opentelemetry.io/otel/sdk v1.10.0 -) +require go.opentelemetry.io/otel/sdk v1.10.0 require ( github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/google/sqlcommenter/go/core v0.0.1-beta // indirect + go.opentelemetry.io/otel v1.10.0 // indirect golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6 // indirect ) require ( github.com/DATA-DOG/go-sqlmock v1.5.0 + github.com/google/sqlcommenter/go/net/http v0.0.1-beta go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.10.0 go.opentelemetry.io/otel/trace v1.10.0 // indirect ) diff --git a/go/go-sql/go.sum b/go/go-sql/go.sum index 14fae557..8db643f7 100644 --- a/go/go-sql/go.sum +++ b/go/go-sql/go.sum @@ -7,6 +7,10 @@ github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbV github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/sqlcommenter/go/core v0.0.1-beta h1:IVszEHanWVeS7UcmP8C3SHa57CmfeqMBj0QUcJ8VZ9Q= +github.com/google/sqlcommenter/go/core v0.0.1-beta/go.mod h1:CZfcqmbIxngExnZ7Se6AsKNVubZhKyi54aeDJZiqTMQ= +github.com/google/sqlcommenter/go/net/http v0.0.1-beta h1:7XQ6poZv+ZJwwHWQHlesq9IMsRus3G6Z9n10qAkrGqE= +github.com/google/sqlcommenter/go/net/http v0.0.1-beta/go.mod h1:tVUqM1YZ/K3eRTdGzeav1GSbw+BXNdTGzSAbLW9CxAc= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= go.opentelemetry.io/otel v1.10.0 h1:Y7DTJMR6zs1xkS/upamJYk0SxxN4C9AqRd77jmZnyY4= @@ -17,8 +21,6 @@ go.opentelemetry.io/otel/sdk v1.10.0 h1:jZ6K7sVn04kk/3DNUdJ4mqRlGDiXAVuIG+MMENpT go.opentelemetry.io/otel/sdk v1.10.0/go.mod h1:vO06iKzD5baltJz1zarxMCNHFpUlUiOy4s65ECtn6kE= go.opentelemetry.io/otel/trace v1.10.0 h1:npQMbR8o7mum8uF95yFbOEJffhs1sbCOfDh8zAJiH5E= go.opentelemetry.io/otel/trace v1.10.0/go.mod h1:Sij3YYczqAdz+EhmGhE6TpTxUO5/F/AzrK+kxfGqySM= -golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7 h1:iGu644GcxtEcrInvDsQRCwJjtCIOlT2V7IRt6ah2Whw= -golang.org/x/sys v0.0.0-20210423185535-09eb48e85fd7/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6 h1:cy1ko5847T/lJ45eyg/7uLprIE/amW5IXxGtEnQdYMI= golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= From fdea29c053b5071b4d1094bc05975d9cf0f82c27 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Fri, 30 Sep 2022 19:40:05 +0530 Subject: [PATCH 2/4] module name bug fix --- go/go-sql/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/go-sql/go.mod b/go/go-sql/go.mod index 060407c1..268c0485 100644 --- a/go/go-sql/go.mod +++ b/go/go-sql/go.mod @@ -1,4 +1,4 @@ -module google.com/sqlcommenter/gosql +module github.com/google/sqlcommenter/go/go-sql go 1.19 From 0bd093dc5f607d64e941fada667744b6acbd11c0 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Fri, 30 Sep 2022 19:48:42 +0530 Subject: [PATCH 3/4] PR changes --- go/{go-sql => database/sql}/README.md | 0 go/{go-sql => database/sql}/go-sql.go | 2 +- go/{go-sql => database/sql}/go-sql_test.go | 2 +- go/{go-sql => database/sql}/go.mod | 8 +++++--- go/{go-sql => database/sql}/go.sum | 0 5 files changed, 7 insertions(+), 5 deletions(-) rename go/{go-sql => database/sql}/README.md (100%) rename go/{go-sql => database/sql}/go-sql.go (99%) rename go/{go-sql => database/sql}/go-sql_test.go (99%) rename go/{go-sql => database/sql}/go.mod (73%) rename go/{go-sql => database/sql}/go.sum (100%) diff --git a/go/go-sql/README.md b/go/database/sql/README.md similarity index 100% rename from go/go-sql/README.md rename to go/database/sql/README.md diff --git a/go/go-sql/go-sql.go b/go/database/sql/go-sql.go similarity index 99% rename from go/go-sql/go-sql.go rename to go/database/sql/go-sql.go index f4aae1b9..9742f24e 100644 --- a/go/go-sql/go-sql.go +++ b/go/database/sql/go-sql.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package gosql +package sql import ( "context" diff --git a/go/go-sql/go-sql_test.go b/go/database/sql/go-sql_test.go similarity index 99% rename from go/go-sql/go-sql_test.go rename to go/database/sql/go-sql_test.go index fedbb7f6..78c1d5bf 100644 --- a/go/go-sql/go-sql_test.go +++ b/go/database/sql/go-sql_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package gosql +package sql import ( "context" diff --git a/go/go-sql/go.mod b/go/database/sql/go.mod similarity index 73% rename from go/go-sql/go.mod rename to go/database/sql/go.mod index 268c0485..681d531f 100644 --- a/go/go-sql/go.mod +++ b/go/database/sql/go.mod @@ -1,13 +1,15 @@ -module github.com/google/sqlcommenter/go/go-sql +module github.com/google/sqlcommenter/go/database/sql go 1.19 -require go.opentelemetry.io/otel/sdk v1.10.0 +require ( + github.com/google/sqlcommenter/go/core v0.0.1-beta + go.opentelemetry.io/otel/sdk v1.10.0 +) require ( github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect - github.com/google/sqlcommenter/go/core v0.0.1-beta // indirect go.opentelemetry.io/otel v1.10.0 // indirect golang.org/x/sys v0.0.0-20220927170352-d9d178bc13c6 // indirect ) diff --git a/go/go-sql/go.sum b/go/database/sql/go.sum similarity index 100% rename from go/go-sql/go.sum rename to go/database/sql/go.sum From dbf857ddd22063b7888ae5e3bd6c3f351bad6531 Mon Sep 17 00:00:00 2001 From: Thiyagu55 Date: Fri, 30 Sep 2022 19:52:51 +0530 Subject: [PATCH 4/4] Updating workflow --- .github/workflows/go-sql-tests.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go-sql-tests.yaml b/.github/workflows/go-sql-tests.yaml index e71b3e6b..5f081e3a 100644 --- a/.github/workflows/go-sql-tests.yaml +++ b/.github/workflows/go-sql-tests.yaml @@ -23,11 +23,11 @@ jobs: - name: Build run: go build -v ./... - working-directory: ./go/go-sql + working-directory: ./go/database/sql - name: Test go-sql run: go test -v ./... - working-directory: ./go/go-sql + working-directory: ./go/database/sql gofmt: runs-on: ubuntu-latest