From c10e80c44bba51ddd3afa9e1536c36b9af7b2f13 Mon Sep 17 00:00:00 2001 From: Wangchong Zhou Date: Wed, 3 Oct 2018 16:30:01 -0700 Subject: [PATCH] optimizations Signed-off-by: Wangchong Zhou --- pkg/mapper/fsm/dump.go | 2 +- pkg/mapper/fsm/formatter.go | 12 ++++-- pkg/mapper/fsm/fsm.go | 29 +++++++++---- pkg/mapper/mapper.go | 2 +- pkg/mapper/mapper_benchmark_test.go | 66 ++++++++++------------------- pkg/mapper/mapper_test.go | 5 ++- 6 files changed, 55 insertions(+), 61 deletions(-) diff --git a/pkg/mapper/fsm/dump.go b/pkg/mapper/fsm/dump.go index 1bf4675..d91e2cf 100644 --- a/pkg/mapper/fsm/dump.go +++ b/pkg/mapper/fsm/dump.go @@ -18,7 +18,7 @@ import ( "io" ) -// DumpFSM accepts a io.writer and write the current FSM into dot file format +// DumpFSM accepts a io.writer and write the current FSM into dot file format. func (f *FSM) DumpFSM(w io.Writer) { idx := 0 states := make(map[int]*mappingState) diff --git a/pkg/mapper/fsm/formatter.go b/pkg/mapper/fsm/formatter.go index 6acd91b..8ae0926 100644 --- a/pkg/mapper/fsm/formatter.go +++ b/pkg/mapper/fsm/formatter.go @@ -30,15 +30,17 @@ type TemplateFormatter struct { fmtString string } -func NewTemplateFormatter(valueExpr string, captureCount int) *TemplateFormatter { - matches := templateReplaceCaptureRE.FindAllStringSubmatch(valueExpr, -1) +// NewTemplateFormatter instantialize a TemplateFormatter +// from given template string and the maxium amount of captures. +func NewTemplateFormatter(template string, captureCount int) *TemplateFormatter { + matches := templateReplaceCaptureRE.FindAllStringSubmatch(template, -1) if len(matches) == 0 { // if no regex reference found, keep it as it is - return &TemplateFormatter{captureCount: 0, fmtString: valueExpr} + return &TemplateFormatter{captureCount: 0, fmtString: template} } var indexes []int - valueFormatter := valueExpr + valueFormatter := template for _, match := range matches { idx, err := strconv.Atoi(match[len(match)-1]) if err != nil || idx > captureCount || idx < 1 { @@ -58,6 +60,8 @@ func NewTemplateFormatter(valueExpr string, captureCount int) *TemplateFormatter } } +// Format accepts a list containing captured strings and return the formatted string +// using the template stored in current TemplateFormatter. func (formatter *TemplateFormatter) Format(captures []string) string { if formatter.captureCount == 0 { // no label substitution, keep as it is diff --git a/pkg/mapper/fsm/fsm.go b/pkg/mapper/fsm/fsm.go index 8e764a9..263dba8 100644 --- a/pkg/mapper/fsm/fsm.go +++ b/pkg/mapper/fsm/fsm.go @@ -52,7 +52,6 @@ func NewFSM(metricTypes []string, maxPossibleTransitions int, orderingDisabled b root := &mappingState{} root.transitions = make(map[string]*mappingState, len(metricTypes)) - metricTypes = append(metricTypes, "") for _, field := range metricTypes { state := &mappingState{} (*state).transitions = make(map[string]*mappingState, maxPossibleTransitions) @@ -65,14 +64,17 @@ func NewFSM(metricTypes []string, maxPossibleTransitions int, orderingDisabled b return &fsm } -// AddState adds a state into the existing FSM -func (f *FSM) AddState(match string, name string, matchMetricType string, maxPossibleTransitions int, result interface{}) int { +// AddState adds a mapping rule into the existing FSM. +// The maxPossibleTransitions parameter sets the expected count of transitions left. +// The result parameter sets the generic type to be returned when fsm found a match in GetMapping. +func (f *FSM) AddState(match string, matchMetricType string, maxPossibleTransitions int, result interface{}) int { // first split by "." matchFields := strings.Split(match, ".") // fill into our FSM roots := []*mappingState{} + // first state is the metric type if matchMetricType == "" { - // if metricType not specified, connect the state from all three types + // if metricType not specified, connect the start state from all three types for _, metricType := range f.metricTypes { roots = append(roots, f.root.transitions[string(metricType)]) } @@ -81,11 +83,14 @@ func (f *FSM) AddState(match string, name string, matchMetricType string, maxPos } var captureCount int var finalStates []*mappingState + // iterating over different start state (different metric types) for _, root := range roots { captureCount = 0 + // for each start state, connect from start state to end state for i, field := range matchFields { state, prs := root.transitions[field] if !prs { + // create a state if it's not exist in the fsm state = &mappingState{} (*state).transitions = make(map[string]*mappingState, maxPossibleTransitions) (*state).maxRemainingLength = len(matchFields) - i - 1 @@ -118,7 +123,9 @@ func (f *FSM) AddState(match string, name string, matchMetricType string, maxPos return captureCount } -// GetMapping implements a mapping algorithm for Glob pattern +// GetMapping using the fsm to find matching rules according to given statsdMetric and statsdMetricType. +// If it finds a match, the final state and the captured strings are returned; +// if there's no match found, nil and a empty list will be returned. func (f *FSM) GetMapping(statsdMetric string, statsdMetricType string) (*mappingState, []string) { matchFields := strings.Split(statsdMetric, ".") currentState := f.root.transitions[statsdMetricType] @@ -156,7 +163,7 @@ func (f *FSM) GetMapping(statsdMetric string, statsdMetricType string) (*mapping captureIdx++ } } else if f.BacktrackingNeeded { - // if backtracking is needed, also check for alternative transition + // if backtracking is needed, also check for alternative transition, i.e. * altState, present := currentState.transitions["*"] if !present || fieldsLeft > altState.maxRemainingLength || fieldsLeft < altState.minRemainingLength { } else { @@ -221,10 +228,11 @@ func (f *FSM) GetMapping(statsdMetric string, statsdMetricType string) (*mapping return finalState, captures } -// TestIfNeedBacktracking test if backtrack is needed for current mappings +// TestIfNeedBacktracking tests if backtrack is needed for given list of mappings +// and whether ordering is disabled. func TestIfNeedBacktracking(mappings []string, orderingDisabled bool) bool { backtrackingNeeded := false - // A has * in rules there's other transisitions at the same state + // A has * in rules, but there's other transisitions at the same state, // this makes A the cause of backtracking ruleByLength := make(map[int][]string) ruleREByLength := make(map[int][]*regexp.Regexp) @@ -255,11 +263,14 @@ func TestIfNeedBacktracking(mappings []string, orderingDisabled bool) bool { if re1 == nil || strings.Index(r1, "*") == -1 { continue } - // if a rule is A.B.C.*.E.*, is there a rule A.B.C.D.x.x or A.B.C.*.E.F? (x is any string or *) + // if rule r1 is A.B.C.*.E.*, is there a rule r2 is A.B.C.D.x.x or A.B.C.*.E.F ? (x is any string or *) + // if such r2 exists, then to match r1 we will need backtracking for index := 0; index < len(r1); index++ { if r1[index] != '*' { continue } + // translate the substring of r1 from 0 to the index of current * into regex + // A.B.C.*.E.* will becomes ^A\.B\.C\. and ^A\.B\.C\.\*\.E\. reStr := strings.Replace(r1[:index], ".", "\\.", -1) reStr = strings.Replace(reStr, "*", "\\*", -1) re := regexp.MustCompile("^" + reStr) diff --git a/pkg/mapper/mapper.go b/pkg/mapper/mapper.go index dc5a735..46e90e6 100644 --- a/pkg/mapper/mapper.go +++ b/pkg/mapper/mapper.go @@ -140,7 +140,7 @@ func (m *MetricMapper) InitFromYAMLString(fileContents string) error { return fmt.Errorf("invalid match: %s", currentMapping.Match) } - captureCount := n.FSM.AddState(currentMapping.Match, currentMapping.Name, string(currentMapping.MatchMetricType), + captureCount := n.FSM.AddState(currentMapping.Match, string(currentMapping.MatchMetricType), remainingMappingsCount, currentMapping) currentMapping.nameFormatter = fsm.NewTemplateFormatter(currentMapping.Name, captureCount) diff --git a/pkg/mapper/mapper_benchmark_test.go b/pkg/mapper/mapper_benchmark_test.go index 1511ee2..0388c59 100644 --- a/pkg/mapper/mapper_benchmark_test.go +++ b/pkg/mapper/mapper_benchmark_test.go @@ -109,11 +109,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -174,11 +173,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -246,11 +244,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -311,11 +308,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -339,11 +335,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -368,11 +363,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -395,11 +389,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -423,11 +416,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -450,11 +442,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -478,11 +469,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -516,11 +506,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -555,11 +544,10 @@ mappings: b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -577,11 +565,10 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -601,11 +588,10 @@ mappings:` + duplicateRules(10, ruleTemplateSingleMatchRegex) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -623,11 +609,10 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -645,11 +630,10 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -669,11 +653,10 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchGlob) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -693,11 +676,10 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchRegex) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -717,11 +699,10 @@ mappings:` + duplicateRules(100, ruleTemplateSingleMatchRegex) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -739,11 +720,10 @@ mappings:` + duplicateRules(100, ruleTemplateMultipleMatchGlob) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -763,11 +743,10 @@ mappings:` + duplicateRules(100, ruleTemplateMultipleMatchRegex) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } @@ -787,11 +766,10 @@ mappings:` + duplicateRules(100, ruleTemplateMultipleMatchRegex) b.Fatalf("Config load error: %s %s", config, err) } - var dummyMetricType MetricType b.ResetTimer() for j := 0; j < b.N; j++ { for _, metric := range mappings { - mapper.GetMapping(metric, dummyMetricType) + mapper.GetMapping(metric, MetricTypeCounter) } } } diff --git a/pkg/mapper/mapper_test.go b/pkg/mapper/mapper_test.go index 2a66259..64fb73d 100644 --- a/pkg/mapper/mapper_test.go +++ b/pkg/mapper/mapper_test.go @@ -571,9 +571,10 @@ mappings: t.Fatalf("%d. Expected bad config, but loaded ok: %s", i, scenario.config) } - var dummyMetricType MetricType = "" for metric, mapping := range scenario.mappings { - m, labels, present := mapper.GetMapping(metric, dummyMetricType) + // exporter will call mapper.GetMapping with valid MetricType + // so we also pass a sane MetricType in testing + m, labels, present := mapper.GetMapping(metric, MetricTypeCounter) if present && mapping.name != "" && m.Name != mapping.name { t.Fatalf("%d.%q: Expected name %v, got %v", i, metric, m.Name, mapping.name) }