Enable uuid randomness pool

[colmsnowplow]

No description provided.

[coveralls]

coverage: 99.466% (+0.002%) from 99.464%
when pulling f982580 on uuid-randomness
into 372c428 on master.

[matus-tomlein]

This might be overthinking it, but is there any risk of this being called multiple times in case users initialize more than one tracker? Would it make sense to have a global flag to indicate if it was already called or not?

[colmsnowplow]

Good point!

This made me think about it and experiment a bit - I committed a workaround where it only gets called when the tracker package first gets booted up.

I haven't got much time to test very thoroughly, but I tested the method as follows:

package mypkg

import "fmt"

func PrintSomething() string {
	fmt.Println("PrintSomething WAS CALLED")

	return "This string is just to avoid go compiler error"
}

var _ = PrintSomething()

func DoSomething() {
	fmt.Println("DoSomething WAS CALLED")
}

package otherpkg

import (
	"fmt"
	mypkg "getOneShard/testpkg"
)

func DoSomethingElse() {
	fmt.Println("DoSomethingElse was called")
	mypkg.DoSomething()
}

func main() {
	mypkg.DoSomething()
	mypkg.DoSomething()
	mypkg.DoSomething()

	otherpkg.DoSomethingElse()

Result:

% go run main.go
PrintSomething WAS CALLED
DoSomething WAS CALLED
DoSomething WAS CALLED
DoSomething WAS CALLED
DoSomethingElse was called
DoSomething WAS CALLED

I don't know if this is an acceptable solution though, or even if I like it very much

[matus-tomlein]

I like this approach, makes sense!

I'll try to do a bit more testing and merge it in.

[matus-tomlein]

I have been reading up on this EnableRandPool() call and have become a bit cautious whether we should actually add it in the tracker.

The reason is that the call changes the global state of the uuid package which may be used by other libraries or by the application itself. I am thinking that it might be better to leave it up to the application to decide whether the function is called and when (I see that people suggest calling it in the main method). If individual libraries in the app call it, there might be potential conflicts within an app. Also there are some reasons why an app would not want to call it – increased memory usage, initialization time, specific security requirements for random number generation. I checked a couple of golang libraries that use the uuid package and they don't seem to call this function.

What do you think? Maybe it would be better to add a recommendation in our docs for apps to call this function instead of forcing it in the tracker?

[colmsnowplow]

I didn't think of that!

I think that's a nice compromise, but also I wouldn't even suggest that a compromise is necessary. In reality UUID conflicts are very rare and the difference here is a ~0.5% improvement.

This may well be a solution without a problem - up to you ofc, but I'm not in favour of adding options that don't have requirements, so I wouldn't be put out by deciding just to shelf this unless we actually hit a reason to do so.

[colmsnowplow]

PS. Thanks for the care and attention!