Drop samples with a null byte in their label values. #211
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Relates to weaveworks/service-conf#507 |
jml
approved these changes
Jan 16, 2017
| } | ||
|
|
||
| if err := util.ValidateSample(sample); err != nil { | ||
| log.Errorf("Error validating sample from user '%s': %v", userID, err) |
There was a problem hiding this comment.
Maybe get the userID inside this if block?
| var validLabelRE = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`) | ||
|
|
||
| // ValidateSample returns an err if the sample is invalid | ||
| func ValidateSample(s *model.Sample) error { |
There was a problem hiding this comment.
Danger of this approach is that we need to remember to call ValidateSample before handling any incoming samples. A more robust solution would be to return a validated sample that has different type to the not-yet-validated sample.
However, not necessary for this PR, and maybe not ever.
|
This has been deployed to prod. I'll go ahead and rebase into master tomorrow. |
tomwilkie
force-pushed
the
drop-null-samples
branch
from
1e872bd to
4d8dae7
Compare
|
Mitigation for #210 |
2b3a1bb Merge pull request #62 from weaveworks/revert-61-test-defaults 8c3883a Revert "Make no-go-get the default, and don't assume -tags netgo" e75c226 Fix bug in GC of firewall rules. e49754e Merge pull request #51 from weaveworks/gc-firewall-rules 191f487 Add flag to enale/disable firewall rules' GC. 567905c Add GC of firewall rules for weave-net-tests to scheduler. 03119e1 Fix typo in GC of firewall rules. bbe3844 Fix regular expression for firewall rules. c5c23ce Pre-change refactoring: splitted gc_project function into smaller methods for better readability. ed5529f GC firewall rules ed8e757 Merge pull request #61 from weaveworks/test-defaults 57856e6 Merge pull request #56 from weaveworks/remove-wcloud dd5f3e6 Add -p flag to test, run test in parallel 62f6f94 Make no-go-get the default, and don't assume -tags netgo 8946588 Merge pull request #60 from weaveworks/2647-gc-weave-net-tests 4085df9 Scheduler now also garbage-collects VMs from weave-net-tests. 4b7d5c6 Merge pull request #59 from weaveworks/57-fix-lint-properly b7f0e69 Merge pull request #58 from weaveworks/fix-lint 794702c Pin version of shfmt ab1b11d Fix lint d1a5e46 Remove wcloud cli tool 81d80f3 Merge pull request #55 from weaveworks/lint-tf 05ad5f2 Review feedback 4c0d046 Use hclfmt to lint terraform. git-subtree-dir: tools git-subtree-split: 2b3a1bbfd122056e51212ee2a53970c9390d2aa7
tomwilkie
force-pushed
the
drop-null-samples
branch
from
96b02ad to
24ec1f7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was causing a prod outage (as we used MustEncode to build the chunk range key, which panics on null bytes).
We'll drop samples for now, and in the future figure out a better way of not dropping the data.