Skip to content

Commit

Permalink
revert: "feat: register db.sql metrics (#2305)" (#2365)
Browse files Browse the repository at this point in the history
This reverts commit 1028e65.
  • Loading branch information
safeer authored Aug 14, 2024
1 parent 71d6cf8 commit acb7f5d
Show file tree
Hide file tree
Showing 14 changed files with 14 additions and 108 deletions.
14 changes: 2 additions & 12 deletions backend/controller/sql/databasetesting/devel.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import (
"strings"
"time"

"github.com/XSAM/otelsql"
_ "github.com/jackc/pgx/v5/stdlib" // pgx driver
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"

"github.com/TBD54566975/ftl/backend/controller/sql"
"github.com/TBD54566975/ftl/internal/log"
Expand All @@ -31,13 +29,9 @@ func CreateForDevel(ctx context.Context, dsn string, recreate bool) (*stdsql.DB,

var conn *stdsql.DB
for range 10 {
conn, err = otelsql.Open("pgx", noDBDSN.String())
conn, err = stdsql.Open("pgx", noDBDSN.String())
if err == nil {
defer conn.Close()
err = otelsql.RegisterDBStatsMetrics(conn, otelsql.WithAttributes(semconv.DBSystemPostgreSQL))
if err != nil {
panic(err)
}
break
}
logger.Debugf("Waiting for database to be ready: %v", err)
Expand Down Expand Up @@ -78,14 +72,10 @@ func CreateForDevel(ctx context.Context, dsn string, recreate bool) (*stdsql.DB,
return nil, err
}

realConn, err := otelsql.Open("pgx", dsn)
realConn, err := stdsql.Open("pgx", dsn)
if err != nil {
return nil, fmt.Errorf("failed to open database: %w", err)
}
err = otelsql.RegisterDBStatsMetrics(realConn, otelsql.WithAttributes(semconv.DBSystemPostgreSQL))
if err != nil {
return nil, fmt.Errorf("failed to register db metrics: %w", err)
}
// Reset transient state in the database to a clean state for development purposes.
// This includes things like resetting the state of async calls, leases,
// controller/runner registration, etc. but not anything more.
Expand Down
7 changes: 2 additions & 5 deletions cmd/ftl-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@ package main

import (
"context"
"database/sql"
"fmt"
"os"
"strconv"
"time"

"github.com/XSAM/otelsql"
"github.com/alecthomas/kong"
"github.com/alecthomas/types/optional"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/secretsmanager"
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"

"github.com/TBD54566975/ftl"
"github.com/TBD54566975/ftl/backend/controller"
Expand Down Expand Up @@ -54,9 +53,7 @@ func main() {
kctx.FatalIfErrorf(err, "failed to initialize observability")

// The FTL controller currently only supports DB as a configuration provider/resolver.
conn, err := otelsql.Open("pgx", cli.ControllerConfig.DSN)
kctx.FatalIfErrorf(err)
err = otelsql.RegisterDBStatsMetrics(conn, otelsql.WithAttributes(semconv.DBSystemPostgreSQL))
conn, err := sql.Open("pgx", cli.ControllerConfig.DSN)
kctx.FatalIfErrorf(err)
dal, err := dal.New(ctx, conn, optional.Some[string](*cli.ControllerConfig.KMSURI))
kctx.FatalIfErrorf(err)
Expand Down
9 changes: 2 additions & 7 deletions cmd/ftl/cmd_box_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ package main

import (
"context"
"database/sql"
"fmt"
"net/url"
"time"

"github.com/XSAM/otelsql"
_ "github.com/jackc/pgx/v5/stdlib" // pgx driver
"github.com/jpillora/backoff"
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
"golang.org/x/sync/errgroup"

"github.com/TBD54566975/ftl/backend/controller"
Expand Down Expand Up @@ -59,14 +58,10 @@ func (b *boxRunCmd) Run(ctx context.Context, projConfig projectconfig.Config) er
}

// Bring up the DB connection and DAL.
conn, err := otelsql.Open("pgx", config.DSN)
conn, err := sql.Open("pgx", config.DSN)
if err != nil {
return fmt.Errorf("failed to bring up DB connection: %w", err)
}
err = otelsql.RegisterDBStatsMetrics(conn, otelsql.WithAttributes(semconv.DBSystemPostgreSQL))
if err != nil {
return fmt.Errorf("failed to register DB metrics: %w", err)
}

wg := errgroup.Group{}
wg.Go(func() error {
Expand Down
11 changes: 3 additions & 8 deletions cmd/ftl/cmd_serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"database/sql"
"errors"
"fmt"
"net"
Expand All @@ -14,10 +15,8 @@ import (
"time"

"connectrpc.com/connect"
"github.com/XSAM/otelsql"
"github.com/alecthomas/types/optional"
_ "github.com/jackc/pgx/v5/stdlib" // pgx driver
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
"golang.org/x/sync/errgroup"

"github.com/TBD54566975/ftl"
Expand Down Expand Up @@ -148,15 +147,11 @@ func (s *serveCmd) run(ctx context.Context, projConfig projectconfig.Config, ini
}
controllerCtx = cf.ContextWithSecrets(controllerCtx, sm)

// Bring up the DB connection for the controller.
conn, err := otelsql.Open("pgx", config.DSN)
// Bring up the DB connection and DAL.
conn, err := sql.Open("pgx", config.DSN)
if err != nil {
return fmt.Errorf("failed to bring up DB connection: %w", err)
}
err = otelsql.RegisterDBStatsMetrics(conn, otelsql.WithAttributes(semconv.DBSystemPostgreSQL))
if err != nil {
return fmt.Errorf("failed to register DB metrics: %w", err)
}

wg.Go(func() error {
if err := controller.Start(controllerCtx, config, runnerScaling, conn); err != nil {
Expand Down
1 change: 0 additions & 1 deletion examples/go/echo/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ require (
connectrpc.com/connect v1.16.2 // indirect
connectrpc.com/grpcreflect v1.2.0 // indirect
connectrpc.com/otelconnect v0.7.1 // indirect
github.com/XSAM/otelsql v0.32.0 // indirect
github.com/alecthomas/atomic v0.1.0-alpha2 // indirect
github.com/alecthomas/concurrency v0.0.2 // indirect
github.com/alecthomas/participle/v2 v2.1.1 // indirect
Expand Down
4 changes: 0 additions & 4 deletions examples/go/echo/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
connectrpc.com/connect v1.16.2 // indirect
connectrpc.com/grpcreflect v1.2.0 // indirect
connectrpc.com/otelconnect v0.7.1 // indirect
github.com/XSAM/otelsql v0.32.0 // indirect
github.com/alecthomas/atomic v0.1.0-alpha2 // indirect
github.com/alecthomas/concurrency v0.0.2 // indirect
github.com/alecthomas/participle/v2 v2.1.1 // indirect
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/BurntSushi/toml v1.4.0
github.com/TBD54566975/golang-tools v0.2.1
github.com/TBD54566975/scaffolder v1.0.0
github.com/XSAM/otelsql v0.32.0
github.com/alecthomas/assert/v2 v2.10.0
github.com/alecthomas/atomic v0.1.0-alpha2
github.com/alecthomas/chroma/v2 v2.14.0
Expand Down Expand Up @@ -69,7 +68,6 @@ require (
golang.org/x/sync v0.8.0
golang.org/x/term v0.23.0
google.golang.org/protobuf v1.34.2
gotest.tools/v3 v3.5.1
modernc.org/sqlite v1.32.0
)

Expand All @@ -89,7 +87,6 @@ require (
github.com/docker/go-units v0.5.0 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/gorilla/websocket v1.5.1 // indirect
github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
github.com/iancoleman/strcase v0.3.0 // indirect
Expand All @@ -113,6 +110,7 @@ require (
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.26.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
golang.org/x/tools v0.23.0 // indirect
gotest.tools/v3 v3.5.1 // indirect
modernc.org/gc/v3 v3.0.0-20240107210532-573471604cb6 // indirect
)

Expand Down
2 changes: 0 additions & 2 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 2 additions & 9 deletions internal/modulecontext/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ import (
"fmt"
"strconv"

"github.com/XSAM/otelsql"
semconv "go.opentelemetry.io/otel/semconv/v1.4.0"

ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
)

Expand All @@ -22,13 +19,9 @@ type Database struct {

// NewDatabase creates a Database that can be added to ModuleContext
func NewDatabase(dbType DBType, dsn string) (Database, error) {
db, err := otelsql.Open("pgx", dsn)
if err != nil {
return Database{}, fmt.Errorf("failed to open database: %w", err)
}
err = otelsql.RegisterDBStatsMetrics(db, otelsql.WithAttributes(semconv.DBSystemPostgreSQL))
db, err := sql.Open("pgx", dsn)
if err != nil {
return Database{}, fmt.Errorf("failed to register db metrics: %w", err)
return Database{}, fmt.Errorf("failed to bring up DB connection: %w", err)
}
return Database{
DSN: dsn,
Expand Down
2 changes: 1 addition & 1 deletion internal/observability/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ import (

func TestSchemaMismatch(t *testing.T) {
dflt := resource.Default()
assert.Equal(t, dflt.SchemaURL(), schemaURL, `in every file that imports go.opentelemetry.io/otel/semconv, change the import to: semconv "go.opentelemetry.io/otel/semconv/v%s"`, path.Base(dflt.SchemaURL()))
assert.Equal(t, dflt.SchemaURL(), schemaURL, `change import in client.go to: semconv "go.opentelemetry.io/otel/semconv/v%s"`, path.Base(dflt.SchemaURL()))
}
31 changes: 1 addition & 30 deletions internal/reflect/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
package reflect

import (
"container/list"
"fmt"
"reflect"
"strings"
Expand Down Expand Up @@ -132,11 +131,6 @@ func copyAny(src any, ptrs map[uintptr]any, copyConf *copyConfig) (dst any) {
return src
}

// Special case list.List to handle its internal structure
if reflect.TypeOf(src) == reflect.TypeFor[*list.List]() {
return copyList(src.(*list.List), ptrs, copyConf)
}

// Look up the corresponding copy function.
switch v.Kind() {
case reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32,
Expand All @@ -145,11 +139,7 @@ func copyAny(src any, ptrs map[uintptr]any, copyConf *copyConfig) (dst any) {
reflect.Complex64, reflect.Complex128, reflect.Func:
dst = copyPremitive(src, ptrs, copyConf)
case reflect.String:
if v.Type() == reflect.TypeFor[string]() {
dst = strings.Clone(src.(string))
} else {
dst = copyStringAlias(src, ptrs, copyConf)
}
dst = strings.Clone(src.(string))
case reflect.Slice:
dst = copySlice(src, ptrs, copyConf)
case reflect.Array:
Expand All @@ -170,18 +160,6 @@ func copyAny(src any, ptrs map[uintptr]any, copyConf *copyConfig) (dst any) {
return
}

func copyList(src *list.List, ptrs map[uintptr]any, copyConf *copyConfig) *list.List {
if src == nil {
return nil
}
dst := list.New()
for e := src.Front(); e != nil; e = e.Next() {
copiedValue := copyAny(e.Value, ptrs, copyConf)
dst.PushBack(copiedValue)
}
return dst
}

func copyPremitive(src any, ptr map[uintptr]any, copyConf *copyConfig) (dst any) {
kind := reflect.ValueOf(src).Kind()
switch kind {
Expand All @@ -192,13 +170,6 @@ func copyPremitive(src any, ptr map[uintptr]any, copyConf *copyConfig) (dst any)
return
}

func copyStringAlias(src any, ptr map[uintptr]any, copyConf *copyConfig) any {
v := reflect.ValueOf(src)
dc := reflect.New(v.Type()).Elem()
dc.Set(v)
return dc.Interface()
}

func copySlice(x any, ptrs map[uintptr]any, copyConf *copyConfig) any {
v := reflect.ValueOf(x)
kind := v.Kind()
Expand Down
21 changes: 0 additions & 21 deletions internal/reflect/reflect_test.go
Original file line number Diff line number Diff line change
@@ -1,30 +1,9 @@
package reflect

import (
"container/list"
"testing"

"gotest.tools/v3/assert"
)

type mystring string
type structWithMystring struct {
str mystring
}

func TestAliasedString(t *testing.T) {
output := DeepCopy(structWithMystring{"asdf"})
assert.Equal(t, output, structWithMystring{"asdf"})
}

func TestListElements(t *testing.T) {
l := list.New()
l.PushBack("one")
output := DeepCopy(l)
assert.Equal(t, output.Front().Value, l.Front().Value)
assert.Equal(t, output.Len(), l.Len())
}

type structOfPointers struct {
intPtr *int
floatPtr *float64
Expand Down

0 comments on commit acb7f5d

Please sign in to comment.