From e0fff41838308b9db735a061ad462a72e494a6f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Wed, 21 Aug 2024 13:54:00 +0200 Subject: [PATCH 1/2] PMM-12451 MongoDB explain. (#3152). * PMM-12451 Todo. * PMM-12451 Changes. * PMM-12451 Version check, refactor. * PMM-12451 Remove Mongo < 4.0.0 compatibility and logic. * PMM-12451 Remove unused annotations. * PMM-12451 Make format. * PMM-12451 Lint. * PMM-12451 Lint. * PMM-12451 Downgrade driver. * PMM-12451 Changes. * PMM-12451 Old tests. * PMM-12451 Test. * Revert "PMM-12451 Old tests." This reverts commit feb3721bbac056d0f0775db9cedacc46a2279a94. * Revert "PMM-12451 Test." This reverts commit acf12a7e18d24f7ac01ec53d74013ea6ef619976. * PMM-12451 Changes. * PMM-12451 Lint. * PMM-12451 Fix wrong version number in comment. * PMM-12451 Remove lsid. * PMM-12451 Test with 1.16.1 driver. * PMM-12451 Unmarshal canonical to false. * Reapply "PMM-12451 Old tests." This reverts commit a5754c877f618dd97f4e8621c1c15914e3079ca1. * Revert "PMM-12451 Test." This reverts commit acf12a7e18d24f7ac01ec53d74013ea6ef619976. * PMM-12451 Return back removeDBField after canonical false. * PMM-12451 Remove rest of lsids in tests. * PMM-12451 Fix comment. * PMM-12451 Handle not supported commands. * PMM-12451 Refactor. * PMM-12451 Fix logic for OriginatingCommand. * PMM-12451 Small test refactor. * Update agent/runner/actions/mongodb_explain_action.go Co-authored-by: Nurlan Moldomurov * PMM-12451 Required changes. * PMM-12451 Adds release notes. * PMM-12451 Add comments to unclear places. * PMM-12451 Changes in comments. * PMM-12451 Add PMM-13017 and PMM-13071 to RN. * Update docs/release-notes/2.43.0.md Co-authored-by: Catalina A <94133018+catalinaadam@users.noreply.github.com> * Update docs/release-notes/2.43.0.md Co-authored-by: Catalina A <94133018+catalinaadam@users.noreply.github.com> * Update agent/runner/actions/mongodb_explain_action.go Co-authored-by: Alex Demidoff * PMM-12451 Improvement to handle dbstats command. * PMM-12451 Fix RN after applied suggestion. --------- Co-authored-by: Nurlan Moldomurov Co-authored-by: Catalina A <94133018+catalinaadam@users.noreply.github.com> Co-authored-by: Alex Demidoff --- .../runner/actions/mongodb_explain_action.go | 170 +++++++++++++- .../actions/mongodb_explain_action_test.go | 218 ++++++++++++++++++ docs/release-notes/2.43.0.md | 3 + go.mod | 2 +- go.sum | 4 +- 5 files changed, 382 insertions(+), 15 deletions(-) diff --git a/agent/runner/actions/mongodb_explain_action.go b/agent/runner/actions/mongodb_explain_action.go index 925a530d0e..0d51a01763 100644 --- a/agent/runner/actions/mongodb_explain_action.go +++ b/agent/runner/actions/mongodb_explain_action.go @@ -18,9 +18,9 @@ import ( "context" "fmt" "path/filepath" + "strings" "time" - "github.com/percona/percona-toolkit/src/go/mongolib/proto" "github.com/pkg/errors" "go.mongodb.org/mongo-driver/bson" "go.mongodb.org/mongo-driver/mongo" @@ -39,6 +39,15 @@ type mongodbExplainAction struct { dsn string } +type explain struct { + Ns string `json:"ns"` + Op string `json:"op"` + Query bson.D `json:"query,omitempty"` + Command bson.D `json:"command,omitempty"` + OriginatingCommand bson.D `json:"originatingCommand,omitempty"` + UpdateObj bson.D `json:"updateobj,omitempty"` +} + var errCannotExplain = fmt.Errorf("cannot explain this type of query") // NewMongoDBExplainAction creates a MongoDB EXPLAIN query Action. @@ -89,28 +98,165 @@ func (a *mongodbExplainAction) Run(ctx context.Context) ([]byte, error) { } defer client.Disconnect(ctx) //nolint:errcheck - var eq proto.ExampleQuery + return explainForQuery(ctx, client, a.params.Query) +} + +func (a *mongodbExplainAction) sealed() {} + +func (e explain) prepareCommand() (bson.D, error) { + command := e.Command + + switch e.Op { + case "query": + if len(command) == 0 { + command = e.Query + } + + if len(command) == 0 || command[0].Key != "find" { + return bson.D{ + {Key: "find", Value: e.getCollection()}, + {Key: "filter", Value: command}, + }, nil + } + + if len(command) != 0 && command[0].Key == "query" { + return bson.D{ + {Key: "find", Value: e.getCollection()}, + {Key: "filter", Value: command[0].Value}, + }, nil + } + + return dropDBField(command), nil + case "update": + if len(command) == 0 { + command = bson.D{ + {Key: "q", Value: e.Query}, + {Key: "u", Value: e.UpdateObj}, + } + } + + return bson.D{ + {Key: "update", Value: e.getCollection()}, + {Key: "updates", Value: []any{command}}, + }, nil + case "remove": + if len(command) == 0 { + command = bson.D{{Key: "q", Value: e.Query}} + } + + return bson.D{ + {Key: "delete", Value: e.getCollection()}, + {Key: "deletes", Value: []any{command}}, + }, nil + case "getmore": + if len(e.OriginatingCommand) == 0 { + return bson.D{{Key: "getmore", Value: ""}}, nil + } + + command = e.OriginatingCommand + + return dropDBField(command), nil + case "command": + command = dropDBField(command) + + if len(command) == 0 { + return command, nil + } + + switch command[0].Key { + // Not supported commands. + case "dbStats": + return nil, errors.Errorf("command %s is not supported for explain", command[0].Key) + case "group": + default: + return command, nil + } + + return fixReduceField(command), nil + // Not supported operations. + case "insert", "drop": + return nil, errors.Errorf("operation %s is not supported for explain", e.Op) + } + + return command, nil +} + +func (e explain) getDB() string { + s := strings.SplitN(e.Ns, ".", 2) + if len(s) == 2 { + return s[0] + } + + return "" +} + +func (e explain) getCollection() string { + s := strings.SplitN(e.Ns, ".", 2) + if len(s) == 2 { + return s[1] + } + + return "" +} + +// dropDBField remove DB field to be able run explain on all supported types. +// Otherwise it could end up with BSON field 'xxx.$db' is a duplicate field. +func dropDBField(command bson.D) bson.D { + for i := range command { + if command[i].Key != "$db" { + continue + } + + if len(command)-1 == i { + return command[:i] + } + + return append(command[:i], command[i+1:]...) + } + + return command +} + +// fixReduceField fixing nil/empty values after unmarshalling funcs. +func fixReduceField(command bson.D) bson.D { + var group bson.D + var ok bool + if group, ok = command[0].Value.(bson.D); !ok { + return command + } - err = bson.UnmarshalExtJSON([]byte(a.params.Query), true, &eq) + for i := range group { + if group[i].Key == "$reduce" { + group[i].Value = "{}" + command[0].Value = group + break + } + } + + return command +} + +func explainForQuery(ctx context.Context, client *mongo.Client, query string) ([]byte, error) { + var e explain + err := bson.UnmarshalExtJSON([]byte(query), false, &e) if err != nil { - return nil, errors.Wrapf(err, "Query: %s", a.params.Query) + return nil, errors.Wrapf(err, "Query: %s", query) } - database := "admin" - if eq.Db() != "" { - database = eq.Db() + preparedCommand, err := e.prepareCommand() + if err != nil { + return nil, errors.Wrap(errCannotExplain, err.Error()) } - res := client.Database(database).RunCommand(ctx, eq.ExplainCmd()) + command := bson.D{{Key: "explain", Value: preparedCommand}} + res := client.Database(e.getDB()).RunCommand(ctx, command) if res.Err() != nil { return nil, errors.Wrap(errCannotExplain, res.Err().Error()) } - result, err := res.DecodeBytes() + result, err := res.Raw() if err != nil { return nil, err } - // We need it because result + return []byte(result.String()), nil } - -func (a *mongodbExplainAction) sealed() {} diff --git a/agent/runner/actions/mongodb_explain_action_test.go b/agent/runner/actions/mongodb_explain_action_test.go index 1f2e46c2ed..3d77b69c44 100644 --- a/agent/runner/actions/mongodb_explain_action_test.go +++ b/agent/runner/actions/mongodb_explain_action_test.go @@ -16,10 +16,13 @@ package actions import ( "context" + "crypto/rand" "encoding/json" "fmt" + "math/big" "os" "path/filepath" + "strconv" "testing" "github.com/stretchr/testify/assert" @@ -33,6 +36,221 @@ import ( "github.com/percona/pmm/version" ) +func TestQueryExplain(t *testing.T) { + database := "testdb" + ctx := context.TODO() + + dsn := tests.GetTestMongoDBDSN(t) + client := tests.OpenTestMongoDB(t, dsn) + t.Cleanup(func() { + defer client.Disconnect(ctx) //nolint:errcheck + defer client.Database(database).Drop(ctx) //nolint:errcheck + }) + + t.Run("Find", func(t *testing.T) { + query := `{ + "ns": "config.collections", + "op": "query", + "command": { + "find": "collections", + "filter": { + "_id": { + "$regex": "^admin.", + "$options": "i" + } + }, + "$db": "config" + } + }` + runExplain(ctx, t, prepareParams(t, query)) + }) + + t.Run("Count", func(t *testing.T) { + query := `{ + "ns": "testdb.collection", + "op": "command", + "command": { + "count": "collection", + "query": { + "a": { + "$numberDouble": "5.0" + }, + "b": { + "$numberDouble": "5.0" + } + }, + "$db": "testdb" + } + }` + runExplain(ctx, t, prepareParams(t, query)) + }) + + t.Run("Count with aggregate", func(t *testing.T) { + query := `{ + "ns": "testdb.collection", + "op": "command", + "command": { + "aggregate": "collection", + "pipeline": [ + { + "$group": { + "_id": null, + "count": { + "$sum": { + "$numberDouble": "1.0" + } + } + } + }, + { + "$project": { + "_id": { + "$numberDouble": "0.0" + } + } + } + ], + "cursor": {}, + "$db": "testdb" + } + }` + runExplain(ctx, t, prepareParams(t, query)) + }) + + t.Run("Update", func(t *testing.T) { + query := `{ + "ns": "testdb.inventory", + "op": "update", + "command": { + "q": { + "item": "paper" + }, + "u": { + "$set": { + "size.uom": "cm", + "status": "P" + }, + "$currentDate": { + "lastModified": true + } + }, + "multi": false, + "upsert": false + } + }` + runExplain(ctx, t, prepareParams(t, query)) + }) + + t.Run("Remove", func(t *testing.T) { + query := `{ + "ns": "testdb.inventory", + "op": "remove", + "command": { + "q": { + "_id": { + "id": { + "$binary": { + "base64": "vN9ImShsRBaCIFJ23YkysA==", + "subType": "04" + } + }, + "uid": { + "$binary": { + "base64": "Y5mrDaxi8gv8RmdTsQ+1j7fmkr7JUsabhNmXAheU0fg=", + "subType": "00" + } + } + } + }, + "limit": { + "$numberInt": "0" + } + } + }` + runExplain(ctx, t, prepareParams(t, query)) + }) + + t.Run("Distinct", func(t *testing.T) { + query := `{ + "ns": "testdb.inventory", + "op": "command", + "command": { + "distinct": "inventory", + "key": "dept", + "query": {} + }, + "$db": "testdb" + } + }` + runExplain(ctx, t, prepareParams(t, query)) + }) + + t.Run("Insert - not supported", func(t *testing.T) { + query := `{ + "ns": "testdb.listingsAndReviews", + "op": "insert", + "command": { + "insert": "listingsAndReviews", + "ordered": true, + "$db": "testdb" + } + }` + runExplainExpectError(ctx, t, prepareParams(t, query)) + }) + + t.Run("Drop - not supported", func(t *testing.T) { + query := `{ + "ns": "testdb.listingsAndReviews", + "op": "command", + "command": { + "drop": "listingsAndReviews", + "$db": "testdb" + } + }` + runExplainExpectError(ctx, t, prepareParams(t, query)) + }) + + t.Run("PMM-12451", func(t *testing.T) { + // Query from customer to prevent wrong date/time, timestamp parsing in future and prevent regression. + query := `{"ns":"testdb.testDoc","op":"query","command":{"find":"testDoc","filter":{"$and":[{"c23":{"$ne":""}},{"c23":{"$ne":null},"delete":{"$ne":true}},{"$and":[{"c23":"985662747"},{"c15":{"$gte":{"$date":"2023-09-19T22:00:00.000Z"}}},{"c15":{"$lte":{"$date":"2023-10-20T21:59:59.000Z"}}},{"c8":{"$in":["X1118630710X","X1118630720X","X1118630730X","X1118630740X","X1118630750X","X1118630760X"]},"c22":{"$in":["X1118630710X","X1118630710XA","X1118630710XB","X1118630710XC","X1118630710XD","X1118630710XE"]},"c34":{"$in":["X1118630710X","X1118630710Y","X1118630710Z","X1118630710U","X1118630710V","X1118630710W"]}},{"c2":"xxxxxxx"},{"c29":{"$in":["X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X","X1118630710X"]}}]}]},"lsid":{"id":{"$binary":{"base64":"n/f5RI2jTTCoyt0y+8D9Cw==","subType":"04"}}},"$db":"testdb"}}` + runExplain(ctx, t, prepareParams(t, query)) + }) +} + +func prepareParams(t *testing.T, query string) *agentpb.StartActionRequest_MongoDBExplainParams { + t.Helper() + + return &agentpb.StartActionRequest_MongoDBExplainParams{ + Dsn: tests.GetTestMongoDBDSN(t), + Query: query, + } +} + +func runExplain(ctx context.Context, t *testing.T, params *agentpb.StartActionRequest_MongoDBExplainParams) { + t.Helper() + + big, err := rand.Int(rand.Reader, big.NewInt(27)) + require.NoError(t, err) + id := strconv.FormatUint(big.Uint64(), 10) + ex, err := NewMongoDBExplainAction(id, 0, params, os.TempDir()) + require.NoError(t, err) + res, err := ex.Run(ctx) + require.NoError(t, err) + require.NotEmpty(t, string(res)) +} + +func runExplainExpectError(ctx context.Context, t *testing.T, params *agentpb.StartActionRequest_MongoDBExplainParams) { + t.Helper() + + big, err := rand.Int(rand.Reader, big.NewInt(27)) + require.NoError(t, err) + id := strconv.FormatUint(big.Uint64(), 10) + ex, err := NewMongoDBExplainAction(id, 0, params, os.TempDir()) + require.NoError(t, err) + _, err = ex.Run(ctx) + require.Error(t, err) +} + func TestMongoDBExplain(t *testing.T) { database := "test" collection := "test_col" diff --git a/docs/release-notes/2.43.0.md b/docs/release-notes/2.43.0.md index a8cd62d187..56e0e25aa1 100644 --- a/docs/release-notes/2.43.0.md +++ b/docs/release-notes/2.43.0.md @@ -54,3 +54,6 @@ If you're looking to upgrade, you can easily [install the latest version of Perc While these vulnerabilities did not directly impact PMM's core functionality, fixing them enhanced the overall security of the PMM Server environment. +- [PMM-12451](https://perconadev.atlassian.net/browse/PMM-12451) and [PMM-13017](https://perconadev.atlassian.net/browse/PMM-13017) - Resolved an issue with parsing JSON objects into the correct data types and aligned the explain functionality with the official MongoDB client. These updates enhance consistency with MongoDB's native tools and provide improved performance insights. + +- [PMM-13071](https://perconadev.atlassian.net/browse/PMM-13071) - The **Explain** tab on the Query Analytics (QAN) page now properly handles unsupported MongoDB query types. For operations like `INSERT`, which don’t support explain functionality, you will now see a clear message saying that the operation is not explainable. This replaces the previous, confusing error message about duplicate BSON fields, offering more accurate feedback in QAN. diff --git a/go.mod b/go.mod index 0a65f7f4c7..0da5be4280 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/stretchr/objx v0.5.2 github.com/stretchr/testify v1.9.0 - go.mongodb.org/mongo-driver v1.16.0 + go.mongodb.org/mongo-driver v1.16.1 go.starlark.net v0.0.0-20230717150657-8a3343210976 golang.org/x/crypto v0.26.0 golang.org/x/sync v0.8.0 diff --git a/go.sum b/go.sum index feefa68523..8992c9df01 100644 --- a/go.sum +++ b/go.sum @@ -526,8 +526,8 @@ github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= go.mongodb.org/mongo-driver v1.11.4/go.mod h1:PTSz5yu21bkT/wXpkS7WR5f0ddqw5quethTUn9WM+2g= -go.mongodb.org/mongo-driver v1.16.0 h1:tpRsfBJMROVHKpdGyc1BBEzzjDUWjItxbVSZ8Ls4BQ4= -go.mongodb.org/mongo-driver v1.16.0/go.mod h1:oB6AhJQvFQL4LEHyXi6aJzQJtBiTQHiAd83l0GdFaiw= +go.mongodb.org/mongo-driver v1.16.1 h1:rIVLL3q0IHM39dvE+z2ulZLp9ENZKThVfuvN/IiN4l8= +go.mongodb.org/mongo-driver v1.16.1/go.mod h1:oB6AhJQvFQL4LEHyXi6aJzQJtBiTQHiAd83l0GdFaiw= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.45.0 h1:x8Z78aZx8cOF0+Kkazoc7lwUNMGy0LrzEMxTm4BbTxg= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.45.0/go.mod h1:62CPTSry9QZtOaSsE3tOzhx6LzDhHnXJ6xHeMNNiM6Q= go.opentelemetry.io/otel v1.24.0 h1:0LAOdjNmQeSTzGBzduGe/rU4tZhMwL5rWgtp9Ku5Jfo= From 954f70b871fb46db2821bfb7517ea09af0dd00e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20=C4=8Ctvrtka?= Date: Wed, 21 Aug 2024 14:04:26 +0200 Subject: [PATCH 2/2] PMM-12451 V3 fix. --- agent/runner/actions/mongodb_explain_action_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/runner/actions/mongodb_explain_action_test.go b/agent/runner/actions/mongodb_explain_action_test.go index 3d77b69c44..f8218d9a8d 100644 --- a/agent/runner/actions/mongodb_explain_action_test.go +++ b/agent/runner/actions/mongodb_explain_action_test.go @@ -217,16 +217,16 @@ func TestQueryExplain(t *testing.T) { }) } -func prepareParams(t *testing.T, query string) *agentpb.StartActionRequest_MongoDBExplainParams { +func prepareParams(t *testing.T, query string) *agentv1.StartActionRequest_MongoDBExplainParams { t.Helper() - return &agentpb.StartActionRequest_MongoDBExplainParams{ + return &agentv1.StartActionRequest_MongoDBExplainParams{ Dsn: tests.GetTestMongoDBDSN(t), Query: query, } } -func runExplain(ctx context.Context, t *testing.T, params *agentpb.StartActionRequest_MongoDBExplainParams) { +func runExplain(ctx context.Context, t *testing.T, params *agentv1.StartActionRequest_MongoDBExplainParams) { t.Helper() big, err := rand.Int(rand.Reader, big.NewInt(27)) @@ -239,7 +239,7 @@ func runExplain(ctx context.Context, t *testing.T, params *agentpb.StartActionRe require.NotEmpty(t, string(res)) } -func runExplainExpectError(ctx context.Context, t *testing.T, params *agentpb.StartActionRequest_MongoDBExplainParams) { +func runExplainExpectError(ctx context.Context, t *testing.T, params *agentv1.StartActionRequest_MongoDBExplainParams) { t.Helper() big, err := rand.Int(rand.Reader, big.NewInt(27))