Skip to content

Preserve structured log attributes in miren logs system #662

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.

Sign up for GitHub

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

Merged
phinze merged 2 commits into from
Mar 11, 2026
Merged

Preserve structured log attributes in miren logs system #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd020f4
Add native map type support to RPC code generation
phinze Mar 11, 2026
625f065
Preserve structured log attributes through to CLI output
phinze Mar 11, 2026
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
26 changes: 22 additions & 4 deletions api/app/app_v1alpha/rpc.gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package app_v1alpha
import (
"context"
"encoding/json"
"maps"
"slices"

"github.com/fxamacker/cbor/v2"
Expand Down Expand Up @@ -1376,10 +1377,11 @@ func (v *ApplicationStatus) UnmarshalJSON(data []byte) error {
}

type logEntryData struct {
Timestamp *standard.Timestamp `cbor:"0,keyasint,omitempty" json:"timestamp,omitempty"`
Line *string `cbor:"1,keyasint,omitempty" json:"line,omitempty"`
Stream *string `cbor:"2,keyasint,omitempty" json:"stream,omitempty"`
Source *string `cbor:"3,keyasint,omitempty" json:"source,omitempty"`
Timestamp *standard.Timestamp `cbor:"0,keyasint,omitempty" json:"timestamp,omitempty"`
Line *string `cbor:"1,keyasint,omitempty" json:"line,omitempty"`
Stream *string `cbor:"2,keyasint,omitempty" json:"stream,omitempty"`
Source *string `cbor:"3,keyasint,omitempty" json:"source,omitempty"`
Attributes *map[string]string `cbor:"4,keyasint,omitempty" json:"attributes,omitempty"`
}

type LogEntry struct {
Expand Down Expand Up @@ -1443,6 +1445,22 @@ func (v *LogEntry) SetSource(source string) {
v.data.Source = &source
}

func (v *LogEntry) HasAttributes() bool {
return v.data.Attributes != nil
}

func (v *LogEntry) Attributes() map[string]string {
if v.data.Attributes == nil {
return nil
}
return *v.data.Attributes
}

func (v *LogEntry) SetAttributes(attributes map[string]string) {
x := maps.Clone(attributes)
v.data.Attributes = &x
}

func (v *LogEntry) MarshalCBOR() ([]byte, error) {
return cbor.Marshal(v.data)
}
Expand Down
5 changes: 5 additions & 0 deletions api/app/rpc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,11 @@ types:
- name: source
type: string
index: 3
- name: attributes
type: map
key: string
value: string
index: 4

- type: LogChunk
fields:
Expand Down
35 changes: 33 additions & 2 deletions cli/commands/logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package commands

import (
"fmt"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -238,11 +239,41 @@ func printLogEntry(ctx *Context, l *app_v1alpha.LogEntry) {
}
prefix = "[" + source + "] "
}
ctx.Printf("%s %s: %s%s\n",
attrs := ""
if l.HasAttributes() {
attrs = formatAttributes(l.Attributes())
}
ctx.Printf("%s %s: %s%s%s\n",
streamTypePrefixes[l.Stream()],
standard.FromTimestamp(l.Timestamp()).Format("2006-01-02 15:04:05"),
prefix,
l.Line())
l.Line(),
attrs)
}

func formatAttributes(m map[string]string) string {
if len(m) == 0 {
return ""
}
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
slices.Sort(keys)

var b strings.Builder
for _, k := range keys {
b.WriteByte(' ')
b.WriteString(k)
b.WriteByte('=')
v := m[k]
if strings.ContainsAny(v, " \t\"\n\r") {
fmt.Fprintf(&b, "%q", v)
} else {
b.WriteString(v)
}
}
return b.String()
}

func streamLogs(ctx *Context, cl *rpc.NetworkClient, app, sandbox string, last *time.Duration, follow bool, filter *logfilter.Filter) error {
Expand Down
143 changes: 143 additions & 0 deletions pkg/rpc/generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ func (g *Generator) properType(name string) *j.Statement {
return j.Id(name)
}

func (g *Generator) mapType(field *DescField) *j.Statement {
return j.Map(g.properType(field.Key)).Add(g.properType(field.Value))
}
Comment on lines +282 to +284
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Go, what types are valid as map keys, and which map key types does encoding/json support for Marshal and Unmarshal?

💡 Result:

Valid Go map key types (language rule)

A Go map key type must be a type for which the comparison operators == and != are fully defined—so the key type must not be a function, map, or slice type. If the key type is an interface, the dynamic (concrete) values stored in the interface must also be comparable, or comparisons may panic at runtime. [1]

Practically, this means map keys can be (among others): booleans, numbers, strings, pointers, channels, interfaces (with the caveat above), and arrays/structs as long as all their elements/fields are comparable. [1]

Map key types supported by encoding/json

json.Marshal supports map keys whose type is: any string type, any integer type, or a type that implements encoding.TextMarshaler. Integer keys are converted to strings; TextMarshaler keys use MarshalText() and become JSON strings. [2]

json.Unmarshal supports map keys whose type is: any string type, any integer type, or a type that implements encoding.TextUnmarshaler (decoded from the JSON object key string). [2]

Note: JSON object keys are strings, so non-string Go map keys are handled via these conversions. [2]

Sources: Go spec (map types/comparability) [1]; encoding/json docs for map key handling in Marshal/Unmarshal [2].

[1] (go.dev)
[2] (pkg.go.dev)

Citations:

🏁 Script executed:

# First, let's locate and examine the mapType function and its context
cd /tmp/repo 2>/dev/null || cd . && git ls-files "**/*.go" | head -20

Repository: mirendev/runtime

Length of output: 583

🏁 Script executed:

# Look at the generator.go file around the mapType function
cat -n pkg/rpc/generator.go | sed -n '270,300p'

Repository: mirendev/runtime

Length of output: 949

🏁 Script executed:

# Search for where mapType is called from and how keys are handled
rg -B 3 -A 3 "mapType" pkg/rpc/

Repository: mirendev/runtime

Length of output: 3342

🏁 Script executed:

# Search for any schema validation related to map keys
rg -i "map.*key|key.*type|validate.*key" pkg/rpc/ --type go

Repository: mirendev/runtime

Length of output: 831

🏁 Script executed:

# Find DescField definition
rg -B 5 -A 10 "type DescField struct" pkg/rpc/

Repository: mirendev/runtime

Length of output: 715

🏁 Script executed:

# Look for schema validation or parsing
rg -i "validate|parse.*schema" pkg/rpc/ --type go | head -30

Repository: mirendev/runtime

Length of output: 366

🏁 Script executed:

# Check test data to see what map key types are being used
fd "testdata" pkg/rpc/ -type d

Repository: mirendev/runtime

Length of output: 230

🏁 Script executed:

# Look at map.go test data
cat pkg/rpc/testdata/map.go

Repository: mirendev/runtime

Length of output: 1492

🏁 Script executed:

# Find and display the Validate method
rg -B 2 -A 30 "func \(t \*DescType\) Validate\(\)" pkg/rpc/generator.go

Repository: mirendev/runtime

Length of output: 904

🏁 Script executed:

# Search for schema files to see what types are actually used
find pkg/rpc/testdata -name "*.yaml" -o -name "*.yml" 2>/dev/null

Repository: mirendev/runtime

Length of output: 249

🏁 Script executed:

# Check if there are any schema files with map fields
rg -l "type:.*map" --type yaml

Repository: mirendev/runtime

Length of output: 100

🏁 Script executed:

# Look for any RPC schema examples
find . -name "*.yaml" -o -name "*.yml" | head -20

Repository: mirendev/runtime

Length of output: 536

🏁 Script executed:

# Check the map.yml test schema
cat pkg/rpc/testdata/map.yml

Repository: mirendev/runtime

Length of output: 277

🏁 Script executed:

# Check a real API schema that uses maps
cat api/app/rpc.yml | head -100

Repository: mirendev/runtime

Length of output: 2085

🏁 Script executed:

# Search for "map" in any schema files to see what key types are used
rg "type:.*map" -A 3 -B 1 --type yaml

Repository: mirendev/runtime

Length of output: 457

🏁 Script executed:

# Search for all map declarations across all schema files
rg -A 5 "type:.*map" api/ --type yaml

Repository: mirendev/runtime

Length of output: 252

🏁 Script executed:

# Check the properType function to see if it does any validation
rg -B 5 -A 15 "func \(g \*Generator\) properType" pkg/rpc/generator.go

Repository: mirendev/runtime

Length of output: 428

🏁 Script executed:

# Look for any comments or documentation about supported map key types
rg -i "map.*key|key.*type.*valid|support.*key" pkg/rpc/ --type go -B 2 -A 2

Repository: mirendev/runtime

Length of output: 1946

Add validation to reject unsupported map key types during schema parsing.

mapType() currently accepts any RPC schema key type without validation. Declarations like key: bytes or key: list would produce invalid Go map types (slices are not comparable and cannot be map keys), and according to the encoding/json package, only string, integer, and TextMarshaler implementations are supported as map keys. This validation should be added to DescType.Validate() instead of silently accepting invalid keys during code generation.


func (g *Generator) deriveType(base, sub string) string {
bracket := strings.IndexByte(base, '[')

Expand Down Expand Up @@ -324,6 +328,11 @@ func (g *Generator) generateServerStructs(f *j.File, t *DescInterface) error {
"json": p.Name + ",omitempty",
})
}
} else if p.Type == "map" {
gr.Id(toCamal(p.Name)).Op("*").Map(g.properType(p.Key)).Add(g.properType(p.Value)).Tag(map[string]string{
"cbor": fmt.Sprintf("%d,keyasint,omitempty", idx),
"json": p.Name + ",omitempty",
})
} else {
gr.Id(capitalize(p.Name)).Op("*").Add(g.properType(p.Type)).Tag(map[string]string{
"cbor": fmt.Sprintf("%d,keyasint,omitempty", idx),
Expand All @@ -350,6 +359,8 @@ func (g *Generator) generateServerStructs(f *j.File, t *DescInterface) error {
Name: p.Name,
Type: p.Type,
Element: p.Element,
Key: p.Key,
Value: p.Value,
Index: idx,
},
)
Expand Down Expand Up @@ -387,6 +398,11 @@ func (g *Generator) generateServerStructs(f *j.File, t *DescInterface) error {
"json": p.Name + ",omitempty",
})
}
} else if p.Type == "map" {
gr.Id(capitalize(p.Name)).Op("*").Map(g.properType(p.Key)).Add(g.properType(p.Value)).Tag(map[string]string{
"cbor": fmt.Sprintf("%d,keyasint,omitempty", idx),
"json": p.Name + ",omitempty",
})
} else {
gr.Id(capitalize(p.Name)).Op("*").Add(g.properType(p.Type)).Tag(map[string]string{
"cbor": fmt.Sprintf("%d,keyasint,omitempty", idx),
Expand All @@ -412,6 +428,8 @@ func (g *Generator) generateServerStructs(f *j.File, t *DescInterface) error {
Name: p.Name,
Type: p.Type,
Element: p.Element,
Key: p.Key,
Value: p.Value,
Index: idx,
},
)
Expand Down Expand Up @@ -541,6 +559,25 @@ func (g *Generator) readForField(f *j.File, t *DescType, field *DescField) {
)
}

f.Line()
case "map":
f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id("Has" + fname).Params().Bool().Block(
j.Return(j.Id("v").Dot("data").Dot(name).Op("!=").Nil()),
)

f.Line()

f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id(fname).Params().Add(g.mapType(field)).Block(
j.If(j.Id("v").Dot("data").Dot(name).Op("==").Nil()).Block(
j.Return(j.Nil()),
),
j.Return(j.Op("*").Id("v").Dot("data").Dot(name)),
)

f.Line()
default:
if g.ti(field.Type).isInterface {
Expand Down Expand Up @@ -712,6 +749,16 @@ func (g *Generator) writeForField(f *j.File, t *DescType, field *DescField) {
)
}

case "map":
f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id("Set"+fname).Params(
j.Id(pname).Add(g.mapType(field)),
).Block(
j.Id("x").Op(":=").Qual("maps", "Clone").Call(j.Id(pname)),
j.Id("v").Dot("data").Dot(name).Op("=").Op("&").Id("x"),
)

default:
if g.ti(field.Type).isInterface {
f.Func().Params(
Expand Down Expand Up @@ -910,6 +957,12 @@ func (g *Generator) generateCompactStruct(f *j.File, t *DescType) error {
})
}

case "map":
gr.Id(toCamal(field.Name)).Add(g.mapType(field)).Tag(map[string]string{
"cbor": fmt.Sprintf("%d,keyasint,omitempty", field.Index),
"json": toSnake(field.Name) + ",omitempty",
})

case "union":
gr.Id(private(t.Type) + toCamal(field.Name))
default:
Expand Down Expand Up @@ -1111,6 +1164,34 @@ func (g *Generator) generateCompactStruct(f *j.File, t *DescType) error {
)
}
}
case "map":
if t.Readable() {
f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id("Has" + fname).Params().Bool().Block(
j.Return(j.True()),
)

f.Line()

f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id(fname).Params().Add(g.mapType(field)).Block(
j.Return(j.Id("v").Dot("data").Dot(name)),
)

f.Line()
}

if t.Writeable() {
f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id("Set" + fname).Params(
j.Id(pname).Add(g.mapType(field)),
).Block(
j.Id("v").Dot("data").Dot(name).Op("=").Qual("maps", "Clone").Call(j.Id(pname)),
)
}
case "union":
f.Func().Params(
j.Id("v").Op("*").Add(recv),
Expand Down Expand Up @@ -1245,6 +1326,12 @@ func (g *Generator) generateStruct(f *j.File) error {
})
}

case "map":
gr.Id(toCamal(field.Name)).Op("*").Add(g.mapType(field)).Tag(map[string]string{
"cbor": fmt.Sprintf("%d,keyasint,omitempty", field.Index),
"json": toSnake(field.Name) + ",omitempty",
})

case "union":
gr.Id(private(t.Type) + toCamal(field.Name))
default:
Expand Down Expand Up @@ -1464,6 +1551,38 @@ func (g *Generator) generateStruct(f *j.File) error {
)
}
}
case "map":
if t.Readable() {
f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id("Has" + fname).Params().Bool().Block(
j.Return(j.Id("v").Dot("data").Dot(name).Op("!=").Nil()),
)

f.Line()

f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id(fname).Params().Add(g.mapType(field)).Block(
j.If(j.Id("v").Dot("data").Dot(name).Op("==").Nil()).Block(
j.Return(j.Nil()),
),
j.Return(j.Op("*").Id("v").Dot("data").Dot(name)),
)

f.Line()
}

if t.Writeable() {
f.Func().Params(
j.Id("v").Op("*").Add(recv),
).Id("Set"+fname).Params(
j.Id(pname).Add(g.mapType(field)),
).Block(
j.Id("x").Op(":=").Qual("maps", "Clone").Call(j.Id(pname)),
j.Id("v").Dot("data").Dot(name).Op("=").Op("&").Id("x"),
)
}
case "union":
f.Func().Params(
j.Id("v").Op("*").Add(recv),
Expand Down Expand Up @@ -1672,6 +1791,8 @@ func (g *Generator) generateClient(f *j.File, i *DescInterface) error {
Name: p.Name,
Type: p.Type,
Element: p.Element,
Key: p.Key,
Value: p.Value,
Index: 0,
})
}
Expand All @@ -1694,6 +1815,8 @@ func (g *Generator) generateClient(f *j.File, i *DescInterface) error {
} else {
gr.Id(private(p.Name)).Index().Id(p.Element)
}
} else if p.Type == "map" {
gr.Id(private(p.Name)).Map(g.properType(p.Key)).Add(g.properType(p.Value))
} else {
gr.Id(private(p.Name)).Add(g.properType(p.Type))
}
Expand Down Expand Up @@ -1731,6 +1854,9 @@ func (g *Generator) generateClient(f *j.File, i *DescInterface) error {
} else if p.Type == "list" {
gr.Id("x").Op(":=").Qual("slices", "Clone").Call(j.Id(private(p.Name)))
gr.Id("args").Dot("data").Dot(toCamal(p.Name)).Op("=").Op("&").Id("x")
} else if p.Type == "map" {
gr.Id("x").Op(":=").Qual("maps", "Clone").Call(j.Id(private(p.Name)))
gr.Id("args").Dot("data").Dot(toCamal(p.Name)).Op("=").Op("&").Id("x")
} else {
gr.Id("args").Dot("data").Dot(toCamal(p.Name)).Op("=").Op("&").Id(private(p.Name))
}
Expand Down Expand Up @@ -2059,6 +2185,15 @@ func (t *DescType) Validate() error {
}
seen[field.Index] = struct{}{}
}

if field.Type == "list" && field.Element == "" {
return fmt.Errorf("field %q in type %s: list requires element", field.Name, t.Type)
}
if field.Type == "map" {
if field.Key == "" || field.Value == "" {
return fmt.Errorf("field %q in type %s: map requires key and value", field.Name, t.Type)
}
}
}

return nil
Expand Down Expand Up @@ -2138,6 +2273,10 @@ func (t *DescType) CalculateOffsets(usertypes map[string]*DescType) {
field.wordOffset = wordOffset
t.pointers++
wordOffset++
case "map":
field.wordOffset = wordOffset
t.pointers++
wordOffset++
default:
if ut, ok := usertypes[field.Type]; ok {
field.wordOffset = wordOffset
Expand All @@ -2156,6 +2295,8 @@ type DescField struct {

Element string `yaml:"element"`
Union []UnionField `yaml:"union,omitempty"`
Key string `yaml:"key,omitempty"`
Value string `yaml:"value,omitempty"`

dataOffset int
wordOffset int
Expand Down Expand Up @@ -2192,4 +2333,6 @@ type DescParamater struct {
Name string `yaml:"name"`
Type string `yaml:"type"`
Element string `yaml:"element,omitempty"`
Key string `yaml:"key,omitempty"`
Value string `yaml:"value,omitempty"`
}
Loading
Loading