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

Tweaks #69

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Tweaks #69

wants to merge 7 commits into from

Conversation

randomizedcoder
Copy link

@cosmo0920 Thanks for the work on fluent-bit-go

I made a little branch with a couple of other minor tweaks.

  1. Dump go version, and update the dependency
    go 1.20
    github.com/ugorji/go/codec

  2. Verify timestamp without reflection
    To verify the timestamp is being decoded correctly added a extractTimeStamp() function to extract the time without using reflection.

  3. More test data
    Also tried to add another test input, and this actually fails. I believe the input data is valid, because I extracted it from my running version of fluentbit. Have I just made a coding error, or has it found a real issue? I added some log lines, and the input data looks ok-ish.

`das@t:~/Downloads/fluent-bit-go/output$ ~/Downloads/msgpack-inspect ./testdata/data | more

  • format: "array32"
    header: "0xdd"
    length: 2
    children:
    • format: "array32"
      header: "0xdd"
      length: 2
      children:
      • format: "fixext8"
        header: "0xd7"
        exttype: 0
        length: 8
        data: "0x64b579fd24b76a00"
      • format: "fixmap"
        header: "0x80"
        length: 0
        children: []
    • format: "map16"
      header: "0xde"
      length: 43
      children:
      • key:
        format: "fixstr"
        header: "0xa4"
        length: 4`

Thanks in advance

@@ -1,5 +1,5 @@
module github.com/fluent/fluent-bit-go

go 1.14
go 1.20
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't bump the Go version until it's necessary.

Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

Please follow DCO. We don’t merge PR without following DCO procedure.

Could you add your test data as binary arrays instead of adding binary files? Adding binary files leads to decrease maintainability for test cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants