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.

Sign up for GitHub

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

Jump to bottom

Fingerprinting function misses a separator #74

Closed
beorn7 opened this issue Feb 12, 2015 · 0 comments · Fixed by #87
Closed

Fingerprinting function misses a separator #74

beorn7 opened this issue Feb 12, 2015 · 0 comments · Fixed by #87
Assignees
@beorn7
Labels
bug

Comments

@beorn7
Copy link
Member

beorn7 commented Feb 12, 2015

x{a="bb",b="c} and x{a="b",bb="c"} have the same fingerprint with the current fingerprinting method.
Reason is that we have a separator between label name and label value, but not between whole label pairs.

This is easy to fix, but invalidates all storage. :-(((
@beorn7 beorn7 added the bug label Feb 12, 2015
@beorn7 beorn7 self-assigned this Feb 12, 2015
beorn7 pushed a commit that referenced this issue Feb 12, 2015
Reduce allocations during fingerprinting.
3c0a4c9 
Also, add a test to expose
#74 .

benchmark           old ns/op     new ns/op     delta
BenchmarkMetric     7034          6272          -10.83%

benchmark           old allocs     new allocs     delta
BenchmarkMetric     52             32             -38.46%

benchmark           old bytes     new bytes     delta
BenchmarkMetric     1976          1800          -8.91%
beorn7 pushed a commit that referenced this issue Feb 12, 2015
Reduce allocations during fingerprinting.
a6ef94a 
Also, add a test to expose
#74 .

benchmark           old ns/op     new ns/op     delta
BenchmarkMetric     7034          6272          -10.83%

benchmark           old allocs     new allocs     delta
BenchmarkMetric     52             32             -38.46%

benchmark           old bytes     new bytes     delta
BenchmarkMetric     1976          1800          -8.91%
beorn7 pushed a commit that referenced this issue Feb 12, 2015
Reduce allocations during fingerprinting.
a9bdd32 
Also, add a test to expose
#74 .

benchmark           old ns/op     new ns/op     delta
BenchmarkMetric     7034          6272          -10.83%

benchmark           old allocs     new allocs     delta
BenchmarkMetric     52             32             -38.46%

benchmark           old bytes     new bytes     delta
BenchmarkMetric     1976          1800          -8.91%
@beorn7 beorn7 mentioned this issue Feb 12, 2015
Merged
beorn7 pushed a commit that referenced this issue Mar 3, 2015
Unify fingerprinting aka signature calculation.
ffdfbeb 
This fixes #74 .

IT CHANGES THE FINGERPRINTING AND THEREFORE INVALIDATES EACH AND EVERY
PERSISTED FINGERPRINT (I.E. YOU HAVE TO WIPE THE STORAGE TO USE THIS).

This commit removes the LabelValuesToSignature function as it is used
nowhere (and broken in the way it is implemented right now).

Also, remove one more golint warning.
@beorn7 beorn7 mentioned this issue Mar 3, 2015
Merged
beorn7 pushed a commit that referenced this issue Mar 3, 2015
Unify fingerprinting aka signature calculation.
12d396a 
This fixes #74 .

IT CHANGES THE FINGERPRINTING AND THEREFORE INVALIDATES EACH AND EVERY
PERSISTED FINGERPRINT (I.E. YOU HAVE TO WIPE THE STORAGE TO USE THIS).

This commit removes the LabelValuesToSignature function as it is used
nowhere (and broken in the way it is implemented right now).

Also, remove one more golint warning.
beorn7 pushed a commit that referenced this issue Mar 3, 2015
Unify fingerprinting aka signature calculation.
af21d45 
This fixes #74 .

IT CHANGES THE FINGERPRINTING AND THEREFORE INVALIDATES EACH AND EVERY
PERSISTED FINGERPRINT (I.E. YOU HAVE TO WIPE THE STORAGE TO USE THIS).

This commit removes the LabelValuesToSignature function as it is used
nowhere (and broken in the way it is implemented right now).

Also, remove one more golint warning.
@beorn7 beorn7 closed this as completed in #87 Mar 3, 2015
Cori1109 added a commit to Cori1109/client_golang that referenced this issue Jan 9, 2023
@Cori1109
Reduce allocations during fingerprinting.
40283a5 
Also, add a test to expose
prometheus/client_golang#74 .

benchmark           old ns/op     new ns/op     delta
BenchmarkMetric     7034          6272          -10.83%

benchmark           old allocs     new allocs     delta
BenchmarkMetric     52             32             -38.46%

benchmark           old bytes     new bytes     delta
BenchmarkMetric     1976          1800          -8.91%
Cori1109 added a commit to Cori1109/client_golang that referenced this issue Jan 9, 2023
@Cori1109
Unify fingerprinting aka signature calculation.
74fefd4 
This fixes prometheus/client_golang#74 .

IT CHANGES THE FINGERPRINTING AND THEREFORE INVALIDATES EACH AND EVERY
PERSISTED FINGERPRINT (I.E. YOU HAVE TO WIPE THE STORAGE TO USE THIS).

This commit removes the LabelValuesToSignature function as it is used
nowhere (and broken in the way it is implemented right now).

Also, remove one more golint warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@beorn7 beorn7

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Unify fingerprinting aka signature calculation. prometheus/client_golang
1 participant
@beorn7