Skip to content

Commit

Permalink
util/log: make logpb.Entry slightly more able to represent crdb-v2 …
Browse files Browse the repository at this point in the history
…entries

Prior to this change, we did not have a public data structure able to
represent the various fields available in a `crdb-v2` log entry: the
existing `logpb.Entry` was not able to distinguish structured and
non-structured entries, and did not have the ability to delimit the
message and the stack trace.

This patch extends `logpb.Entry` to make it more able to represent
`crdb-v2` entries, at least for the purpose of extending `debug
merge-log` towards conversion between logging formats.

Additionally, the patch adds a *best effort* attempt at populating
the new fields in the `crdb-v1` entry parser. This is best effort
because the crdb-v1 parser is lossy so we cannot faithfully
parse its entries reliably.

Release note: None
  • Loading branch information
knz committed Jun 5, 2021
1 parent 1d88b9d commit bab7d79
Show file tree
Hide file tree
Showing 8 changed files with 234 additions and 63 deletions.
2 changes: 1 addition & 1 deletion docs/generated/logformats.md
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ to indicate whether the remainder of the line is a
structured entry, or a continuation of a previous entry.

Finally, in the previous format, structured entries
were prefixed with the string `Structured entry:`. In
were prefixed with the string `Structured entry: `. In
the new format, they are prefixed by the `=` continuation
indicator.

Expand Down
21 changes: 21 additions & 0 deletions pkg/util/log/format_crdb_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,27 @@ func (d *EntryDecoder) Decode(entry *logpb.Entry) error {
r = d.sensitiveEditor(r)
entry.Message = string(r.msg)
entry.Redactable = r.redactable

if strings.HasPrefix(entry.Message, structuredEntryPrefix+"{") || /* crdb-v1 prefix */
strings.HasPrefix(entry.Message, " ={") /* crdb-v2 prefix, happens when using the v1 parser against a v2 file */ {
if entry.Message[0] == ' ' {
entry.StructuredStart = 2
} else {
entry.StructuredStart = uint32(len(structuredEntryPrefix))
}
if nl := strings.IndexByte(entry.Message, '\n'); nl != -1 {
entry.StructuredEnd = uint32(nl)
entry.StackTraceStart = uint32(nl + 1)
} else {
entry.StructuredEnd = uint32(len(entry.Message))
}
}
// Note: we only know how to populate entry.StackTraceStart upon
// parse if the entry was structured (see above). If it is not
// structured, we cannot distinguish where the message ends and
// where the stack trace starts. This is another reason why the
// crdb-v1 format is lossy.

return nil
}
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/util/log/format_crdb_v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ func TestCrdbV1EncodeDecode(t *testing.T) {
buf.WriteString("# after parse:\n")
for _, entry := range outputEntries {
fmt.Fprintf(&buf, "%# v\n", pretty.Formatter(entry))
if entry.StructuredEnd != 0 {
var payload interface{}
if err := json.Unmarshal([]byte(entry.Message[entry.StructuredStart:entry.StructuredEnd]), &payload); err != nil {
td.Fatalf(t, "JSON entry not parsable: %v", err)
}
fmt.Fprintf(&buf, "JSON payload in previous entry: %+v\n", payload)
}
}
return buf.String()

Expand Down
2 changes: 1 addition & 1 deletion pkg/util/log/format_crdb_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ to indicate whether the remainder of the line is a
structured entry, or a continuation of a previous entry.
Finally, in the previous format, structured entries
were prefixed with the string ` + "`Structured entry:`" + `. In
were prefixed with the string ` + "`" + structuredEntryPrefix + "`" + `. In
the new format, they are prefixed by the ` + "`=`" + ` continuation
indicator.
`)
Expand Down
7 changes: 6 additions & 1 deletion pkg/util/log/log_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,16 +248,21 @@ func (e logEntry) convertToLegacy() (res logpb.Entry) {
// At this point, the message only contains the JSON fields of the
// payload. Add the decoration suitable for our legacy file
// format.
res.Message = "Structured entry: {" + res.Message + "}"
res.Message = structuredEntryPrefix + "{" + res.Message + "}"
res.StructuredStart = uint32(len(structuredEntryPrefix))
res.StructuredEnd = uint32(len(res.Message))
}

if e.stacks != nil {
res.StackTraceStart = uint32(len(res.Message)) + 1
res.Message += "\n" + string(e.stacks)
}

return res
}

const structuredEntryPrefix = "Structured entry: "

func renderTagsAsString(tags *logtags.Buffer, redactable bool) string {
if redactable {
return string(renderTagsAsRedactable(tags))
Expand Down
193 changes: 149 additions & 44 deletions pkg/util/log/logpb/log.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions pkg/util/log/logpb/log.proto
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,28 @@ message Entry {

// Channel is the channel on which the message was sent.
Channel channel = 10;

// StructuredEnd, if non-zero, indicates that the entry
// is structured; it is also the index
// inside the Message field where the JSON payload ends (exclusive).
uint32 structured_end = 11;

// StructuredStart, when StructuredEnd is non-zero, is the index
// inside the Message field where the JSON payload starts (inclusive).
uint32 structured_start = 12;

// StackTraceStart is the index inside Message where a detailed
// stack trace starts. If zero, no stack trace is present. Stack
// traces are always separated from the message using a newline
// character. If a stack trace is included, StackTracePosition is
// the index of the character immediately after the newline
// character.
//
// We use an index-in-string field in the protobuf, instead of two
// separate string fields, because previous-version consumers of
// Entry are still expecting the message and the stack trace in the
// same field.
uint32 stack_trace_start = 13;
}

// A FileDetails holds all of the particulars that can be parsed by the name of
Expand Down
Loading

0 comments on commit bab7d79

Please sign in to comment.