New hashing algorithm for fingerprint processor #15418

rvillablanca · 2020-01-09T14:01:19Z

This adds a the xxHash hashing to the list of available methods in the fingerprint processor.

elasticmachine · 2020-01-09T14:01:21Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2020-01-09T14:01:21Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

rvillablanca · 2020-01-09T14:02:04Z

Ready to be reviewed @ycombinator

libbeat/processors/fingerprint/hash.go

ycombinator · 2020-01-09T18:30:40Z

CI fails like so:

>> check: Checking source code for common problems
processors/fingerprint/hash.go:28:2: cannot find package "github.com/OneOfOne/xxhash" in any of:
	/home/travis/gopath/src/github.com/elastic/beats/vendor/github.com/OneOfOne/xxhash (vendor tree)
	/home/travis/.gimme/versions/go1.13.5.linux.amd64/src/github.com/OneOfOne/xxhash (from $GOROOT)
	/home/travis/gopath/src/github.com/OneOfOne/xxhash (from $GOPATH)

This is because we need to vendor the github.com/OneOfOne/xxhash package as part of this PR.

ycombinator · 2020-01-09T18:30:55Z

jenkins, test this

ycombinator · 2020-01-09T18:32:36Z

Could you also add a line about this enhancement in the CHANGELOG.next.asciidoc file?

rvillablanca · 2020-01-09T18:54:40Z

CI fails like so:

>> check: Checking source code for common problems
processors/fingerprint/hash.go:28:2: cannot find package "github.com/OneOfOne/xxhash" in any of:
	/home/travis/gopath/src/github.com/elastic/beats/vendor/github.com/OneOfOne/xxhash (vendor tree)
	/home/travis/.gimme/versions/go1.13.5.linux.amd64/src/github.com/OneOfOne/xxhash (from $GOROOT)
	/home/travis/gopath/src/github.com/OneOfOne/xxhash (from $GOPATH)

This is because we need to vendor the github.com/OneOfOne/xxhash package as part of this PR.

@ycombinator Do you have vendoring process automated with any make/mage task or should I copy manually the dependency ? I was looking for some task but without success

ycombinator · 2020-01-09T19:06:03Z

Do you have vendoring process automated with any make/mage task or should I copy manually the dependency ?

We use govendor. See https://www.elastic.co/guide/en/beats/devguide/current/beats-contributing.html#dependencies.

rvillablanca · 2020-01-09T19:14:33Z

Ok all sugestions were solved

CHANGELOG.next.asciidoc

libbeat/processors/fingerprint/hash.go

ycombinator · 2020-01-09T19:18:42Z

@rvillablanca Left a couple of minor suggestions but this is looking good otherwise!

ycombinator · 2020-01-09T20:21:20Z

BTW, looks like CI is now failing on an outdated NOTICE.txt file. This should be fixed if you run make notice in the root directory.

rvillablanca · 2020-01-09T20:23:25Z

I will check

ycombinator · 2020-01-09T21:06:25Z

Also, could you add the new hash method (xxhash) to the fingerprint processor docs? See https://github.com/elastic/beats/blob/master/libbeat/processors/fingerprint/docs/fingerprint.asciidoc.

* Using a new constructor function xxhash.NewHash64

ycombinator · 2020-01-10T01:01:25Z

jenkins, test this

ycombinator

LGTM! Thanks for the contribution @rvillablanca and for working through the issue with the hash.Hash interface. I'll merge this once CI looks good.

rvillablanca · 2020-01-10T01:03:11Z

I think we are done with all @ycombinator, any questions do not hesitate to ask me

ycombinator · 2020-01-10T05:37:27Z

NOTICE.txt needs an update again, since the version of the dependency was updated.

rvillablanca · 2020-01-10T12:46:19Z

Ok NOTICE.txt updated

ycombinator · 2020-01-10T13:09:09Z

jenkins, test this

ycombinator · 2020-01-10T14:56:57Z

Travis CI is green. Jenkins CI failures are unrelated. Merging.

* XXHash configuration and test * Removed unused option * Inlining function * Changelog documentation * xxhash library vendored * Updated changelog * Update NOTICE.txt * * Update version github.com/OneOfOne/xxhash@1.2.7 * Using a new constructor function xxhash.NewHash64 * Add xxhash as option for hashing algorithm in fingerprint's docs * Update NOTICE.txt Co-authored-by: Rodrigo Villablanca Vásquez <villa061004@gmail.com>

rvillablanca added 2 commits January 9, 2020 00:52

XXHash configuration and test

2163ff0

Removed unused option

c23c552

ycombinator reviewed Jan 9, 2020

View reviewed changes

libbeat/processors/fingerprint/hash.go Outdated Show resolved Hide resolved

ycombinator self-assigned this Jan 9, 2020

ycombinator added [zube]: In Review needs_backport PR is waiting to be backported to other branches. review Team:Beats v7.6.0 v8.0.0 labels Jan 9, 2020

zube bot added [zube]: In Progress and removed [zube]: In Review labels Jan 9, 2020

ycombinator added [zube]: In Review and removed [zube]: In Progress labels Jan 9, 2020

rvillablanca added 2 commits January 9, 2020 15:42

Inlining function

8c8829c

Changelog documentation

993fd03

xxhash library vendored

9898a72

ycombinator reviewed Jan 9, 2020

View reviewed changes

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved

ycombinator reviewed Jan 9, 2020

View reviewed changes

libbeat/processors/fingerprint/hash.go Outdated Show resolved Hide resolved

Updated changelog

5d8dd66

Update NOTICE.txt

08629b2

rvillablanca added 2 commits January 9, 2020 21:53

* Update version github.com/OneOfOne/xxhash@1.2.7

1600a65

* Using a new constructor function xxhash.NewHash64

Add xxhash as option for hashing algorithm in fingerprint's docs

c4b9e7e

ycombinator approved these changes Jan 10, 2020

View reviewed changes

Update NOTICE.txt

fa793c9

ycombinator merged commit 7b1ac24 into elastic:master Jan 10, 2020

zube bot added [zube]: Done and removed [zube]: In Review labels Jan 10, 2020

rvillablanca deleted the ft/xxhash branch January 10, 2020 14:58

This was referenced Jan 10, 2020

[7.x] New hashing algorithm for fingerprint processor (#15418) #15469

Merged

[Fingerprint processor] Add xxHash as hash option #15080

Closed

andresrc removed the [zube]: Done label Jan 17, 2020

andresrc added the Team:Integrations Label for the Integrations team label Mar 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New hashing algorithm for fingerprint processor #15418

New hashing algorithm for fingerprint processor #15418

rvillablanca commented Jan 9, 2020 •

edited by zube bot

Loading

elasticmachine commented Jan 9, 2020

elasticmachine commented Jan 9, 2020

rvillablanca commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 9, 2020 •

edited

Loading

rvillablanca commented Jan 9, 2020 •

edited

Loading

ycombinator commented Jan 9, 2020

rvillablanca commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 9, 2020

rvillablanca commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 10, 2020

ycombinator left a comment

rvillablanca commented Jan 10, 2020

ycombinator commented Jan 10, 2020

rvillablanca commented Jan 10, 2020

ycombinator commented Jan 10, 2020

ycombinator commented Jan 10, 2020

New hashing algorithm for fingerprint processor #15418

New hashing algorithm for fingerprint processor #15418

Conversation

rvillablanca commented Jan 9, 2020 • edited by zube bot Loading

elasticmachine commented Jan 9, 2020

elasticmachine commented Jan 9, 2020

rvillablanca commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 9, 2020 • edited Loading

rvillablanca commented Jan 9, 2020 • edited Loading

ycombinator commented Jan 9, 2020

rvillablanca commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 9, 2020

rvillablanca commented Jan 9, 2020

ycombinator commented Jan 9, 2020

ycombinator commented Jan 10, 2020

ycombinator left a comment

Choose a reason for hiding this comment

rvillablanca commented Jan 10, 2020

ycombinator commented Jan 10, 2020

rvillablanca commented Jan 10, 2020

ycombinator commented Jan 10, 2020

ycombinator commented Jan 10, 2020

rvillablanca commented Jan 9, 2020 •

edited by zube bot

Loading

ycombinator commented Jan 9, 2020 •

edited

Loading

rvillablanca commented Jan 9, 2020 •

edited

Loading