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

revert: "feat: register db.sql metrics (#2305)" #2365

Merged
merged 2 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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