Browse Source

feat: add rest/httpc to make http requests governacible (#1638)

* feat: change x-trace-id to traceparent to follow opentelemetry

* feat: add rest/httpc to make http requests governacible

* chore: remove blank lines
Kevin Wan 3 years ago
parent
commit
3279a7ef0f

+ 1 - 1
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

+ 1 - 5
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))
 		})
 	}

+ 8 - 0
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)
+)

+ 28 - 0
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
+}

+ 34 - 0
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)
+}

+ 53 - 0
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
+}

+ 39 - 0
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)
+}