Repository: zeromicro/go-zero
Pull request #1638 (2022-03-13 05:55:01)

feat: add rest/httpc to make http requests governacible


Patch:
diff --git a/ROADMAP.md b/ROADMAP.md
index 1da02bcb..517c6126 100644
--- a/ROADMAP.md
+++ b/ROADMAP.md
@@ -23,6 +23,6 @@ We hope that the items listed below will inspire further engagement from the com
 - [x] Support `context` in redis related methods for timeout and tracing
 - [x] Support `context` in sql related methods for timeout and tracing
 - [ ] Support `context` in mongodb related methods for timeout and tracing
-- [ ] Add `httpx.Client` with governance, like circuit breaker etc.
+- [x] Add `httpc.Do` with HTTP call governance, like circuit breaker etc.
 - [ ] Support `goctl doctor` command to report potential issues for given service
 - [ ] Support `goctl mock` command to start a mocking server with given `.api` file
diff --git a/core/breaker/breaker.go b/core/breaker/breaker.go
index 81dd438f..08acf8c7 100644
--- a/core/breaker/breaker.go
+++ b/core/breaker/breaker.go
@@ -171,7 +171,7 @@ func (lt loggedThrottle) allow() (Promise, error) {
 func (lt loggedThrottle) doReq(req func() error, fallback func(err error) error, acceptable Acceptable) error {
 	return lt.logError(lt.internalThrottle.doReq(req, fallback, func(err error) bool {
 		accept := acceptable(err)
-		if !accept {
+		if !accept && err != nil {
 			lt.errWin.add(err.Error())
 		}
 		return accept
diff --git a/rest/handler/tracinghandler.go b/rest/handler/tracinghandler.go
index 0b1cdf7e..be220f1c 100644
--- a/rest/handler/tracinghandler.go
+++ b/rest/handler/tracinghandler.go
@@ -32,11 +32,7 @@ func TracingHandler(serviceName, path string) func(http.Handler) http.Handler {
 			defer span.End()
 
 			// convenient for tracking error messages
-			sc := span.SpanContext()
-			if sc.HasTraceID() {
-				w.Header().Set(trace.TraceIdKey, sc.TraceID().String())
-			}
-
+			propagator.Inject(spanCtx, propagation.HeaderCarrier(w.Header()))
 			next.ServeHTTP(w, r.WithContext(spanCtx))
 		})
 	}
diff --git a/rest/httpc/internal/interceptor.go b/rest/httpc/internal/interceptor.go
new file mode 100644
index 00000000..d7171b9f
--- /dev/null
+++ b/rest/httpc/internal/interceptor.go
@@ -0,0 +1,8 @@
+package internal
+
+import "net/http"
+
+type (
+	Interceptor     func(r *http.Request) (*http.Request, ResponseHandler)
+	ResponseHandler func(*http.Response)
+)
diff --git a/rest/httpc/internal/loginterceptor.go b/rest/httpc/internal/loginterceptor.go
new file mode 100644
index 00000000..1e79f153
--- /dev/null
+++ b/rest/httpc/internal/loginterceptor.go
@@ -0,0 +1,28 @@
+package internal
+
+import (
+	"net/http"
+
+	"github.com/zeromicro/go-zero/core/logx"
+	"github.com/zeromicro/go-zero/core/timex"
+	"go.opentelemetry.io/otel/propagation"
+)
+
+func LogInterceptor(r *http.Request) (*http.Request, ResponseHandler) {
+	start := timex.Now()
+	return r, func(resp *http.Response) {
+		duration := timex.Since(start)
+		var tc propagation.TraceContext
+		ctx := tc.Extract(r.Context(), propagation.HeaderCarrier(resp.Header))
+		logger := logx.WithContext(ctx).WithDuration(duration)
+		if isOkResponse(resp.StatusCode) {
+			logger.Infof("[HTTP] %d - %s %s/%s", resp.StatusCode, r.Method, r.Host, r.RequestURI)
+		} else {
+			logger.Errorf("[HTTP] %d - %s %s/%s", resp.StatusCode, r.Method, r.Host, r.RequestURI)
+		}
+	}
+}
+
+func isOkResponse(code int) bool {
+	return code < http.StatusBadRequest
+}
diff --git a/rest/httpc/requests.go b/rest/httpc/requests.go
new file mode 100644
index 00000000..05460745
--- /dev/null
+++ b/rest/httpc/requests.go
@@ -0,0 +1,53 @@
+package httpc
+
+import (
+	"net/http"
+
+	"github.com/zeromicro/go-zero/core/breaker"
+	"github.com/zeromicro/go-zero/core/logx"
+	"github.com/zeromicro/go-zero/rest/httpc/internal"
+)
+
+var interceptors = []internal.Interceptor{
+	internal.LogInterceptor,
+}
+
+type Option func(cli *http.Client)
+
+// Do sends an HTTP request to the service assocated with the given key.
+func Do(key string, r *http.Request, opts ...Option) (resp *http.Response, err error) {
+	var respHandlers []internal.ResponseHandler
+	for _, interceptor := range interceptors {
+		var h internal.ResponseHandler
+		r, h = interceptor(r)
+		respHandlers = append(respHandlers, h)
+	}
+
+	resp, err = doRequest(key, r, opts...)
+	if err != nil {
+		logx.Errorf("[HTTP] %s %s/%s - %v", r.Method, r.Host, r.RequestURI, err)
+		return
+	}
+
+	for i := len(respHandlers) - 1; i >= 0; i-- {
+		respHandlers[i](resp)
+	}
+
+	return
+}
+
+func doRequest(key string, r *http.Request, opts ...Option) (resp *http.Response, err error) {
+	brk := breaker.GetBreaker(key)
+	err = brk.DoWithAcceptable(func() error {
+		var cli http.Client
+		for _, opt := range opts {
+			opt(&cli)
+		}
+		resp, err = cli.Do(r)
+		return err
+	}, func(err error) bool {
+		return err == nil && resp.StatusCode < http.StatusInternalServerError
+	})
+
+	return
+}


Test patch:
diff --git a/rest/httpc/internal/loginterceptor_test.go b/rest/httpc/internal/loginterceptor_test.go
new file mode 100644
index 00000000..108dfdee
--- /dev/null
+++ b/rest/httpc/internal/loginterceptor_test.go
@@ -0,0 +1,34 @@
+package internal
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLogInterceptor(t *testing.T) {
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	}))
+	req, err := http.NewRequest(http.MethodGet, svr.URL, nil)
+	assert.Nil(t, err)
+	req, handler := LogInterceptor(req)
+	resp, err := http.DefaultClient.Do(req)
+	assert.Nil(t, err)
+	handler(resp)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}
+
+func TestLogInterceptorServerError(t *testing.T) {
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	req, err := http.NewRequest(http.MethodGet, svr.URL, nil)
+	assert.Nil(t, err)
+	req, handler := LogInterceptor(req)
+	resp, err := http.DefaultClient.Do(req)
+	assert.Nil(t, err)
+	handler(resp)
+	assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
+}
diff --git a/rest/httpc/requests_test.go b/rest/httpc/requests_test.go
new file mode 100644
index 00000000..151dac62
--- /dev/null
+++ b/rest/httpc/requests_test.go
@@ -0,0 +1,39 @@
+package httpc
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDo(t *testing.T) {
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	}))
+	req, err := http.NewRequest(http.MethodGet, svr.URL, nil)
+	assert.Nil(t, err)
+	resp, err := Do("foo", req, func(cli *http.Client) {
+		cli.Transport = http.DefaultTransport
+	})
+	assert.Nil(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}
+
+func TestDoNotFound(t *testing.T) {
+	svr := httptest.NewServer(http.NotFoundHandler())
+	req, err := http.NewRequest(http.MethodGet, svr.URL, nil)
+	assert.Nil(t, err)
+	resp, err := Do("foo", req)
+	assert.Nil(t, err)
+	assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+}
+
+func TestDoMoved(t *testing.T) {
+	svr := httptest.NewServer(http.RedirectHandler("/foo", http.StatusMovedPermanently))
+	req, err := http.NewRequest(http.MethodGet, svr.URL, nil)
+	assert.Nil(t, err)
+	_, err = Do("foo", req)
+	// too many redirects
+	assert.NotNil(t, err)
+}