Add custom order support for console encode #1496
Conversation
|
|
WalkthroughThe changes introduce a configurable and ordered field output for the console encoder in the logging system. A new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ConsoleEncoder
participant Buffer
participant JSONEncoder
App->>ConsoleEncoder: EncodeEntry(entry, fields)
ConsoleEncoder->>ConsoleEncoder: Iterate ConsoleFieldOrder
alt Field is Message
ConsoleEncoder->>Buffer: Append raw message
ConsoleEncoder->>JSONEncoder: Encode context fields as JSON
JSONEncoder-->>ConsoleEncoder: JSON context string
ConsoleEncoder->>Buffer: Append context JSON
else Field is Stack
ConsoleEncoder->>Buffer: Append stack trace (with newline if set)
else Other Fields
ConsoleEncoder->>Buffer: Append field value
end
ConsoleEncoder-->>App: Return formatted Buffer
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
halfcrazy
force-pushed
the
feat-console-order
branch
from
8ee21f7 to
f7cc1ba
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
zapcore/encoder.go (1)
359-360: Typo & wording – “filed” → “field”The comment reads “Configures the console filed order”.
Spelling should be “field”, and adding an article clarifies the intent.- // Configures the console filed order. Defaults to Time Level Name Callee Function Message Stack + // Configures the console **field** order. Defaults to Time Level Name Callee Function Message Stackzapcore/console_encoder.go (3)
37-47: Exported identifiers need package‑doc comments
OrderFieldand the associated constants are exported but undocumented, which triggersreviveand hampers godoc usability. Add a short comment block explaining the purpose and allowed values.Example:
// OrderField represents one of the logical parts of a log entry that can be // arranged by ConsoleFieldOrder. type OrderField string🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 37-37: exported: exported type OrderField should have comment or be unexported
(revive)
[warning] 40-40: exported: exported const OrderFieldTime should have comment (or a comment on this block) or be unexported
(revive)
103-169: Potential memory churn – cloning a JSON encoder per entryInside
OrderFieldMessagea new JSON encoder is cloned every time an entry is logged merely to encode the context. For high‑volume logging this is a noticeable allocation hotspot.Consider re‑using a scratch encoder from a sync.Pool, or extending
sliceArrayEncoderto accept key/value pairs directly, eliminating the clone altogether.
This is an optimisation suggestion; correctness is unaffected.
213-239: Dead code –writeContextandaddSeparatorIfNecessaryAfter the refactor
EncodeEntryno longer callswriteContext; consequently bothwriteContextand its helper are now unused.Consider removing them to keep the codebase clean and to silence dead‑code linters.
🧰 Tools
🪛 golangci-lint (1.64.8)
213-213: func
consoleEncoder.writeContextis unused(unused)
234-234: func
consoleEncoder.addSeparatorIfNecessaryis unused(unused)
zapcore/console_encoder_test.go (1)
131-135: Formatting nits picked up by gofumpt
golangci-lintflags several “file is not properly formatted” warnings.
Runninggofumpt -w zapcore/console_encoder_test.gowill auto‑fix spacing and import grouping without altering logic.
Keeping tests formatted improves diff readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
zapcore/console_encoder.go(3 hunks)zapcore/console_encoder_test.go(1 hunks)zapcore/encoder.go(1 hunks)zapcore/json_encoder.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
zapcore/encoder.go (1)
zapcore/console_encoder.go (1)
OrderField(37-37)
zapcore/console_encoder_test.go (5)
zapcore/console_encoder.go (9)
OrderField(37-37)OrderFieldTime(40-40)OrderFieldLevel(41-41)OrderFieldName(42-42)OrderFieldCallee(43-43)OrderFieldFunction(44-44)OrderFieldMessage(45-45)OrderFieldStack(46-46)NewConsoleEncoder(73-84)field.go (2)
String(222-224)Time(347-352)zapcore/encoder.go (1)
EncoderConfig(331-361)zapcore/field.go (1)
StringType(67-67)zapcore/entry.go (2)
Entry(143-150)EntryCaller(70-76)
zapcore/console_encoder.go (2)
zapcore/encoder.go (2)
Encoder(457-468)FullNameEncoder(313-315)field.go (3)
Time(347-352)String(222-224)Stack(367-369)
🪛 golangci-lint (1.64.8)
zapcore/console_encoder_test.go
145-145: File is not properly formatted
(gofumpt)
223-223: File is not properly formatted
(gofumpt)
248-248: File is not properly formatted
(gofumpt)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
330-330: File is not properly formatted
(gofumpt)
342-342: File is not properly formatted
(gofumpt)
354-354: File is not properly formatted
(gofumpt)
zapcore/console_encoder.go
[warning] 37-37: exported: exported type OrderField should have comment or be unexported
(revive)
[warning] 40-40: exported: exported const OrderFieldTime should have comment (or a comment on this block) or be unexported
(revive)
49-49: File is not properly formatted
(gofumpt)
zapcore/json_encoder.go
Outdated
| if cfg.ConsoleFieldOrder == nil { | ||
| cfg.ConsoleFieldOrder = defaultConsoleOrder | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prevent silent mis‑configuration when order slice is non‑nil but empty
ConsoleFieldOrder == nil is caught, but an empty slice (len == 0) still bypasses the default and later causes the encoder to emit nothing. Guard both cases:
-if cfg.ConsoleFieldOrder == nil {
+if len(cfg.ConsoleFieldOrder) == 0 {
cfg.ConsoleFieldOrder = defaultConsoleOrder
}To verify that switching to a len(...) == 0 check safely covers both nil and empty slices across the codebase, let’s locate where ConsoleFieldOrder and defaultConsoleOrder are defined and referenced:
🏁 Script executed:
#!/bin/bash
# Find the definition and usage of ConsoleFieldOrder in the JSON encoder:
rg "ConsoleFieldOrder" -n zapcore/json_encoder.go
# Locate where defaultConsoleOrder is declared:
rg "defaultConsoleOrder" -n zapcore/json_encoder.go
# Check other references to ConsoleFieldOrder across the repo:
rg "ConsoleFieldOrder" -n .Length of output: 1051
Guard against empty ConsoleFieldOrder slices
In zapcore/json_encoder.go (lines 93–95), the current nil‑only check lets an explicitly empty slice slip through and causes the encoder to emit nothing. Update the guard to cover both nil and empty slices:
- if cfg.ConsoleFieldOrder == nil {
+ if len(cfg.ConsoleFieldOrder) == 0 {
cfg.ConsoleFieldOrder = defaultConsoleOrder
}Without this change, setting cfg.ConsoleFieldOrder = []OrderField{} silently disables all fields.
You may also want to add a test in zapcore/console_encoder_test.go that verifies an empty slice falls back to defaultConsoleOrder.
halfcrazy
force-pushed
the
feat-console-order
branch
from
f7cc1ba to
0953d90
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
zapcore/console_encoder.go (1)
192-195:⚠️ Potential issueFix type assertions to avoid runtime panics
The current type assertions
e.(int64)ande.(uint64)will panic if the matched element is not exactly of that type (e.g., when it's anint32). This is a critical issue as this branch will be hit for numeric timestamps and counters.Replace the type assertions with individual handling for each numeric type:
- case int, int8, int16, int32, int64: - line.AppendInt(int64(e.(int64))) - case uint, uint8, uint16, uint32, uint64: - line.AppendUint(uint64(e.(uint64))) + case int: + line.AppendInt(int64(e)) + case int8: + line.AppendInt(int64(e)) + case int16: + line.AppendInt(int64(e)) + case int32: + line.AppendInt(int64(e)) + case int64: + line.AppendInt(e) + case uint: + line.AppendUint(uint64(e)) + case uint8: + line.AppendUint(uint64(e)) + case uint16: + line.AppendUint(uint64(e)) + case uint32: + line.AppendUint(uint64(e)) + case uint64: + line.AppendUint(e)
🧹 Nitpick comments (7)
zapcore/console_encoder.go (3)
37-47: Add documentation for exported type and constantsThe exported type
OrderFieldand its constants (OrderFieldTime,OrderFieldLevel, etc.) should have proper documentation comments explaining their purpose and usage, which is to specify the order of fields in the console encoder output.+// OrderField represents a field that can be included in console encoder output. type OrderField string +// Console encoder field constants that can be used to configure field order. const ( + // OrderFieldTime represents the timestamp field OrderFieldTime OrderField = "Time" + // OrderFieldLevel represents the log level field OrderFieldLevel OrderField = "Level" + // OrderFieldName represents the logger name field OrderFieldName OrderField = "Name" + // OrderFieldCallee represents the caller file and line field OrderFieldCallee OrderField = "Callee" + // OrderFieldFunction represents the caller function field OrderFieldFunction OrderField = "Function" + // OrderFieldMessage represents the log message field OrderFieldMessage OrderField = "Message" + // OrderFieldStack represents the stack trace field OrderFieldStack OrderField = "Stack" )🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 37-37: exported: exported type OrderField should have comment or be unexported
(revive)
[warning] 40-40: exported: exported const OrderFieldTime should have comment (or a comment on this block) or be unexported
(revive)
49-50: Format consistent with Go styleThe variable declaration is split across lines in a way that doesn't conform to Go formatting standards.
-var defaultConsoleOrder = []OrderField{OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, - OrderFieldFunction, OrderFieldMessage, OrderFieldStack} +var defaultConsoleOrder = []OrderField{ + OrderFieldTime, + OrderFieldLevel, + OrderFieldName, + OrderFieldCallee, + OrderFieldFunction, + OrderFieldMessage, + OrderFieldStack, +}🧰 Tools
🪛 golangci-lint (1.64.8)
49-49: File is not properly formatted
(gofumpt)
156-160: Consider more efficient string handling for context fieldsThe current implementation converts the context buffer to a string, adds curly braces, and then appends it as a string. This creates multiple string allocations. Consider using direct buffer operations instead.
// Check if the cloned encoder has any content. if contextEncoder.buf.Len() > 0 { - // Manually add curly braces because the buffer contains only the inner KVs. - jsonStr := "{" + contextEncoder.buf.String() + "}" - arr.AppendString(jsonStr) + // Add context as JSON directly to array + arr.AppendString("{") + arr.AppendString(contextEncoder.buf.String()) + arr.AppendString("}") }Or alternatively, for even better performance with no intermediate string allocations:
if contextEncoder.buf.Len() > 0 { - // Manually add curly braces because the buffer contains only the inner KVs. - jsonStr := "{" + contextEncoder.buf.String() + "}" - arr.AppendString(jsonStr) + // Use a temporary buffer to construct the JSON context + contextBuf := bufferpool.Get() + defer contextBuf.Free() + contextBuf.AppendByte('{') + contextBuf.Write(contextEncoder.buf.Bytes()) + contextBuf.AppendByte('}') + arr.AppendString(contextBuf.String()) }zapcore/console_encoder_test.go (4)
145-147: Format list elements consistentlyThis list would be more readable with each element on its own line, following Go style guidelines.
- order: []OrderField{OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, - OrderFieldFunction, OrderFieldMessage, OrderFieldStack}, + order: []OrderField{ + OrderFieldTime, + OrderFieldLevel, + OrderFieldName, + OrderFieldCallee, + OrderFieldFunction, + OrderFieldMessage, + OrderFieldStack, + },🧰 Tools
🪛 golangci-lint (1.64.8)
145-145: File is not properly formatted
(gofumpt)
223-225: Apply consistent formatting to all field order slicesThroughout the test file, there are several multiline field order declarations that could benefit from consistent formatting for improved readability.
The formatting suggestion provided for the first slice at lines 145-147 should be consistently applied to all similar slice declarations in the test file. For example, the slice at lines 330-332 should be formatted as:
- order: []OrderField{OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, - OrderFieldFunction, OrderFieldMessage, OrderFieldStack}, + order: []OrderField{ + OrderFieldTime, + OrderFieldLevel, + OrderFieldName, + OrderFieldCallee, + OrderFieldFunction, + OrderFieldMessage, + OrderFieldStack, + },Also applies to: 276-278, 281-283, 286-288, 330-332, 342-344, 354-356
🧰 Tools
🪛 golangci-lint (1.64.8)
223-223: File is not properly formatted
(gofumpt)
248-248: Simplify field creation syntaxThe Field initialization could be simplified by removing the redundant Field type specification.
- fields := []Field{Field{Key: "foo", Type: StringType, String: "bar"}} + fields := []Field{{Key: "foo", Type: StringType, String: "bar"}}🧰 Tools
🪛 golangci-lint (1.64.8)
248-248: File is not properly formatted
(gofumpt)
303-305: Remove commented-out test assertionsLines 304-305 contain commented-out assertions. These should either be uncommented if necessary or removed completely to maintain clean test code.
- // Can't assert exact output easily due to field order and separator variations - // assert.Contains(t, buf.String(), sep, "Separator not found in output") - // No longer check format, as different console encoder versions might behave differently. + // Note: Exact output verification is challenging due to field order and separator variations
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
zapcore/console_encoder.go(3 hunks)zapcore/console_encoder_test.go(1 hunks)zapcore/encoder.go(1 hunks)zapcore/json_encoder.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- zapcore/encoder.go
🚧 Files skipped from review as they are similar to previous changes (1)
- zapcore/json_encoder.go
🧰 Additional context used
🧬 Code Graph Analysis (2)
zapcore/console_encoder.go (2)
zapcore/encoder.go (2)
Encoder(457-468)FullNameEncoder(313-315)field.go (3)
Time(347-352)String(222-224)Stack(367-369)
zapcore/console_encoder_test.go (5)
zapcore/console_encoder.go (9)
OrderField(37-37)OrderFieldTime(40-40)OrderFieldLevel(41-41)OrderFieldName(42-42)OrderFieldCallee(43-43)OrderFieldFunction(44-44)OrderFieldMessage(45-45)OrderFieldStack(46-46)NewConsoleEncoder(73-84)field.go (2)
String(222-224)Time(347-352)zapcore/encoder.go (1)
EncoderConfig(331-361)zapcore/field.go (1)
StringType(67-67)zapcore/entry.go (2)
Entry(143-150)EntryCaller(70-76)
🪛 golangci-lint (1.64.8)
zapcore/console_encoder.go
[warning] 37-37: exported: exported type OrderField should have comment or be unexported
(revive)
[warning] 40-40: exported: exported const OrderFieldTime should have comment (or a comment on this block) or be unexported
(revive)
49-49: File is not properly formatted
(gofumpt)
zapcore/console_encoder_test.go
145-145: File is not properly formatted
(gofumpt)
223-223: File is not properly formatted
(gofumpt)
248-248: File is not properly formatted
(gofumpt)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
330-330: File is not properly formatted
(gofumpt)
342-342: File is not properly formatted
(gofumpt)
354-354: File is not properly formatted
(gofumpt)
🔇 Additional comments (1)
zapcore/console_encoder_test.go (1)
137-418: Great comprehensive test coverageThe test suite thoroughly covers critical aspects of the field ordering feature:
- Different permutations of field orders
- Context field handling with various field orders
- Different separator strings
- Missing field scenarios
This comprehensive approach ensures the feature works as expected in various scenarios. The debug information in the tests will also be helpful for diagnosing any future issues.
🧰 Tools
🪛 golangci-lint (1.64.8)
145-145: File is not properly formatted
(gofumpt)
223-223: File is not properly formatted
(gofumpt)
248-248: File is not properly formatted
(gofumpt)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
330-330: File is not properly formatted
(gofumpt)
342-342: File is not properly formatted
(gofumpt)
354-354: File is not properly formatted
(gofumpt)
halfcrazy
force-pushed
the
feat-console-order
branch
from
0953d90 to
8329c71
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
zapcore/console_encoder.go (2)
37-47: Add godoc comments for exported type and constantsThe
OrderFieldtype and its associated constants are exported but lack documentation comments. Following Go best practices, all exported declarations should have comments that explain their purpose and usage.+// OrderField defines the type for fields that can be ordered in console encoder output. type OrderField string const ( + // OrderFieldTime represents the timestamp field in log entries. OrderFieldTime OrderField = "Time" + // OrderFieldLevel represents the log level field. OrderFieldLevel OrderField = "Level" + // OrderFieldName represents the logger name field. OrderFieldName OrderField = "Name" + // OrderFieldCallee represents the caller (file:line) field. OrderFieldCallee OrderField = "Callee" + // OrderFieldFunction represents the calling function name field. OrderFieldFunction OrderField = "Function" + // OrderFieldMessage represents the log message field. OrderFieldMessage OrderField = "Message" + // OrderFieldStack represents the stack trace field. OrderFieldStack OrderField = "Stack" )🧰 Tools
🪛 golangci-lint (1.64.8)
[warning] 37-37: exported: exported type OrderField should have comment or be unexported
(revive)
[warning] 40-40: exported: exported const OrderFieldTime should have comment (or a comment on this block) or be unexported
(revive)
49-50: Fix formatting of multi-line slice declarationThe declaration of
defaultConsoleOrderspans multiple lines but doesn't follow Go formatting conventions. The slice elements should either all be on one line or each on its own line with proper indentation.-var defaultConsoleOrder = []OrderField{OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, - OrderFieldFunction, OrderFieldMessage, OrderFieldStack} +var defaultConsoleOrder = []OrderField{ + OrderFieldTime, + OrderFieldLevel, + OrderFieldName, + OrderFieldCallee, + OrderFieldFunction, + OrderFieldMessage, + OrderFieldStack, +}🧰 Tools
🪛 golangci-lint (1.64.8)
49-49: File is not properly formatted
(gofumpt)
zapcore/console_encoder_test.go (3)
145-147: Fix multi-line slice formatting in test casesMultiple test cases have slice initializers that span multiple lines without proper Go formatting. Similar to the issue in the main file, these should be properly formatted either as single-line or with each element on its own line.
For example:
- order: []OrderField{OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, - OrderFieldFunction, OrderFieldMessage, OrderFieldStack}, + order: []OrderField{ + OrderFieldTime, + OrderFieldLevel, + OrderFieldName, + OrderFieldCallee, + OrderFieldFunction, + OrderFieldMessage, + OrderFieldStack, + },This same pattern should be applied to all similar multi-line slice initializations throughout the test file.
Also applies to: 223-225, 276-278, 281-283, 286-288, 330-332, 342-344, 354-356
🧰 Tools
🪛 golangci-lint (1.64.8)
145-145: File is not properly formatted
(gofumpt)
248-249: Simplify Field creationThe test is creating a Field directly with struct initialization, but the codebase likely has helper functions for this purpose.
- fields := []Field{Field{Key: "foo", Type: StringType, String: "bar"}} + fields := []Field{String("foo", "bar")}🧰 Tools
🪛 golangci-lint (1.64.8)
248-248: File is not properly formatted
(gofumpt)
268-313: Add assertions to separator testingThis test currently logs output but doesn't make any assertions about the results. While exact output matching might be difficult, you could still verify certain properties of the output.
Consider adding basic assertions to verify that:
- The output contains the message
- The separator appears in the output
- The number of separators is appropriate for the fields present
For example:
// Count occurrences of the separator in the output sepCount := strings.Count(consoleOut.String(), sep) // Calculate expected separator count (fields - 1, accounting for missing fields) expectedFields := countPresentFields(entry, o.order) expectedSeps := expectedFields - 1 if expectedSeps > 0 { assert.Equal(t, expectedSeps, sepCount, "Unexpected number of separators") } assert.Contains(t, consoleOut.String(), entry.Message, "Output should contain the message")You would need to implement a helper
countPresentFields()function to determine how many fields are actually present in the output.🧰 Tools
🪛 golangci-lint (1.64.8)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
zapcore/console_encoder.go(3 hunks)zapcore/console_encoder_test.go(1 hunks)zapcore/encoder.go(1 hunks)zapcore/json_encoder.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- zapcore/encoder.go
- zapcore/json_encoder.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
zapcore/console_encoder_test.go (5)
zapcore/console_encoder.go (9)
OrderField(37-37)OrderFieldTime(40-40)OrderFieldLevel(41-41)OrderFieldName(42-42)OrderFieldCallee(43-43)OrderFieldFunction(44-44)OrderFieldMessage(45-45)OrderFieldStack(46-46)NewConsoleEncoder(73-84)field.go (2)
String(222-224)Time(347-352)zapcore/encoder.go (1)
EncoderConfig(331-361)zapcore/field.go (1)
StringType(67-67)zapcore/entry.go (2)
Entry(143-150)EntryCaller(70-76)
🪛 golangci-lint (1.64.8)
zapcore/console_encoder.go
[warning] 37-37: exported: exported type OrderField should have comment or be unexported
(revive)
[warning] 40-40: exported: exported const OrderFieldTime should have comment (or a comment on this block) or be unexported
(revive)
49-49: File is not properly formatted
(gofumpt)
zapcore/console_encoder_test.go
145-145: File is not properly formatted
(gofumpt)
223-223: File is not properly formatted
(gofumpt)
248-248: File is not properly formatted
(gofumpt)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
330-330: File is not properly formatted
(gofumpt)
342-342: File is not properly formatted
(gofumpt)
354-354: File is not properly formatted
(gofumpt)
🔇 Additional comments (4)
zapcore/console_encoder.go (2)
103-167: Well-structured field handling based on configured orderThe field processing approach is well-designed, iterating through the configured order and handling each field appropriately. This implementation successfully handles field ordering while maintaining proper type handling and formatting.
171-222: Properly fixed type assertion and optimized buffer handlingThe buffer writing loop is well-optimized with proper type handling. This approach correctly handles each numeric type individually, avoiding the previous potential runtime panics from incorrect type assertions. The special handling of newline-prefixed stack traces is also a nice touch.
zapcore/console_encoder_test.go (2)
137-207: Thorough test coverage for field ordering functionalityThe test cases provide excellent coverage of different field ordering scenarios, with clear expected outputs for each case. The debug output inclusion is also helpful for troubleshooting.
🧰 Tools
🪛 golangci-lint (1.64.8)
145-145: File is not properly formatted
(gofumpt)
315-418: Comprehensive testing of edge casesThe tests for missing fields are thorough and well-structured, covering important edge cases like zero time values, missing logger names, undefined callers, and empty stacks. This attention to edge cases is commendable.
🧰 Tools
🪛 golangci-lint (1.64.8)
330-330: File is not properly formatted
(gofumpt)
342-342: File is not properly formatted
(gofumpt)
354-354: File is not properly formatted
(gofumpt)
halfcrazy
force-pushed
the
feat-console-order
branch
from
8329c71 to
9997486
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
zapcore/console_encoder_test.go (3)
248-248: Simplify field declaration by removing redundant type.The field declaration can be simplified by removing the redundant type specification.
- fields := []Field{Field{Key: "foo", Type: StringType, String: "bar"}} + fields := []Field{{Key: "foo", Type: StringType, String: "bar"}}🧰 Tools
🪛 golangci-lint (1.64.8)
248-248: File is not properly formatted
(gofumpt)
268-311: Consider adding basic assertions to the separator test.While exact output verification may be challenging with different separators and field orders, consider adding some basic assertions to verify that the output contains the expected fields in some form, rather than just logging the output.
You could add assertions that check for the presence of specific fields in the output, such as:
// Example assertion to check if all fields are present output := consoleOut.String() assert.Contains(t, output, "hello") assert.Contains(t, output, "info") assert.Contains(t, output, sep) // Check separator is used🧰 Tools
🪛 golangci-lint (1.64.8)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
145-147: Fix formatting of multiline slices according to gofumpt.The static analysis tool golangci-lint (gofumpt) indicates formatting issues with these multiline slice literals. According to Go formatting conventions, all elements in a multiline slice literal should be indented consistently.
For example, apply this change to properly format these multiline slices:
- order: []OrderField{OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, - OrderFieldFunction, OrderFieldMessage, OrderFieldStack}, + order: []OrderField{ + OrderFieldTime, OrderFieldLevel, OrderFieldName, OrderFieldCallee, + OrderFieldFunction, OrderFieldMessage, OrderFieldStack, + },Also applies to: 223-225, 276-278, 281-283, 286-288, 328-330, 340-342, 352-354, 364-366, 390-392
🧰 Tools
🪛 golangci-lint (1.64.8)
145-145: File is not properly formatted
(gofumpt)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
zapcore/console_encoder.go(3 hunks)zapcore/console_encoder_test.go(1 hunks)zapcore/encoder.go(1 hunks)zapcore/json_encoder.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- zapcore/encoder.go
- zapcore/json_encoder.go
- zapcore/console_encoder.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
zapcore/console_encoder_test.go (5)
zapcore/console_encoder.go (9)
OrderField(38-38)OrderFieldTime(43-43)OrderFieldLevel(45-45)OrderFieldName(47-47)OrderFieldCallee(49-49)OrderFieldFunction(51-51)OrderFieldMessage(53-53)OrderFieldStack(55-55)NewConsoleEncoder(89-100)field.go (2)
String(222-224)Time(347-352)zapcore/encoder.go (1)
EncoderConfig(331-361)zapcore/field.go (1)
StringType(67-67)zapcore/entry.go (2)
Entry(143-150)EntryCaller(70-76)
🪛 golangci-lint (1.64.8)
zapcore/console_encoder_test.go
145-145: File is not properly formatted
(gofumpt)
223-223: File is not properly formatted
(gofumpt)
248-248: File is not properly formatted
(gofumpt)
276-276: File is not properly formatted
(gofumpt)
281-281: File is not properly formatted
(gofumpt)
286-286: File is not properly formatted
(gofumpt)
328-328: File is not properly formatted
(gofumpt)
340-340: File is not properly formatted
(gofumpt)
352-352: File is not properly formatted
(gofumpt)
364-364: File is not properly formatted
(gofumpt)
🔇 Additional comments (4)
zapcore/console_encoder_test.go (4)
137-207: Great test coverage for the new field ordering functionality.This test thoroughly covers various field ordering scenarios, from default order to custom orders with fields in different positions. The test cases are comprehensive and verify that the console encoder correctly respects the specified order of fields.
🧰 Tools
🪛 golangci-lint (1.64.8)
145-145: File is not properly formatted
(gofumpt)
209-213: Clean helper function implementation.This helper function nicely encapsulates the creation of an encoder config with the specified field order, keeping the test code DRY.
215-266: Good test coverage for context fields with different ordering configurations.The test properly verifies that context fields are correctly included in the output alongside the ordered entry fields.
🧰 Tools
🪛 golangci-lint (1.64.8)
223-223: File is not properly formatted
(gofumpt)
248-248: File is not properly formatted
(gofumpt)
313-416: Excellent test coverage for missing fields scenarios.This test thoroughly covers various scenarios where fields may be missing from the entry, ensuring that the encoder handles these cases correctly. The test cases are comprehensive and well-structured.
🧰 Tools
🪛 golangci-lint (1.64.8)
328-328: File is not properly formatted
(gofumpt)
340-340: File is not properly formatted
(gofumpt)
352-352: File is not properly formatted
(gofumpt)
364-364: File is not properly formatted
(gofumpt)
fix
#1031
#499
#388 (comment)
Summary by CodeRabbit