From 3279a7ef0f292cd78a3e835a6be0c4a8491f1998 Mon Sep 17 00:00:00 2001 From: Kevin Wan Date: Sun, 13 Mar 2022 14:11:14 +0800 Subject: [PATCH] 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 --- core/breaker/breaker.go | 2 +- rest/handler/tracinghandler.go | 6 +-- rest/httpc/internal/interceptor.go | 8 ++++ rest/httpc/internal/loginterceptor.go | 28 ++++++++++++ rest/httpc/internal/loginterceptor_test.go | 34 ++++++++++++++ rest/httpc/requests.go | 53 ++++++++++++++++++++++ rest/httpc/requests_test.go | 39 ++++++++++++++++ 7 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 rest/httpc/internal/interceptor.go create mode 100644 rest/httpc/internal/loginterceptor.go create mode 100644 rest/httpc/internal/loginterceptor_test.go create mode 100644 rest/httpc/requests.go create mode 100644 rest/httpc/requests_test.go 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/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.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 +} 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) +}