Circuit Breaking Per Method (#773)

* Check for unnecessary file changes * Fix circuit breaker method name for http client
uber · Jul 28, 2021 · d83f2df · d83f2df
1 parent 9232e35
commit d83f2df
Show file tree

Hide file tree

Showing 15 changed files with 434 additions and 402 deletions.
diff --git a/codegen/template_bundle/template_files.go b/codegen/template_bundle/template_files.go
diff --git a/codegen/templates/grpc_client.tmpl b/codegen/templates/grpc_client.tmpl
@@ -69,6 +69,18 @@ func {{$exportName}}(deps *module.Dependencies) Client {
 			{{- end -}}
 		{{- end}}
 	}
+
+	// circuitBreakerDisabled sets whether circuit-breaker should be disabled
+	circuitBreakerDisabled := false
+	if deps.Default.Config.ContainsKey("clients.{{$clientID}}.circuitBreakerDisabled") {
+		circuitBreakerDisabled = deps.Default.Config.MustGetBoolean("clients.{{$clientID}}.circuitBreakerDisabled")
+	}
+	if !circuitBreakerDisabled {
+		for methodKey := range methodNames {
+			configureCircuitBreaker(deps, timeoutInMS, methodNames[methodKey])
+		}
+	}
+
 	return &{{$clientName}}{
 		{{range $i, $s := $services -}}
 		{{camel $s.Name}}Client: gen.New{{pascal $s.Name}}YARPCClient(oc),
@@ -81,21 +93,13 @@ func {{$exportName}}(deps *module.Dependencies) Client {
 		"{{$clientID}}",
 		routingKey,
 		requestUUIDHeaderKey,
-		!configureCircuitBreaker(deps, timeoutInMS),
+		circuitBreakerDisabled,
 		timeoutInMS,
 		),
 	}
 }
 
-func configureCircuitBreaker(deps *module.Dependencies, timeoutVal int) bool {
-	// circuitBreakerDisabled sets whether circuit-breaker should be disabled
-	circuitBreakerDisabled := false
-	if deps.Default.Config.ContainsKey("clients.{{$clientID}}.circuitBreakerDisabled") {
-		circuitBreakerDisabled = deps.Default.Config.MustGetBoolean("clients.{{$clientID}}.circuitBreakerDisabled")
-	}
-	if circuitBreakerDisabled {
-		return false
-	}
+func configureCircuitBreaker(deps *module.Dependencies, timeoutVal int, method string) {
 	// sleepWindowInMilliseconds sets the amount of time, after tripping the circuit,
 	// to reject requests before allowing attempts again to determine if the circuit should again be closed
 	sleepWindowInMilliseconds := 5000
@@ -119,15 +123,13 @@ func configureCircuitBreaker(deps *module.Dependencies, timeoutVal int) bool {
 	if deps.Default.Config.ContainsKey("clients.{{$clientID}}.requestVolumeThreshold") {
 		requestVolumeThreshold = int(deps.Default.Config.MustGetInt("clients.{{$clientID}}.requestVolumeThreshold"))
 	}
-
-	hystrix.ConfigureCommand("{{$clientID}}", hystrix.CommandConfig{
+	hystrix.ConfigureCommand(method, hystrix.CommandConfig{
 		MaxConcurrentRequests:  maxConcurrentRequests,
 		ErrorPercentThreshold:  errorPercentThreshold,
 		SleepWindow:            sleepWindowInMilliseconds,
 		RequestVolumeThreshold: requestVolumeThreshold,
 		Timeout:                timeoutVal,
 	})
-	return true
 }
 
 {{range $i, $svc := .ProtoServices -}}
@@ -163,7 +165,7 @@ func (e *{{$clientName}}) {{$methodName}}(
 	if e.opts.CircuitBreakerDisabled {
 		result, err = runFunc(ctx, request, opts...)
 	} else {
-		err = hystrix.DoC(ctx, "{{$clientID}}", func(ctx context.Context) error {
+		err = hystrix.DoC(ctx, "{{$methodName}}", func(ctx context.Context) error {
 			result, err = runFunc(ctx, request, opts...)
 			return err
 		}, nil)

diff --git a/codegen/templates/http_client.tmpl b/codegen/templates/http_client.tmpl
@@ -109,8 +109,21 @@ func {{$exportName}}(deps *module.Dependencies) Client {
 		followRedirect = deps.Default.Config.MustGetBoolean("clients.{{$clientID}}.followRedirect")
 	}
 
-
-	circuitBreakerDisabled := configureCircuitBreaker(deps, timeoutVal)
+	methodNames := map[string]string{
+				{{range $serviceMethod, $methodName := $exposedMethods -}}
+				"{{$methodName}}": "{{$serviceMethod}}",
+				{{end}}
+	}
+	// circuitBreakerDisabled sets whether circuit-breaker should be disabled
+	circuitBreakerDisabled := false
+	if deps.Default.Config.ContainsKey("clients.{{$clientID}}.circuitBreakerDisabled") {
+		circuitBreakerDisabled = deps.Default.Config.MustGetBoolean("clients.{{$clientID}}.circuitBreakerDisabled")
+	}
+	if !circuitBreakerDisabled {
+		for methodKey := range methodNames {
+			configureCircuitBreaker(deps, timeoutVal, methodKey)
+		}
+	}
 
 	return &{{$clientName}}{
 		clientID: "{{$clientID}}",
@@ -124,11 +137,7 @@ func {{$exportName}}(deps *module.Dependencies) Client {
 		httpClient: zanzibar.NewHTTPClientContext(
 			deps.Default.ContextLogger, deps.Default.ContextMetrics, deps.Default.JSONWrapper,
 			"{{$clientID}}",
-			map[string]string{
-				{{range $serviceMethod, $methodName := $exposedMethods -}}
-				"{{$methodName}}": "{{$serviceMethod}}",
-				{{end}}
-			},
+			methodNames,
 			baseURL,
 			defaultHeaders,
 			timeout,
@@ -155,12 +164,7 @@ func initializeAltRoutingMap(altServiceDetail config.AlternateServiceDetail) map
 }
 {{end -}}
 
-func configureCircuitBreaker(deps *module.Dependencies, timeoutVal int) bool {
-	// circuitBreakerDisabled sets whether circuit-breaker should be disabled
-	circuitBreakerDisabled := false
-	if deps.Default.Config.ContainsKey("clients.{{$clientID}}.circuitBreakerDisabled") {
-		circuitBreakerDisabled = deps.Default.Config.MustGetBoolean("clients.{{$clientID}}.circuitBreakerDisabled")
-	}
+func configureCircuitBreaker(deps *module.Dependencies, timeoutVal int, method string) {
 	// sleepWindowInMilliseconds sets the amount of time, after tripping the circuit,
 	// to reject requests before allowing attempts again to determine if the circuit should again be closed
 	sleepWindowInMilliseconds := 5000
@@ -184,16 +188,13 @@ func configureCircuitBreaker(deps *module.Dependencies, timeoutVal int) bool {
 	if deps.Default.Config.ContainsKey("clients.{{$clientID}}.requestVolumeThreshold") {
 		requestVolumeThreshold = int(deps.Default.Config.MustGetInt("clients.{{$clientID}}.requestVolumeThreshold"))
 	}
-	if !circuitBreakerDisabled {
-		hystrix.ConfigureCommand("{{$clientID}}", hystrix.CommandConfig{
+	hystrix.ConfigureCommand(method, hystrix.CommandConfig{
 			MaxConcurrentRequests:  maxConcurrentRequests,
 			ErrorPercentThreshold:  errorPercentThreshold,
 			SleepWindow:            sleepWindowInMilliseconds,
 			RequestVolumeThreshold: requestVolumeThreshold,
 			Timeout:                timeoutVal,
 		})
-	}
-	return circuitBreakerDisabled
 }
 
 // HTTPClient returns the underlying HTTP client, should only be
@@ -302,7 +303,7 @@ func (c *{{$clientName}}) {{$methodName}}(
 	} else {
 		// We want hystrix ckt-breaker to count errors only for system issues
 		var clientErr error
-		err = hystrix.DoC(ctx, "{{$clientID}}", func(ctx context.Context) error {
+		err = hystrix.DoC(ctx, "{{$methodName}}", func(ctx context.Context) error {
 			res, clientErr = req.Do()
 			if res != nil {
 				// This is not a system error/issue. Downstream responded