Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat: add prometheus metrics to database.Store#7713

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
johnstcn merged 11 commits intomainfromcj/db-prom
May 31, 2023
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
11 commits
Select commitHold shift + click to select a range
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
6 changes: 4 additions & 2 deletionscli/server.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -65,6 +65,7 @@ import (
"github.com/coder/coder/coderd/autobuild/executor"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/database/dbfake"
"github.com/coder/coder/coderd/database/dbmetrics"
"github.com/coder/coder/coderd/database/dbpurge"
"github.com/coder/coder/coderd/database/migrations"
"github.com/coder/coder/coderd/devtunnel"
Expand DownExpand Up@@ -586,7 +587,8 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
}

if cfg.InMemoryDatabase {
options.Database = dbfake.New()
// This is only used for testing.
options.Database = dbmetrics.New(dbfake.New(), options.PrometheusRegistry)
options.Pubsub = database.NewPubsubInMemory()
} else {
sqlDB, err := connectToPostgres(ctx, logger, sqlDriver, cfg.PostgresURL.String())
Expand All@@ -597,7 +599,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
_ = sqlDB.Close()
}()

options.Database = database.New(sqlDB)
options.Database =dbmetrics.New(database.New(sqlDB), options.PrometheusRegistry)
options.Pubsub, err = database.NewPubsub(ctx, sqlDB, cfg.PostgresURL.String())
if err != nil {
return xerrors.Errorf("create pubsub: %w", err)
Expand Down
10 changes: 10 additions & 0 deletionscoderd/coderd.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,7 @@ import (
"github.com/coder/coder/coderd/awsidentity"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/database/dbauthz"
"github.com/coder/coder/coderd/database/dbmetrics"
"github.com/coder/coder/coderd/database/dbtype"
"github.com/coder/coder/coderd/gitauth"
"github.com/coder/coder/coderd/gitsshkey"
Expand DownExpand Up@@ -176,6 +177,11 @@ func New(options *Options) *API {
options = &Options{}
}

// Safety check: if we're not running a unit test, we *must* have a Prometheus registry.
if options.PrometheusRegistry == nil && flag.Lookup("test.v") == nil {
panic("developer error: options.PrometheusRegistry is nil and not running a unit test")
}

Comment on lines +180 to +184
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

review: I'm open to removing this, but I think it's a good check to have. We don't want to accidentally publish a release with no prometheus metrics.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Do we always do this? Seems wasteful if--prometheus-enable=false.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yes, we do. It is a little wasteful, but less error-prone than having to check if the prometheus registerer is nil everywhere.

if options.DeploymentValues.DisableOwnerWorkspaceExec {
rbac.ReloadBuiltinRoles(&rbac.RoleOptions{
NoOwnerWorkspaceExec: true,
Expand All@@ -185,6 +191,10 @@ func New(options *Options) *API {
if options.Authorizer == nil {
options.Authorizer = rbac.NewCachingAuthorizer(options.PrometheusRegistry)
}
// The below are no-ops if already wrapped.
if options.PrometheusRegistry != nil {
options.Database = dbmetrics.New(options.Database, options.PrometheusRegistry)
}
options.Database = dbauthz.New(
options.Database,
options.Authorizer,
Expand Down
13 changes: 13 additions & 0 deletionscoderd/database/db.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,11 +24,20 @@ type Store interface {
querier
// customQuerier contains custom queries that are not generated.
customQuerier
// wrapper allows us to detect if the interface has been wrapped.
wrapper

Ping(ctx context.Context) (time.Duration, error)
InTx(func(Store) error, *sql.TxOptions) error
}

type wrapper interface {
// Wrappers returns a list of wrappers that have been applied to the store.
// This is used to detect if the store has already wrapped, and avoid
// double-wrapping.
Wrappers() []string
}

// DBTX represents a database connection or transaction.
type DBTX interface {
ExecContext(context.Context, string, ...interface{}) (sql.Result, error)
Expand DownExpand Up@@ -60,6 +69,10 @@ type sqlQuerier struct {
db DBTX
}

func (*sqlQuerier) Wrappers() []string {
return []string{}
}

// Ping returns the time it takes to ping the database.
func (q *sqlQuerier) Ping(ctx context.Context) (time.Duration, error) {
start := time.Now()
Expand Down
9 changes: 8 additions & 1 deletioncoderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,7 @@ import (
"fmt"

"github.com/google/uuid"
"golang.org/x/exp/slices"
"golang.org/x/xerrors"

"github.com/open-policy-agent/opa/topdown"
Expand All@@ -17,6 +18,8 @@ import (

var _ database.Store = (*querier)(nil)

const wrapname = "dbauthz.querier"

// NoActorError wraps ErrNoRows for the api to return a 404. This is the correct
// response when the user is not authorized.
var NoActorError = xerrors.Errorf("no authorization actor in context: %w", sql.ErrNoRows)
Expand DownExpand Up@@ -89,7 +92,7 @@ type querier struct {
func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger) database.Store {
// If the underlying db store is already a querier, return it.
// Do not double wrap.
if_, ok :=db.(*querier); ok {
ifslices.Contains(db.Wrappers(), wrapname) {
return db
}
return &querier{
Expand All@@ -99,6 +102,10 @@ func New(db database.Store, authorizer rbac.Authorizer, logger slog.Logger) data
}
}

func (q *querier) Wrappers() []string {
return append(q.db.Wrappers(), wrapname)
}

// authorizeContext is a helper function to authorize an action on an object.
func (q *querier) authorizeContext(ctx context.Context, action rbac.Action, object rbac.Objecter) error {
act, ok := ActorFromContext(ctx)
Expand Down
2 changes: 1 addition & 1 deletioncoderd/database/dbauthz/dbauthz_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ func TestDBAuthzRecursive(t *testing.T) {
for i := 2; i < method.Type.NumIn(); i++ {
ins = append(ins, reflect.New(method.Type.In(i)).Elem())
}
if method.Name == "InTx" || method.Name == "Ping" {
if method.Name == "InTx" || method.Name == "Ping"|| method.Name == "Wrappers"{
continue
}
// Log the name of the last method, so if there is a panic, it is
Expand Down
13 changes: 10 additions & 3 deletionscoderd/database/dbauthz/setup_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@ import (
"strings"
"testing"

"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/open-policy-agent/opa/topdown"
"github.com/stretchr/testify/require"
Expand All@@ -19,14 +20,16 @@ import (
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/database/dbauthz"
"github.com/coder/coder/coderd/database/dbfake"
"github.com/coder/coder/coderd/database/dbmock"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/rbac/regosql"
"github.com/coder/coder/coderd/util/slice"
)

var skipMethods = map[string]string{
"InTx": "Not relevant",
"Ping": "Not relevant",
"InTx": "Not relevant",
"Ping": "Not relevant",
"Wrappers": "Not relevant",
}

// TestMethodTestSuite runs MethodTestSuite.
Expand All@@ -52,7 +55,11 @@ type MethodTestSuite struct {
// SetupSuite sets up the suite by creating a map of all methods on querier
// and setting their count to 0.
func (s *MethodTestSuite) SetupSuite() {
az := dbauthz.New(nil, nil, slog.Make())
ctrl := gomock.NewController(s.T())
mockStore := dbmock.NewMockStore(ctrl)
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@Emyrk I think it makes sense to use the mock here instead ofnil, as it will allow us to immediately see if we accidentally call methods of the underlyingdatabase.Store.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It would just panic if the db is set to nil, which would tell us a call was made. Does the mock panic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ah I see though, for the wrappers method.

johnstcn reacted with thumbs up emoji
// We intentionally set no expectations apart from this.
mockStore.EXPECT().Wrappers().Return([]string{}).AnyTimes()
az := dbauthz.New(mockStore, nil, slog.Make())
// Take the underlying type of the interface.
azt := reflect.TypeOf(az).Elem()
s.methodAccounting = make(map[string]int)
Expand Down
4 changes: 4 additions & 0 deletionscoderd/database/dbfake/databasefake.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -97,6 +97,10 @@ type fakeQuerier struct {
*data
}

func (*fakeQuerier) Wrappers() []string {
return []string{}
}

type fakeTx struct {
*fakeQuerier
locks map[int64]struct{}
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp