Migrate HTTP server to net/http (dapr#6248)

* Convert pprof server to net/http Signed-off-by: ItalyPaleAle <[email protected]> * 🧹 Signed-off-by: ItalyPaleAle <[email protected]> * Converted some middlewares to net/http Signed-off-by: ItalyPaleAle <[email protected]> * Fixed tests Signed-off-by: ItalyPaleAle <[email protected]> * Migrated metrics middleware Signed-off-by: ItalyPaleAle <[email protected]> * Working on tracing middleware Signed-off-by: ItalyPaleAle <[email protected]> * Completed converting middlewares Signed-off-by: ItalyPaleAle <[email protected]> * 💄 Signed-off-by: ItalyPaleAle <[email protected]> * Changed the web server Signed-off-by: ItalyPaleAle <[email protected]> * Removed streaming from HTTP API since it is not working at this time See dapr#6246 Signed-off-by: ItalyPaleAle <[email protected]> * Fixed useMaxBodySize middleware Signed-off-by: ItalyPaleAle <[email protected]> * Fixed tracing headers Signed-off-by: ItalyPaleAle <[email protected]> * Should fix tests Signed-off-by: ItalyPaleAle <[email protected]> * Should actually fix tests Signed-off-by: ItalyPaleAle <[email protected]> * Should fix limiting request body Signed-off-by: ItalyPaleAle <[email protected]> * Add unit tests for limitreadcloser Signed-off-by: ItalyPaleAle <[email protected]> * Document changes in responsewriter Signed-off-by: ItalyPaleAle <[email protected]> * status codes use net/http Signed-off-by: ItalyPaleAle <[email protected]> * Method constants use net/http too Signed-off-by: ItalyPaleAle <[email protected]> * Handle ErrServerClosed better Signed-off-by: ItalyPaleAle <[email protected]> * Tweak SDK pipeline Signed-off-by: ItalyPaleAle <[email protected]> * Added InvokeMethodRequest.WithHTTPHeaders Signed-off-by: ItalyPaleAle <[email protected]> * Fix propagation of trace context between net/http and fasthttp Signed-off-by: ItalyPaleAle <[email protected]> * Fix handling of error responses from app in CallLocalStream Signed-off-by: ItalyPaleAle <[email protected]> * 💄 Signed-off-by: ItalyPaleAle <[email protected]> --------- Signed-off-by: ItalyPaleAle <[email protected]> Co-authored-by: Loong Dai <[email protected]>
msfussell · May 31, 2023 · 5aba3c9 · 5aba3c9
1 parent 054df8c
commit 5aba3c9
Show file tree

Hide file tree

Showing 34 changed files with 1,250 additions and 759 deletions.
diff --git a/.github/workflows/dapr-test-sdk.yml b/.github/workflows/dapr-test-sdk.yml
@@ -79,8 +79,6 @@ jobs:
         with:
           header: ${{ github.run_id }}-python
           number: ${{ env.PR_NUMBER }}
-          hide: true
-          hide_classify: OUTDATED
           GITHUB_TOKEN: ${{ secrets.DAPR_BOT_TOKEN }}
           message: |
             # Dapr SDK Python test
@@ -210,8 +208,6 @@ jobs:
         with:
           header: ${{ github.run_id }}-java
           number: ${{ env.PR_NUMBER }}
-          hide: true
-          hide_classify: OUTDATED
           GITHUB_TOKEN: ${{ secrets.DAPR_BOT_TOKEN }}
           message: |
             # Dapr SDK Java test
@@ -383,8 +379,6 @@ jobs:
         with:
           header: ${{ github.run_id }}-js
           number: ${{ env.PR_NUMBER }}
-          hide: true
-          hide_classify: OUTDATED
           GITHUB_TOKEN: ${{ secrets.DAPR_BOT_TOKEN }}
           message: |
             # Dapr SDK JS test

diff --git a/go.mod b/go.mod
@@ -4,7 +4,6 @@ go 1.20
 
 require (
 	contrib.go.opencensus.io/exporter/prometheus v0.4.2
-	github.com/AdhityaRamadhanus/fasthttpcors v0.0.0-20170121111917-d4c07198763a
 	github.com/PaesslerAG/jsonpath v0.1.1
 	github.com/PuerkitoBio/purell v1.2.0
 	github.com/argoproj/argo-rollouts v1.4.1
@@ -13,6 +12,7 @@ require (
 	github.com/dapr/kit v0.0.5
 	github.com/evanphx/json-patch/v5 v5.6.0
 	github.com/fasthttp/router v1.4.18
+	github.com/go-chi/cors v1.2.1
 	github.com/go-logr/logr v1.2.4
 	github.com/golang/mock v1.6.0
 	github.com/golang/protobuf v1.5.3
@@ -24,7 +24,6 @@ require (
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
 	github.com/hashicorp/go-hclog v1.5.0
 	github.com/hashicorp/go-msgpack/v2 v2.1.0
-	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/golang-lru/v2 v2.0.2
 	github.com/hashicorp/raft v1.4.0
 	github.com/hashicorp/raft-boltdb v0.0.0-20230125174641-2a8082862702
@@ -241,6 +240,7 @@ require (
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
 	github.com/hashicorp/go-msgpack v0.5.5 // indirect
+	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/hashicorp/go-rootcerts v1.0.2 // indirect
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/golang-lru v0.5.4 // indirect

diff --git a/go.sum b/go.sum
@@ -66,8 +66,6 @@ github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMb
 github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4=
 github.com/99designs/keyring v1.2.1 h1:tYLp1ULvO7i3fI5vE21ReQuj99QFSs7lGm0xWyJo87o=
 github.com/99designs/keyring v1.2.1/go.mod h1:fc+wB5KTk9wQ9sDx0kFXB3A0MaeGHM9AwRStKOQ5vOA=
-github.com/AdhityaRamadhanus/fasthttpcors v0.0.0-20170121111917-d4c07198763a h1:XVdatQFSP2YhJGjqLLIfW8QBk4loz/SCe/PxkXDiW+s=
-github.com/AdhityaRamadhanus/fasthttpcors v0.0.0-20170121111917-d4c07198763a/go.mod h1:C0A1KeiVHs+trY6gUTPhhGammbrZ30ZfXRW/nuT7HLw=
 github.com/AthenZ/athenz v1.10.39 h1:mtwHTF/v62ewY2Z5KWhuZgVXftBej1/Tn80zx4DcawY=
 github.com/AthenZ/athenz v1.10.39/go.mod h1:3Tg8HLsiQZp81BJY58JBeU2BR6B/H4/0MQGfCwhHNEA=
 github.com/Azure/azure-sdk-for-go v68.0.0+incompatible h1:fcYLmCpyNYRnvJbPerq7U0hS+6+I79yEDJBqVNcqUzU=
@@ -529,6 +527,8 @@ github.com/gin-gonic/gin v1.7.7/go.mod h1:axIBovoeJpVj8S3BwE0uPMTeReE4+AfFtqpqaZ
 github.com/go-asn1-ber/asn1-ber v1.3.1/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkProFKoKdwZRWMe0=
 github.com/go-chi/chi v4.0.2+incompatible/go.mod h1:eB3wogJHnLi3x/kFX2A+IbTBlXxmMeXJVKy9tTv1XzQ=
 github.com/go-chi/chi/v5 v5.0.7/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
+github.com/go-chi/cors v1.2.1 h1:xEC8UT3Rlp2QuWNEr4Fs/c2EAGVKBwy/1vHx3bppil4=
+github.com/go-chi/cors v1.2.1/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
 github.com/go-co-op/gocron v1.9.0/go.mod h1:DbJm9kdgr1sEvWpHCA7dFFs/PGHPMil9/97EXCRPr4k=
 github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=

diff --git a/pkg/diagnostics/http_monitoring.go b/pkg/diagnostics/http_monitoring.go
@@ -15,16 +15,17 @@ package diagnostics
 
 import (
 	"context"
+	"net/http"
 	"strconv"
 	"strings"
 	"time"
 
-	"github.com/valyala/fasthttp"
 	"go.opencensus.io/stats"
 	"go.opencensus.io/stats/view"
 	"go.opencensus.io/tag"
 
 	diagUtils "github.com/dapr/dapr/pkg/diagnostics/utils"
+	"github.com/dapr/dapr/utils/responsewriter"
 )
 
 // To track the metrics for fasthttp using opencensus, this implementation is inspired by
@@ -209,27 +210,32 @@ func (h *httpMetrics) Init(appID string) error {
 	)
 }
 
-// FastHTTPMiddleware is the middleware to track http server-side requests.
-func (h *httpMetrics) FastHTTPMiddleware(next fasthttp.RequestHandler) fasthttp.RequestHandler {
-	return func(ctx *fasthttp.RequestCtx) {
-		reqContentSize := ctx.Request.Header.ContentLength()
-		if reqContentSize < 0 {
-			reqContentSize = 0
+// HTTPMiddleware is the middleware to track HTTP server-side requests.
+func (h *httpMetrics) HTTPMiddleware(next http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		var reqContentSize int64
+		if cl := r.Header.Get("content-length"); cl != "" {
+			reqContentSize, _ = strconv.ParseInt(cl, 10, 64)
+			if reqContentSize < 0 {
+				reqContentSize = 0
+			}
 		}
 
-		method := string(ctx.Method())
-		path := h.convertPathToMetricLabel(string(ctx.Path()))
+		path := h.convertPathToMetricLabel(r.URL.Path)
 
-		h.ServerRequestReceived(ctx, method, path, int64(reqContentSize))
+		h.ServerRequestReceived(r.Context(), r.Method, path, reqContentSize)
+
+		// Wrap the writer in a ResponseWriter so we can collect stats such as status code and size
+		w = responsewriter.EnsureResponseWriter(w)
 
 		start := time.Now()
 
-		next(ctx)
+		next(w, r)
 
-		status := strconv.Itoa(ctx.Response.StatusCode())
 		elapsed := float64(time.Since(start) / time.Millisecond)
-		respSize := int64(len(ctx.Response.Body()))
-		h.ServerRequestCompleted(ctx, method, path, status, respSize, elapsed)
+		status := strconv.Itoa(w.(responsewriter.ResponseWriter).Status())
+		respSize := int64(w.(responsewriter.ResponseWriter).Size())
+		h.ServerRequestCompleted(r.Context(), r.Method, path, status, respSize, elapsed)
 	}
 }
 

diff --git a/pkg/diagnostics/http_monitoring_test.go b/pkg/diagnostics/http_monitoring_test.go
@@ -1,34 +1,34 @@
 package diagnostics
 
 import (
-	"net"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/valyala/fasthttp"
 	"go.opencensus.io/stats/view"
 )
 
-func TestFastHTTPMiddleware(t *testing.T) {
+func TestHTTPMiddleware(t *testing.T) {
 	requestBody := "fake_requestDaprBody"
 	responseBody := "fake_responseDaprBody"
 
-	testRequestCtx := fakeFastHTTPRequestCtx(requestBody)
-
-	fakeHandler := func(ctx *fasthttp.RequestCtx) {
-		time.Sleep(100 * time.Millisecond)
-		ctx.Response.SetBodyRaw([]byte(responseBody))
-	}
+	testRequest := fakeHTTPRequest(requestBody)
 
 	// create test httpMetrics
 	testHTTP := newHTTPMetrics()
 	testHTTP.Init("fakeID")
 
-	handler := testHTTP.FastHTTPMiddleware(fakeHandler)
+	handler := testHTTP.HTTPMiddleware(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(100 * time.Millisecond)
+		w.Write([]byte(responseBody))
+	})
 
 	// act
-	handler(testRequestCtx)
+	handler(httptest.NewRecorder(), testRequest)
 
 	// assert
 	rows, err := view.RetrieveData("http/server/request_count")
@@ -46,29 +46,24 @@ func TestFastHTTPMiddleware(t *testing.T) {
 	assert.Equal(t, 1, len(rows))
 	assert.Equal(t, "app_id", rows[0].Tags[0].Key.Name())
 	assert.Equal(t, "fakeID", rows[0].Tags[0].Value)
-	assert.True(t, (rows[0].Data).(*view.DistributionData).Min == float64(len([]byte(requestBody))))
+	assert.Equal(t, float64(len(requestBody)), (rows[0].Data).(*view.DistributionData).Min)
 
 	rows, err = view.RetrieveData("http/server/response_bytes")
 	assert.NoError(t, err)
 	assert.Equal(t, 1, len(rows))
-	assert.True(t, (rows[0].Data).(*view.DistributionData).Min == float64(len([]byte(responseBody))))
+	assert.Equal(t, float64(len(responseBody)), (rows[0].Data).(*view.DistributionData).Min)
 
 	rows, err = view.RetrieveData("http/server/latency")
 	assert.NoError(t, err)
 	assert.Equal(t, 1, len(rows))
 	assert.True(t, (rows[0].Data).(*view.DistributionData).Min >= 100.0)
 }
 
-func TestFastHTTPMiddlewareWhenMetricsDisabled(t *testing.T) {
+func TestHTTPMiddlewareWhenMetricsDisabled(t *testing.T) {
 	requestBody := "fake_requestDaprBody"
 	responseBody := "fake_responseDaprBody"
 
-	testRequestCtx := fakeFastHTTPRequestCtx(requestBody)
-
-	fakeHandler := func(ctx *fasthttp.RequestCtx) {
-		time.Sleep(100 * time.Millisecond)
-		ctx.Response.SetBodyRaw([]byte(responseBody))
-	}
+	testRequest := fakeHTTPRequest(requestBody)
 
 	// create test httpMetrics
 	testHTTP := newHTTPMetrics()
@@ -79,10 +74,13 @@ func TestFastHTTPMiddlewareWhenMetricsDisabled(t *testing.T) {
 	views := []*view.View{v}
 	view.Unregister(views...)
 
-	handler := testHTTP.FastHTTPMiddleware(fakeHandler)
+	handler := testHTTP.HTTPMiddleware(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(100 * time.Millisecond)
+		w.Write([]byte(responseBody))
+	})
 
 	// act
-	handler(testRequestCtx)
+	handler(httptest.NewRecorder(), testRequest)
 
 	// assert
 	rows, err := view.RetrieveData("http/server/request_count")
@@ -121,34 +119,16 @@ func TestConvertPathToMethodName(t *testing.T) {
 	}
 }
 
-func fakeFastHTTPRequestCtx(expectedBody string) *fasthttp.RequestCtx {
-	expectedMethod := fasthttp.MethodPost
-	expectedRequestURI := "/invoke/method/testmethod"
-	expectedTransferEncoding := "encoding"
-	expectedHost := "dapr.io"
-	expectedRemoteAddr := "1.2.3.4:6789"
-	expectedHeader := map[string]string{
-		"Correlation-ID":  "e6f4bb20-96c0-426a-9e3d-991ba16a3ebb",
-		"XXX-Remote-Addr": "192.168.0.100",
+func fakeHTTPRequest(body string) *http.Request {
+	req, err := http.NewRequest(http.MethodPost, "http://dapr.io/invoke/method/testmethod", strings.NewReader(body))
+	if err != nil {
+		panic(err)
 	}
+	req.Header.Set("Correlation-ID", "e6f4bb20-96c0-426a-9e3d-991ba16a3ebb")
+	req.Header.Set("XXX-Remote-Addr", "192.168.0.100")
+	req.Header.Set("Transfer-Encoding", "encoding")
+	// This is normally set automatically when the request is sent to a server, but in this case we are not using a real server
+	req.Header.Set("Content-Length", strconv.FormatInt(req.ContentLength, 10))
 
-	var ctx fasthttp.RequestCtx
-	var req fasthttp.Request
-
-	req.Header.SetMethod(expectedMethod)
-	req.SetRequestURI(expectedRequestURI)
-	req.Header.SetHost(expectedHost)
-	req.Header.Add(fasthttp.HeaderTransferEncoding, expectedTransferEncoding)
-	req.Header.SetContentLength(len([]byte(expectedBody)))
-	req.BodyWriter().Write([]byte(expectedBody)) //nolint:errcheck
-
-	for k, v := range expectedHeader {
-		req.Header.Set(k, v)
-	}
-
-	remoteAddr, _ := net.ResolveTCPAddr("tcp", expectedRemoteAddr)
-
-	ctx.Init(&req, remoteAddr, nil)
-
-	return &ctx
+	return req
 }