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

Fix logging of deeply nested documents #4167

Merged
merged 7 commits into from Mar 12, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -68,7 +68,7 @@ require (
golang.org/x/net v0.21.0 // indirect
golang.org/x/sync v0.6.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/tools v0.18.0 // indirect
golang.org/x/tools v0.18.0 // indirect; https://github.com/golang/go/issues/66259
google.golang.org/genproto/googleapis/api v0.0.0-20240102182953-50ed04b92917 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240102182953-50ed04b92917 // indirect
google.golang.org/grpc v1.61.0 // indirect
Expand Down
52 changes: 0 additions & 52 deletions integration/basic_test.go
Expand Up @@ -552,58 +552,6 @@ func TestDebugError(t *testing.T) {
})
}

func TestCheckingNestedDocuments(t *testing.T) {
t.Skip("https://github.com/FerretDB/FerretDB/issues/3759")

for name, tc := range map[string]struct {
doc any
err *mongo.CommandError
}{
"1ok": {
doc: CreateNestedDocument(1),
},
"10ok": {
doc: CreateNestedDocument(10),
},
"100ok": {
doc: CreateNestedDocument(100),
},
"179ok": {
doc: CreateNestedDocument(179),
},
"180fail": {
doc: CreateNestedDocument(180),
err: &mongo.CommandError{
Message: "bson.Array.ReadFrom (document has exceeded the max supported nesting: 179.",
},
},
"180endedWithDocumentFail": {
doc: bson.D{{"v", CreateNestedDocument(179)}},
err: &mongo.CommandError{
Message: "bson.Document.ReadFrom (document has exceeded the max supported nesting: 179.",
},
},
"1000fail": {
doc: CreateNestedDocument(1000),
err: &mongo.CommandError{
Message: "bson.Document.ReadFrom (document has exceeded the max supported nesting: 179.",
},
},
} {
t.Run(name, func(t *testing.T) {
ctx, collection := setup.Setup(t)

_, err := collection.InsertOne(ctx, tc.doc)
if tc.err != nil {
AssertEqualCommandError(t, *tc.err, err)
return
}

require.NoError(t, err)
})
}
}

func TestPingCommand(t *testing.T) {
t.Parallel()

Expand Down
6 changes: 3 additions & 3 deletions integration/benchmarks_test.go
Expand Up @@ -54,7 +54,7 @@ func BenchmarkFind(b *testing.B) {
b.Run(name, func(b *testing.B) {
var firstDocs, docs int

for i := 0; i < b.N; i++ {
for range b.N {
cursor, err := s.Collection.Find(s.Ctx, bc.filter)
require.NoError(b, err)

Expand Down Expand Up @@ -112,7 +112,7 @@ func BenchmarkReplaceOne(b *testing.B) {
filter := bson.D{{"_id", doc[0].Value}}
var res *mongo.UpdateResult

for i := 0; i < b.N; i++ {
for i := range b.N {
doc[1].Value = int64(i + 1)

res, err = collection.ReplaceOne(ctx, filter, doc)
Expand Down Expand Up @@ -148,7 +148,7 @@ func BenchmarkInsertMany(b *testing.B) {
b.Run(fmt.Sprintf("%s/Batch%d", provider.Name(), batchSize), func(b *testing.B) {
b.StopTimer()

for i := 0; i < b.N; i++ {
for range b.N {
require.NoError(b, collection.Drop(ctx))

iter := provider.NewIterator()
Expand Down
2 changes: 1 addition & 1 deletion integration/go.mod
Expand Up @@ -68,7 +68,7 @@ require (
golang.org/x/sync v0.6.0 // indirect
golang.org/x/sys v0.17.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/tools v0.18.0 // indirect
golang.org/x/tools v0.18.0 // indirect; https://github.com/golang/go/issues/66259
google.golang.org/genproto/googleapis/api v0.0.0-20240102182953-50ed04b92917 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240102182953-50ed04b92917 // indirect
google.golang.org/grpc v1.61.0 // indirect
Expand Down
26 changes: 0 additions & 26 deletions integration/integration.go
Expand Up @@ -458,29 +458,3 @@ func GenerateDocuments(startID, endID int32) (bson.A, []bson.D) {

return arr, docs
}

// CreateNestedDocument creates a mock BSON document that consists of nested arrays and documents.
// The nesting level is based on integer parameter.
func CreateNestedDocument(n int) bson.D {
return createNestedDocument(n, false).(bson.D)
}

// createNestedDocument creates the nested n times object that consists of
// documents and arrays. If the arr is true, the root value will be array.
//
// This function should be used only internally.
// To generate values for tests please use
// exported CreateNestedDocument function.
func createNestedDocument(n int, arr bool) any {
var child any

if n > 0 {
child = createNestedDocument(n-1, !arr)
}

if arr {
return bson.A{child}
}

return bson.D{{"v", child}}
}
44 changes: 0 additions & 44 deletions integration/nested_document_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion internal/bson/bson_test.go
Expand Up @@ -228,7 +228,7 @@ func benchmark(b *testing.B, testCases []testCase, newFunc func() bsontype) {
b.SetBytes(br.Size())
b.ResetTimer()

for i := 0; i < b.N; i++ {
for range b.N {
_, seekErr = br.Seek(0, io.SeekStart)

v = newFunc()
Expand Down
2 changes: 1 addition & 1 deletion internal/bson2/array.go
Expand Up @@ -141,7 +141,7 @@

// LogValue implements slog.LogValuer interface.
func (arr *Array) LogValue() slog.Value {
return slogValue(arr)
return slogValue(arr, 1)

Check warning on line 144 in internal/bson2/array.go

View check run for this annotation

Codecov / codecov/patch

internal/bson2/array.go#L144

Added line #L144 was not covered by tests
}

// LogMessage returns an indented representation as a string,
Expand Down
13 changes: 13 additions & 0 deletions internal/bson2/bson2_test.go
Expand Up @@ -285,6 +285,19 @@ var normalTestCases = []normalTestCase{
"timestamp": [Timestamp(42), Timestamp(0)],
}`,
},
{
name: "nested",
raw: testutil.MustParseDumpFile("testdata", "nested.hex"),
tdoc: must.NotFail(makeNested(false, 150).(*Document).Convert()),
m: `
{
"f": [
{
"f": [{"f": [{"f": [{"f": [{"f": [{"f": [{"f": [{"f": [{"f": [{...}]}]}]}]}]}]}]}]}],
},
],
}`,
},
{
name: "float64Doc",
raw: RawDocument{
Expand Down
2 changes: 1 addition & 1 deletion internal/bson2/document.go
Expand Up @@ -175,7 +175,7 @@ func (doc *Document) Encode() (RawDocument, error) {

// LogValue implements slog.LogValuer interface.
func (doc *Document) LogValue() slog.Value {
return slogValue(doc)
return slogValue(doc, 1)
}

// LogMessage returns an indented representation as a string,
Expand Down
51 changes: 35 additions & 16 deletions internal/bson2/logging.go
Expand Up @@ -25,9 +25,12 @@
"time"
)

// flowLimit is the maximum length of a flow/inline/compact representation of a BSON value.
// logMaxFlowLength is the maximum length of a flow/inline/compact representation of a BSON value.
// It may be set to 0 to disable flow representation.
const flowLimit = 80
const logMaxFlowLength = 80

// logMaxDepth is the maximum depth of a recursive representation of a BSON value.
const logMaxDepth = 20

// nanBits is the most common pattern of a NaN float64 value, the same as math.Float64bits(math.NaN()).
const nanBits = 0b111111111111000000000000000000000000000000000000000000000000001
Expand All @@ -42,13 +45,17 @@
// More information is subsequently lost in handlers output;
// for example, float64(42), int32(42), and int64(42) values would all look the same
// (`f64=42 i32=42 i64=42` or `{"f64":42,"i32":42,"i64":42}`).
func slogValue(v any) slog.Value {
func slogValue(v any, depth int) slog.Value {
switch v := v.(type) {
case *Document:
if depth > logMaxDepth {
return slog.StringValue("Document<...>")
}

var attrs []slog.Attr

for _, f := range v.fields {
attrs = append(attrs, slog.Attr{Key: f.name, Value: slogValue(f.value)})
attrs = append(attrs, slog.Attr{Key: f.name, Value: slogValue(f.value, depth+1)})
}

return slog.GroupValue(attrs...)
Expand All @@ -61,10 +68,14 @@
return slog.StringValue("RawDocument<" + strconv.Itoa(len(v)) + ">")

case *Array:
if depth > logMaxDepth {
return slog.StringValue("Array<...>")

Check warning on line 72 in internal/bson2/logging.go

View check run for this annotation

Codecov / codecov/patch

internal/bson2/logging.go#L72

Added line #L72 was not covered by tests
}

var attrs []slog.Attr

for i, v := range v.elements {
attrs = append(attrs, slog.Attr{Key: strconv.Itoa(i), Value: slogValue(v)})
attrs = append(attrs, slog.Attr{Key: strconv.Itoa(i), Value: slogValue(v, depth+1)})
}

return slog.GroupValue(attrs...)
Expand Down Expand Up @@ -131,24 +142,28 @@
// The result is optimized for large values such as full request documents.
// All information is preserved.
func logMessage(v any) string {
return logMessageIndent(v, "")
return logMessageIndent(v, "", 1)
}

// logMessageIndent is a variant of [logMessage] with an indentation for recursive calls.
func logMessageIndent(v any, indent string) string {
// logMessageIndent is a variant of [logMessage] with an indentation and depth for recursive calls.
func logMessageIndent(v any, indent string, depth int) string {
switch v := v.(type) {
case *Document:
l := len(v.fields)
if l == 0 {
return "{}"
}

if flowLimit > 0 {
if depth > logMaxDepth {
return "{...}"
}

if logMaxFlowLength > 0 {
res := "{"

for i, f := range v.fields {
res += strconv.Quote(f.name) + `: `
res += logMessageIndent(f.value, "")
res += logMessageIndent(f.value, "", depth+1)

if i != l-1 {
res += ", "
Expand All @@ -157,7 +172,7 @@

res += `}`

if len(res) < flowLimit {
if len(res) < logMaxFlowLength {
return res
}
}
Expand All @@ -167,7 +182,7 @@
for _, f := range v.fields {
res += indent + " "
res += strconv.Quote(f.name) + `: `
res += logMessageIndent(f.value, indent+" ") + ",\n"
res += logMessageIndent(f.value, indent+" ", depth+1) + ",\n"
}

res += indent + `}`
Expand All @@ -183,11 +198,15 @@
return "[]"
}

if flowLimit > 0 {
if depth > logMaxDepth {
return "[...]"

Check warning on line 202 in internal/bson2/logging.go

View check run for this annotation

Codecov / codecov/patch

internal/bson2/logging.go#L202

Added line #L202 was not covered by tests
}

if logMaxFlowLength > 0 {
res := "["

for i, e := range v.elements {
res += logMessageIndent(e, "")
res += logMessageIndent(e, "", depth+1)

if i != l-1 {
res += ", "
Expand All @@ -196,7 +215,7 @@

res += `]`

if len(res) < flowLimit {
if len(res) < logMaxFlowLength {
return res
}
}
Expand All @@ -205,7 +224,7 @@

for _, e := range v.elements {
res += indent + " "
res += logMessageIndent(e, indent+" ") + ",\n"
res += logMessageIndent(e, indent+" ", depth+1) + ",\n"
}

res += indent + `]`
Expand Down