Align usage of hashing functions

[jpkrohling]

Some parts of the code base are using custom-made hashing functions, others are using hash/fnv, while others are using other things. I believe we should unify that as much as possible, hopefully with hash/fnv as it's very performant and good enough for non-cryptographic usage.

opentelemetry-collector-contrib/processor/probabilisticsamplerprocessor/probabilisticsampler.go

Line 156 in 6e2b67b

func hash(key []byte, seed uint32) (hash uint32) {

opentelemetry-collector-contrib/processor/tailsamplingprocessor/internal/sampling/probabilistic.go

Line 18 in bff12af

"hash/fnv"

opentelemetry-collector-contrib/exporter/loadbalancingexporter/consistent_hashing.go

Line 18 in a811dbb

"hash/crc32"

opentelemetry-collector-contrib/processor/groupbytraceprocessor/event.go

Line 21 in a811dbb

"hash/maphash"

[VineethReddy02]

Looking into this! :)

[jpkrohling]

There might be cases where it's OK to deviate from the standard, like for #5981. As @ydrozhdzhal mentioned:

I checked the fnv and it doesn't feet my needs, because it works with byte arrays and hashstructure can hash entire structure(you can find this in MR),

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[jpkrohling]

@VineethReddy02, are you still interested in this?

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[jpkrohling]

This is again up for grabs.

[oertl]

The project SMHasher compares various non-cryptographic hash functions in terms of speed and hash quality. The fastest hash functions without quality problems are listed here. FNV is not one of them.

[jmichalek132]

Will try to find time to submit PR for this over the weekend.

[jpkrohling]

@oertl, right now, we have some usage of fnv and I'd align on that first, given there should be only a few deviating usages. I am open to discussing changing the hashing algorithm to something else. Would you please open an issue with your arguments?

[oertl]

@jpkrohling I am fine with them being aligned first. I just wanted to point out that there are better hash functions available in the meantime. The alignment may be the ideal moment to switch to a better hash function.

By the way, Go uses a variant of wyhash as its default hash function (see https://go.dev/src/runtime/hash64.go), which SMHasher lists among the recommended hash functions.

[jpkrohling]

@oertl, I'd happily review a PR aligning the code base with a new hash function, especially if it's well-documented that it's a better fit for our use-cases (which seems to be the case!).

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[fatsheep9146]

@jmichalek132 are you still working on this?

[jmichalek132]

Actually didn't get to it at all, unfortunately.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• issue: Github issue template generation code needs this to generate the corresponding labels.

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[amritanshu-pandey]

I am keen to give it a try.

[atoulme]

assigned to you @amritanshu-pandey