Fix/apimonitoring (#1624)

* add benchmark Signed-off-by: Sandor Szücs <[email protected]> * add map with sync.Mutex % go test -bench=Benchmark_CreateFilter_FullConfigSingleApiNakadi -benchmem ./filters/apiusagemonitoring -count 5 G -vE 'ERRO|INFO|level=info|level=error' goos: linux goarch: amd64 pkg: github.com/zalando/skipper/filters/apiusagemonitoring Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2590 527939 ns/op 55714 B/op 1379 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2474 447375 ns/op 55688 B/op 1379 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2511 525478 ns/op 55711 B/op 1379 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2509 449144 ns/op 55751 B/op 1379 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2293 451283 ns/op 55655 B/op 1379 allocs/op PASS ok github.com/zalando/skipper/filters/apiusagemonitoring 8.266s Signed-off-by: Sandor Szücs <[email protected]> * cache based on sync.Map % go test -bench=Benchmark_CreateFilter_FullConfigSingleApiNakadi -benchmem ./filters/apiusagemonitoring -count 5 G -vE 'ERRO|INFO|level=info|level=error' goos: linux goarch: amd64 pkg: github.com/zalando/skipper/filters/apiusagemonitoring Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2534 446211 ns/op 55806 B/op 1381 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2520 447876 ns/op 55691 B/op 1381 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2373 447189 ns/op 55745 B/op 1381 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2473 438565 ns/op 55700 B/op 1381 allocs/op Benchmark_CreateFilter_FullConfigSingleApiNakadi-4 2491 449118 ns/op 55780 B/op 1381 allocs/op PASS ok github.com/zalando/skipper/filters/apiusagemonitoring 6.833s Signed-off-by: Sandor Szücs <[email protected]> * change as commented Signed-off-by: Sandor Szücs <[email protected]> * no need to store if we have a successful load Signed-off-by: Sandor Szücs <[email protected]> * remove as commented Signed-off-by: Sandor Szücs <[email protected]>
zalando · Dec 1, 2020 · a427211 · a427211
1 parent f67229f
commit a427211
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 6 deletions.
diff --git a/filters/apiusagemonitoring/spec.go b/filters/apiusagemonitoring/spec.go
@@ -5,6 +5,7 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/sirupsen/logrus"
 
@@ -20,9 +21,23 @@ const (
 )
 
 var (
-	log = logrus.WithField("filter", Name)
+	log      = logrus.WithField("filter", Name)
+	regCache = sync.Map{}
 )
 
+func loadOrCompileRegex(pattern string) (*regexp.Regexp, error) {
+	var err error
+	var reg *regexp.Regexp
+	regI, ok := regCache.Load(pattern)
+	if !ok {
+		reg, err = regexp.Compile(pattern)
+		regCache.Store(pattern, reg)
+	} else {
+		reg = regI.(*regexp.Regexp)
+	}
+	return reg, err
+}
+
 // NewApiUsageMonitoring creates a new instance of the API Monitoring filter
 // specification (its factory).
 func NewApiUsageMonitoring(
@@ -52,8 +67,8 @@ func NewApiUsageMonitoring(
 			clientKeyList = append(clientKeyList, strippedKey)
 		}
 	}
-	// compile realms regex
-	realmsTrackingMatcher, err := regexp.Compile(realmsTrackingPattern)
+
+	realmsTrackingMatcher, err := loadOrCompileRegex(realmsTrackingPattern)
 	if err != nil {
 		log.Errorf(
 			"api-usage-monitoring-realmsTrackingPattern-tracking-pattern (global config) ignored: error compiling regular expression %q: %v",
@@ -231,14 +246,17 @@ func (s *apiUsageMonitoringSpec) buildPathInfoListFromConfiguration(apis []*apiC
 			}
 			existingPathPattern[pathPattern] = info
 
-			// Compile the regular expression
-			pathPatternMatcher, err := regexp.Compile(pathPattern)
+			pathPatternMatcher, err := loadOrCompileRegex(pathPattern)
 			if err != nil {
 				log.Errorf(
 					"args[%d].path_templates[%d] ignored: error compiling regular expression %q for path %q: %v",
 					apiIndex, templateIndex, pathPattern, info.PathTemplate, err)
 				continue
 			}
+			if pathPatternMatcher == nil {
+				continue
+			}
+
 			info.Matcher = pathPatternMatcher
 
 			// Add valid entry to the results
@@ -272,13 +290,16 @@ func (s *apiUsageMonitoringSpec) buildClientTrackingInfo(apiIndex int, api *apiC
 		return nil
 	}
 
-	clientTrackingMatcher, err := regexp.Compile(api.ClientTrackingPattern)
+	clientTrackingMatcher, err := loadOrCompileRegex(api.ClientTrackingPattern)
 	if err != nil {
 		log.Errorf(
 			"args[%d].client_tracking_pattern ignored (no client tracking): error compiling regular expression %q: %v",
 			apiIndex, api.ClientTrackingPattern, err)
 		return nil
 	}
+	if clientTrackingMatcher == nil {
+		return nil
+	}
 
 	return &clientTrackingInfo{
 		ClientTrackingMatcher: clientTrackingMatcher,

diff --git a/filters/apiusagemonitoring/spec_test.go b/filters/apiusagemonitoring/spec_test.go
@@ -584,3 +584,62 @@ func Test_CreatePathPattern(t *testing.T) {
 		assert.Equalf(t, path.expectedPathPattern, actualPathPattern, message)
 	}
 }
+
+func Benchmark_CreateFilter_FullConfigSingleApiNakadi(b *testing.B) {
+	// Includes paths:
+	//   - normal (no variable part)
+	//   - with {name} variable paths
+	//   - with :name variable paths
+	//   - with/without head/trailing slash
+	spec := NewApiUsageMonitoring(
+		true,
+		"https://identity.zalando.com/realm",
+		"https://identity.zalando.com/managed-id,sub",
+		"services[.].*")
+
+	args := []interface{}{`{
+		"application_id": "my_app",
+		"tag": "staging",
+		"api_id": "my_api",
+		"path_templates": [
+			"/event-types",
+			"/event-types/{name}",
+			"/event-types/{name}/cursor-distances",
+			"/event-types/{name}/cursors-lag",
+			"/event-types/{name}/deleted-events",
+			"/event-types/{name}/events",
+			"/event-types/{name}/partition-count",
+			"/event-types/{name}/partitions",
+			"/event-types/{name}/partitions/{partition}",
+			"/event-types/{name}/schemas",
+			"/event-types/{name}/schemas/{version}",
+			"/event-types/{name}/shifted-cursors",
+			"/event-types/{name}/timelines",
+			"/metrics",
+			"/registry/enrichment-strategies",
+			"/registry/partition-strategies",
+			"/settings/admins",
+			"/settings/blacklist",
+			"/settings/blacklist/{blacklist_type}/{name}",
+			"/settings/features",
+			"/storages",
+			"/storages/default/{id}",
+			"/storages/{id}",
+			"/subscriptions",
+			"/subscriptions/{subscription_id}",
+			"/subscriptions/{subscription_id}/cursors",
+			"/subscriptions/{subscription_id}/events",
+			"/subscriptions/{subscription_id}/stats"
+		]
+	}`}
+	for n := 0; n < b.N; n++ {
+		f, err := spec.CreateFilter(args)
+		if err != nil {
+			b.Fatalf("Failed to run CreateFilter: %v", err)
+		}
+		filter, ok := f.(*apiUsageMonitoringFilter)
+		if !ok || filter == nil {
+			b.Fatal("Failed to convert filter")
+		}
+	}
+}