-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Switch to different UUID lib due to non-random generated UUIDs #8458
Conversation
|
Seems like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM just a minor things about the changelog.
Thanks for propagating this error!
CHANGELOG.asciidoc
Outdated
@@ -45,6 +45,7 @@ https://github.com/elastic/beats/compare/v6.4.0...master[Check the HEAD diff] | |||
- Add backoff support to x-pack monitoring outputs. {issue}7966[7966] | |||
- Fix a race condition with the `add_host_metadata` and the event serialization. {pull}8223[8223] | |||
- Enforce that data used by k8s or docker doesn't use any reference. {pull}8240[8240] | |||
- Switch to different UUID lib: github.com/gofrs/uuid. {pull}8485[8485] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we insert why we switch probably adding a word about collision in the old library.
5bf4cbe
to
5ec6d4e
Compare
Updated changelog entry and rebased on top of #8469 |
Make check fails on CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking it to this issue. Only minor comments from me.
@@ -35,7 +36,11 @@ func init() { | |||
beatMetrics = monitoring.Default.NewRegistry("beat") | |||
monitoring.NewFunc(beatMetrics, "info", reportInfo, monitoring.Report) | |||
|
|||
ephemeralID = uuid.NewV4() | |||
var err error | |||
ephemeralID, err = uuid.NewV4() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the impact to monitoring if the ephemeral ID is Nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The beat won't show up in some visualizations.
key = uuid.NewV4() | ||
key, err := uuid.NewV4() | ||
if err != nil { | ||
return uuid.UUID{}, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use uuid.Nil
here? It's the same but slightly more clear.
I just merged #8559, which adds a new use of this lib in |
* no longer maintained * contains critical bug which leads to generating similar and same UUIDs
This should be ready for the next round of review. |
…ic#8458) The previously used `github.com/satori/go-uuid` lib has a critical bug and it is no longer maintained. The community moved to an active fork of this existing lib named `github.com/gofrs/uuid`. I haven't seen other UUID libs worth mentioning apart from this. Changes compared to the previous dependency: * `uuid.NewV4` returns an error if the function call failed * `uuid.Equal` is deprecated and removed. The Way is to use `==` instead. Closes elastic#8077 (cherry picked from commit 554ddcd)
#8640) The previously used `github.com/satori/go-uuid` lib has a critical bug and it is no longer maintained. The community moved to an active fork of this existing lib named `github.com/gofrs/uuid`. I haven't seen other UUID libs worth mentioning apart from this. Changes compared to the previous dependency: * `uuid.NewV4` returns an error if the function call failed * `uuid.Equal` is deprecated and removed. The Way is to use `==` instead. Closes #8077 (cherry picked from commit 554ddcd)
The previously used
github.com/satori/go-uuid
lib has a critical bug and it is no longer maintained. The community moved to an active fork of this existing lib namedgithub.com/gofrs/uuid
. I haven't seen other UUID libs worth mentioning apart from this.Changes compared to the previous dependency:
uuid.NewV4
returns an error if the function call faileduuid.Equal
is deprecated and removed. The Way is to use==
instead.TODO
github.com/elastic/go-ucfg
Closes #8077