-
Notifications
You must be signed in to change notification settings - Fork 5.5k
stats: use RE2 and a better pattern to accelerate a single stats tag-extraction RE #8831
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.
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
Changes from 22 commits
6ffa3ee
fa3573a
42ac3d0
e9b9c88
d692af2
fd34b44
e278b2f
7e416e8
74abaa4
a4f297c
c0a8983
eba2624
edf2bb2
626a092
b1bfbdc
46a3900
0fa1ca2
fad62de
1976786
8c79d6c
5c801a6
c445533
00a0ba1
5cf3b48
39783c4
b5bac7e
f66d732
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
|
|
||
| #include "envoy/common/exception.h" | ||
|
|
||
| #include "common/common/assert.h" | ||
| #include "common/common/fmt.h" | ||
| #include "common/common/perf_annotation.h" | ||
| #include "common/common/regex.h" | ||
|
|
@@ -23,12 +24,11 @@ bool regexStartsWithDot(absl::string_view regex) { | |
|
|
||
| } // namespace | ||
|
|
||
| TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex, | ||
| const std::string& substr) | ||
| : name_(name), prefix_(std::string(extractRegexPrefix(regex))), substr_(substr), | ||
| regex_(Regex::Utility::parseStdRegex(regex)) {} | ||
| TagExtractorImplBase::TagExtractorImplBase(const std::string& name, const std::string& regex, | ||
| const std::string& substr) | ||
| : name_(name), prefix_(std::string(extractRegexPrefix(regex))), substr_(substr) {} | ||
|
|
||
| std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { | ||
| std::string TagExtractorImplBase::extractRegexPrefix(absl::string_view regex) { | ||
| std::string prefix; | ||
| if (absl::StartsWith(regex, "^")) { | ||
| for (absl::string_view::size_type i = 1; i < regex.size(); ++i) { | ||
|
|
@@ -47,10 +47,10 @@ std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { | |
| return prefix; | ||
| } | ||
|
|
||
| TagExtractorPtr TagExtractorImpl::createTagExtractor(const std::string& name, | ||
| const std::string& regex, | ||
| const std::string& substr) { | ||
|
|
||
| TagExtractorPtr TagExtractorImplBase::createTagExtractor(const std::string& name, | ||
| const std::string& regex, | ||
| const std::string& substr, | ||
| Regex::Type re_type) { | ||
| if (name.empty()) { | ||
| throw EnvoyException("tag_name cannot be empty"); | ||
| } | ||
|
|
@@ -59,19 +59,29 @@ TagExtractorPtr TagExtractorImpl::createTagExtractor(const std::string& name, | |
| throw EnvoyException(fmt::format( | ||
| "No regex specified for tag specifier and no default regex for name: '{}'", name)); | ||
| } | ||
| return TagExtractorPtr{new TagExtractorImpl(name, regex, substr)}; | ||
| switch (re_type) { | ||
| case Regex::Type::Re2: | ||
| return std::make_unique<TagExtractorRe2Impl>(name, regex, substr); | ||
| case Regex::Type::StdRegex: | ||
| return std::make_unique<TagExtractorStdRegexImpl>(name, regex, substr); | ||
| } | ||
| NOT_REACHED_GCOVR_EXCL_LINE; | ||
| } | ||
|
|
||
| bool TagExtractorImpl::substrMismatch(absl::string_view stat_name) const { | ||
| bool TagExtractorImplBase::substrMismatch(absl::string_view stat_name) const { | ||
| return !substr_.empty() && stat_name.find(substr_) == absl::string_view::npos; | ||
| } | ||
|
|
||
| bool TagExtractorImpl::extractTag(absl::string_view stat_name, TagVector& tags, | ||
| IntervalSet<size_t>& remove_characters) const { | ||
| TagExtractorStdRegexImpl::TagExtractorStdRegexImpl(const std::string& name, const std::string regex, | ||
| const std::string& substr) | ||
| : TagExtractorImplBase(name, regex, substr), regex_(Regex::Utility::parseStdRegex(regex)) {} | ||
|
|
||
| bool TagExtractorStdRegexImpl::extractTag(absl::string_view stat_name, std::vector<Tag>& tags, | ||
| IntervalSet<size_t>& remove_characters) const { | ||
| PERF_OPERATION(perf); | ||
|
|
||
| if (substrMismatch(stat_name)) { | ||
| PERF_RECORD(perf, "re-skip-substr", name_); | ||
| PERF_RECORD(perf, "re-skip", name_); | ||
| return false; | ||
| } | ||
|
|
||
|
|
@@ -105,5 +115,50 @@ bool TagExtractorImpl::extractTag(absl::string_view stat_name, TagVector& tags, | |
| return false; | ||
| } | ||
|
|
||
| TagExtractorRe2Impl::TagExtractorRe2Impl(const std::string& name, const std::string regex, | ||
| const std::string& substr) | ||
| : TagExtractorImplBase(name, regex, substr), regex_(regex) {} | ||
|
|
||
| bool TagExtractorRe2Impl::extractTag(absl::string_view stat_name, std::vector<Tag>& tags, | ||
| IntervalSet<size_t>& remove_characters) const { | ||
| PERF_OPERATION(perf); | ||
|
|
||
| if (substrMismatch(stat_name)) { | ||
| PERF_RECORD(perf, "re2-skip", name_); | ||
| return false; | ||
| } | ||
|
|
||
| // remove_subexpr is the first submatch. It represents the portion of the string to be removed. | ||
| re2::StringPiece remove_subexpr, value_subexpr; | ||
|
|
||
| // The regex must match and contain one or more subexpressions (all after the first are ignored). | ||
| if (re2::RE2::FullMatch(re2::StringPiece(stat_name.data(), stat_name.size()), regex_, | ||
| &remove_subexpr, &value_subexpr) && | ||
| !remove_subexpr.empty()) { | ||
|
|
||
| // value_subexpr is the optional second submatch. It is usually inside the first submatch | ||
| // (remove_subexpr) to allow the expression to strip off extra characters that should be removed | ||
| // from the string but also not necessary in the tag value ("." for example). If there is no | ||
| // second submatch, then the value_subexpr is the same as the remove_subexpr. | ||
| if (value_subexpr.empty()) { | ||
| value_subexpr = remove_subexpr; | ||
| } | ||
|
|
||
| tags.emplace_back(); | ||
| Tag& tag = tags.back(); | ||
| tag.name_ = name_; | ||
| tag.value_ = std::string(value_subexpr); | ||
|
|
||
| // Determines which characters to remove from stat_name to elide remove_subexpr. | ||
| std::string::size_type start = remove_subexpr.data() - stat_name.data(); | ||
| std::string::size_type end = remove_subexpr.data() + remove_subexpr.size() - stat_name.data(); | ||
| remove_characters.insert(start, end); | ||
| PERF_RECORD(perf, "re2-match", name_); | ||
| return true; | ||
|
Comment on lines
+132
to
+158
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you consider options for avoiding duplicating a lot of the logic/comments between here and the std::regex code?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I messed around a bit and was only really able to profitably factor out the code to add a tag to the tags array. There were enough subtle difference in all the other parallels that it didn't seem worth it to refactor. LMK if you have other ideas. |
||
| } | ||
| PERF_RECORD(perf, "re2-miss", name_); | ||
| return false; | ||
| } | ||
|
|
||
| } // namespace Stats | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,12 +6,15 @@ | |
|
|
||
| #include "envoy/stats/tag_extractor.h" | ||
|
|
||
| #include "common/common/regex.h" | ||
|
|
||
| #include "absl/strings/string_view.h" | ||
| #include "re2/re2.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Stats { | ||
|
|
||
| class TagExtractorImpl : public TagExtractor { | ||
| class TagExtractorImplBase : public TagExtractor { | ||
| public: | ||
| /** | ||
| * Creates a tag extractor from the regex provided. name and regex must be non-empty. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update this comment with new parameter
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done; good catch. |
||
|
|
@@ -23,13 +26,12 @@ class TagExtractorImpl : public TagExtractor { | |
| * @return TagExtractorPtr newly constructed TagExtractor. | ||
| */ | ||
| static TagExtractorPtr createTagExtractor(const std::string& name, const std::string& regex, | ||
| const std::string& substr = ""); | ||
| const std::string& substr = "", | ||
| Regex::Type re_type = Regex::Type::StdRegex); | ||
|
|
||
| TagExtractorImpl(const std::string& name, const std::string& regex, | ||
| const std::string& substr = ""); | ||
| TagExtractorImplBase(const std::string& name, const std::string& regex, | ||
| const std::string& substr = ""); | ||
| std::string name() const override { return name_; } | ||
| bool extractTag(absl::string_view tag_extracted_name, TagVector& tags, | ||
| IntervalSet<size_t>& remove_characters) const override; | ||
| absl::string_view prefixToken() const override { return prefix_; } | ||
|
|
||
| /** | ||
|
|
@@ -39,7 +41,7 @@ class TagExtractorImpl : public TagExtractor { | |
| */ | ||
| bool substrMismatch(absl::string_view stat_name) const; | ||
|
|
||
| private: | ||
| protected: | ||
| /** | ||
| * Examines a regex string, looking for the pattern: ^alphanumerics_with_underscores\. | ||
| * Returns "alphanumerics_with_underscores" if that pattern is found, empty-string otherwise. | ||
|
|
@@ -50,8 +52,31 @@ class TagExtractorImpl : public TagExtractor { | |
| const std::string name_; | ||
| const std::string prefix_; | ||
| const std::string substr_; | ||
| }; | ||
|
|
||
| class TagExtractorStdRegexImpl : public TagExtractorImplBase { | ||
| public: | ||
| TagExtractorStdRegexImpl(const std::string& name, const std::string regex, | ||
|
jmarantz marked this conversation as resolved.
Outdated
|
||
| const std::string& substr = ""); | ||
|
|
||
| bool extractTag(absl::string_view tag_extracted_name, std::vector<Tag>& tags, | ||
| IntervalSet<size_t>& remove_characters) const override; | ||
|
|
||
| private: | ||
| const std::regex regex_; | ||
| }; | ||
|
|
||
| class TagExtractorRe2Impl : public TagExtractorImplBase { | ||
| public: | ||
| TagExtractorRe2Impl(const std::string& name, const std::string regex, | ||
| const std::string& substr = ""); | ||
|
|
||
| bool extractTag(absl::string_view tag_extracted_name, std::vector<Tag>& tags, | ||
| IntervalSet<size_t>& remove_characters) const override; | ||
|
|
||
| private: | ||
| re2::RE2 regex_; | ||
|
jmarantz marked this conversation as resolved.
Outdated
|
||
| }; | ||
|
|
||
| } // namespace Stats | ||
| } // namespace Envoy | ||
Uh oh!
There was an error while loading. Please reload this page.