Adding Sanitizer interface and implementation #70
Conversation
| /** | ||
| * Sanitize returns a sanitized version of the input string value. | ||
| */ | ||
| public interface Sanitize { |
There was a problem hiding this comment.
Please try for a different name. Sanitize next to Sanitizer confused me thoroughly
There was a problem hiding this comment.
The Sanitizer interface matches the uber-go/tally Sanitizer interface.
The Sanitize interface is meant to match the uber-go/tally SanitizeFn interface. Name it SanitizeFn too?
There was a problem hiding this comment.
Please elaborate on what this interface is needed for
|
|
||
| // first check if the provided character is valid | ||
| boolean validCurr = | ||
| ranges.stream().anyMatch(range -> ch >= range.low() && ch <= range.high()) |
There was a problem hiding this comment.
is this operation performance-sensitive? If so, this looks very expensive
There was a problem hiding this comment.
It matches the uber-go/tally sanitzeFn(). Like the go version, it optimizes allocations by avoiding copying if the all characters are valid. Any suggestions on how to optimize this in Java?
|
Any progress on this PR? @ravirajj @inetchitailo ? |
|
@SokolAndrey @andrewmains12 @alexeykudinkin can you help take a look? I am needing this in my project. Thanks so much |
| @@ -313,6 +320,14 @@ public Snapshot snapshot() { | |||
| return snap; | |||
| } | |||
|
|
|||
| private ImmutableMap<String, String> copyAndSanitizeMap(Map<String, String> tags) { | |||
There was a problem hiding this comment.
Let's avoid copying if there are no changes to the map
| * @param name the name string | ||
| * @return the sanitized name | ||
| */ | ||
| String name(String name); |
There was a problem hiding this comment.
Please follow the naming convention -- method name should bear a verb in it
| /** | ||
| * Sanitize returns a sanitized version of the input string value. | ||
| */ | ||
| public interface Sanitize { |
There was a problem hiding this comment.
Please elaborate on what this interface is needed for
|
@alexeykudinkin @ravirajj Hello, thanks so much for this. Thanks |
|
@longquanzheng works for me |
ScopeBulidercan be configured with the standardM3Sanitizer.