output: flush complex nested values, not just map[string]string. #68
Conversation
send the full nested complex values when flushing to output plugins
in `Message.Record` as a `map[string]interface{}` instead of as a
simple `map[string]string`. This will allow receiving float64 and
other simple types as well as nested records with key/value pairs
and arrays.
tests have also been added for this new functionality.
Signed-off-by: Phillip Whelan <[email protected]>
pwhelan
requested review from
cosmo0920,
nicolasparada,
niedbalski,
lwatiker and
jasmingacic
cshared.go
Outdated
| func decodeEntry(tag string, entry []any) (*Message, error) { | ||
| var t time.Time | ||
|
|
||
| slice := reflect.ValueOf(entry) |
There was a problem hiding this comment.
I know you just moved the code around and changed downbelow the record type, but is there a need to use reflection here? I don't think so...
if len(entry) != 2 { ... }
var t time.Time
switch t := entry[0].(type) { ... }
data, ok := entry[1](map[string]any])
...
There was a problem hiding this comment.
This was needed to create slice and obtain the types of the first element to distinguish types of the time.
Fluent Bit sometimes uses only BigEndianTime but sometimes a bit of different situations.
When using timestamp with metadata, we need to detect those usages.
There was a problem hiding this comment.
But the msgpack decoder will already handle the time and use bigEndianTime type. I think we can just go with type assertion instead of reflection.
There was a problem hiding this comment.
Ah, it was not effective when I tested. Because once decoded for metadata part which includes timestamp additional metadata, it's just represented with wrapped up as an untyped object not array.
There was a problem hiding this comment.
I modified the code in decodeEntry to forego the use of reflect.
cshared_test.go
Outdated
| if count <= 0 { | ||
| return fmt.Errorf("no records flushed") | ||
| } | ||
| fmt.Print(printout.String()) |
There was a problem hiding this comment.
Shouldn't you be doing some assertions here instead of just printing the result?
There was a problem hiding this comment.
will add assertions.
There was a problem hiding this comment.
I've added assertions, one of them is pretty weird though:
foo, ok := record["foo"]
if !ok {
t.Errorf("unable to access value for foo")
return nil
}
// this is a little bit disconcerting. does the msgpack
// decoder default to using []uint8 for strings?
if foostr, okfoo := foo.([]uint8); !okfoo {
t.Errorf("unable to get value for foo")
return nil
} else if string(foostr) != "bar" {
t.Errorf("invalid value for foo")
return nil
}Apparently the decoder is favouring []uint8 over string (which is technically mostly accurate for C strings... but....).
cshared.go
Outdated
| func decodeEntry(tag string, entry []any) (*Message, error) { | ||
| var t time.Time | ||
|
|
||
| slice := reflect.ValueOf(entry) |
There was a problem hiding this comment.
This was needed to create slice and obtain the types of the first element to distinguish types of the time.
Fluent Bit sometimes uses only BigEndianTime but sometimes a bit of different situations.
When using timestamp with metadata, we need to detect those usages.
Signed-off-by: Phillip Whelan <[email protected]>
Signed-off-by: Phillip Whelan <[email protected]>
|
@nicolasparada Any insights? It's OK for you? |
* replace msgpack library * todo: fix time printing * remove time test * ci: enable cgo * ci: C deps * sudo * ci: install cmetrics * include tests from pr #68 * fix test * test utl
send the full nested complex values when flushing to output plugins in
Message.Recordas amap[string]interface{}instead of as a simplemap[string]string. This will allow receiving float64 and other simple types as well as nested records with key/value pairs and arrays.tests have also been added for this new functionality.