From 86547047f74ad1868b63aef937a7bc65b5fcff7b Mon Sep 17 00:00:00 2001 From: Vincent Landgraf Date: Mon, 14 Dec 2020 08:05:18 +0100 Subject: [PATCH 1/5] refactoring, move http middleware to separate location --- .../middleware/error_middleware.go} | 14 +++++++------- .../middleware/error_middleware_test.go} | 14 +++++++------- http/{ => middleware}/context.go | 6 +++--- http/{ => middleware}/context_test.go | 10 +++++----- http/{ => middleware}/errors.go | 2 +- http/{ => middleware}/metrics.go | 4 ++-- http/router.go | 5 +++-- 7 files changed, 28 insertions(+), 27 deletions(-) rename http/{middleware.go => jsonapi/middleware/error_middleware.go} (76%) rename http/{middleware_test.go => jsonapi/middleware/error_middleware_test.go} (92%) rename http/{ => middleware}/context.go (95%) rename http/{ => middleware}/context_test.go (91%) rename http/{ => middleware}/errors.go (93%) rename http/{ => middleware}/metrics.go (97%) diff --git a/http/middleware.go b/http/jsonapi/middleware/error_middleware.go similarity index 76% rename from http/middleware.go rename to http/jsonapi/middleware/error_middleware.go index 3e2485a32..2822b40a5 100644 --- a/http/middleware.go +++ b/http/jsonapi/middleware/error_middleware.go @@ -1,4 +1,4 @@ -package http +package middleware import ( "errors" @@ -9,7 +9,7 @@ import ( "github.com/pace/bricks/maintenance/log" ) -type jsonApiErrorWriter struct { +type errorMiddleware struct { http.ResponseWriter req *http.Request statusCode int @@ -17,7 +17,7 @@ type jsonApiErrorWriter struct { hasBytes bool } -func (e *jsonApiErrorWriter) Write(b []byte) (int, error) { +func (e *errorMiddleware) Write(b []byte) (int, error) { if e.hasErr { log.Req(e.req).Warn().Msgf("Error already sent, ignoring: %q", string(b)) return 0, nil @@ -41,16 +41,16 @@ func (e *jsonApiErrorWriter) Write(b []byte) (int, error) { return n, err } -func (e *jsonApiErrorWriter) WriteHeader(code int) { +func (e *errorMiddleware) WriteHeader(code int) { e.statusCode = code e.ResponseWriter.WriteHeader(code) } -// JsonApiErrorWriterMiddleware is a middleware that wraps http.ResponseWriter +// ErrorMiddleware is a middleware that wraps http.ResponseWriter // such that it forces responses with status codes 4xx/5xx to have // Content-Type: application/vnd.api+json -func JsonApiErrorWriterMiddleware(next http.Handler) http.Handler { +func Error(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - next.ServeHTTP(&jsonApiErrorWriter{ResponseWriter: w, req: r}, r) + next.ServeHTTP(&errorMiddleware{ResponseWriter: w, req: r}, r) }) } diff --git a/http/middleware_test.go b/http/jsonapi/middleware/error_middleware_test.go similarity index 92% rename from http/middleware_test.go rename to http/jsonapi/middleware/error_middleware_test.go index 6d1939f2e..6ed9799ea 100644 --- a/http/middleware_test.go +++ b/http/jsonapi/middleware/error_middleware_test.go @@ -1,4 +1,4 @@ -package http +package middleware import ( "encoding/json" @@ -14,7 +14,7 @@ import ( const payload = "dummy response data" -func TestJsonApiErrorMiddleware(t *testing.T) { +func TestErrorMiddleware(t *testing.T) { for _, statusCode := range []int{200, 201, 400, 402, 500, 503} { for _, responseContentType := range []string{"text/plain", "text/html", runtime.JSONAPIContentType} { r := mux.NewRouter() @@ -23,7 +23,7 @@ func TestJsonApiErrorMiddleware(t *testing.T) { w.WriteHeader(statusCode) _, _ = io.WriteString(w, payload) }).Methods("GET") - r.Use(JsonApiErrorWriterMiddleware) + r.Use(Error) rec := httptest.NewRecorder() req := httptest.NewRequest("GET", "/foo", nil) @@ -74,7 +74,7 @@ func TestJsonApiErrorMiddlewareMultipleErrorWrite(t *testing.T) { if _, err := io.WriteString(w, payload); err != nil { t.Fatal(err) } - if jsonWriter, ok := w.(*jsonApiErrorWriter); ok && !jsonWriter.hasErr { + if jsonWriter, ok := w.(*errorMiddleware); ok && !jsonWriter.hasErr { t.Fatal("expected hasErr flag to be set") } if _, err := io.WriteString(w, payload); err != nil { @@ -84,7 +84,7 @@ func TestJsonApiErrorMiddlewareMultipleErrorWrite(t *testing.T) { t.Fatal(err) } }).Methods("GET") - r.Use(JsonApiErrorWriterMiddleware) + r.Use(Error) rec := httptest.NewRecorder() req := httptest.NewRequest("GET", "/foo", nil) @@ -117,7 +117,7 @@ func TestJsonApiErrorMiddlewareInvalidWriteOrder(t *testing.T) { if _, err := io.WriteString(w, payload); err != nil { t.Fatal(err) } - jsonWriter, ok := w.(*jsonApiErrorWriter) + jsonWriter, ok := w.(*errorMiddleware) if ok && !jsonWriter.hasBytes { t.Fatal("expected hasBytes flag to be set") } @@ -125,7 +125,7 @@ func TestJsonApiErrorMiddlewareInvalidWriteOrder(t *testing.T) { w.Header().Set("Content-Type", "text/plain") _, _ = io.WriteString(w, payload) // will get discarded }).Methods("GET") - r.Use(JsonApiErrorWriterMiddleware) + r.Use(Error) rec := httptest.NewRecorder() req := httptest.NewRequest("GET", "/foo", nil) diff --git a/http/context.go b/http/middleware/context.go similarity index 95% rename from http/context.go rename to http/middleware/context.go index 13096f63b..445b08363 100644 --- a/http/context.go +++ b/http/middleware/context.go @@ -1,7 +1,7 @@ // Copyright © 2020 by PACE Telematics GmbH. All rights reserved. // Created at 2020/08/27 by Marius Neugebauer -package http +package middleware import ( "context" @@ -10,10 +10,10 @@ import ( "net/http" ) -// RequestInContextMiddleware stores a representation of the request in the +// RequestInContext stores a representation of the request in the // context of said request. Some information of that request can then be // accessed through the context using functions of this package. -func RequestInContextMiddleware(next http.Handler) http.Handler { +func RequestInContext(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxReq := ctxRequest{ RemoteAddr: r.RemoteAddr, diff --git a/http/context_test.go b/http/middleware/context_test.go similarity index 91% rename from http/context_test.go rename to http/middleware/context_test.go index f17bbd147..316a64c29 100644 --- a/http/context_test.go +++ b/http/middleware/context_test.go @@ -1,7 +1,7 @@ // Copyright © 2020 by PACE Telematics GmbH. All rights reserved. // Created at 2020/08/27 by Marius Neugebauer -package http_test +package middleware_test import ( "context" @@ -9,7 +9,7 @@ import ( "net/http" "testing" - . "github.com/pace/bricks/http" + . "github.com/pace/bricks/http/middleware" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,7 +18,7 @@ func TestContextTransfer(t *testing.T) { r, err := http.NewRequest("GET", "http://example.com/", nil) require.NoError(t, err) r.Header.Set("User-Agent", "Foobar") - RequestInContextMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + RequestInContext(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { ctx := ContextTransfer(r.Context(), context.Background()) userAgent, err := GetUserAgentFromContext(ctx) assert.NoError(t, err) @@ -79,7 +79,7 @@ func TestGetXForwardedForHeaderFromContext(t *testing.T) { if c.XForwardedFor != "" { r.Header.Set("X-Forwarded-For", c.XForwardedFor) } - RequestInContextMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + RequestInContext(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { ctx := r.Context() xForwardedFor, err := GetXForwardedForHeaderFromContext(ctx) if c.ExpectErr != nil { @@ -102,7 +102,7 @@ func TestGetUserAgentFromContext(t *testing.T) { r, err := http.NewRequest("GET", "http://example.com/", nil) require.NoError(t, err) r.Header.Set("User-Agent", "Foobar") - RequestInContextMiddleware(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + RequestInContext(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { ctx := r.Context() userAgent, err := GetUserAgentFromContext(ctx) assert.NoError(t, err) diff --git a/http/errors.go b/http/middleware/errors.go similarity index 93% rename from http/errors.go rename to http/middleware/errors.go index 7fcd1a73a..0d82f5ec8 100644 --- a/http/errors.go +++ b/http/middleware/errors.go @@ -1,7 +1,7 @@ // Copyright © 2020 by PACE Telematics GmbH. All rights reserved. // Created at 2020/08/27 by Marius Neugebauer -package http +package middleware import "errors" diff --git a/http/metrics.go b/http/middleware/metrics.go similarity index 97% rename from http/metrics.go rename to http/middleware/metrics.go index 5eb19330c..77b21166d 100644 --- a/http/metrics.go +++ b/http/middleware/metrics.go @@ -1,4 +1,4 @@ -package http +package middleware import ( "net/http" @@ -59,7 +59,7 @@ func init() { prometheus.MustRegister(paceHTTPInFlightGauge, paceHTTPCounter, paceHTTPDuration, paceHTTPResponseSize) } -func metricsMiddleware(next http.Handler) http.Handler { +func Metrics(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { paceHTTPInFlightGauge.Inc() defer paceHTTPInFlightGauge.Dec() diff --git a/http/router.go b/http/router.go index 2cf2716bf..f6cc29995 100644 --- a/http/router.go +++ b/http/router.go @@ -8,6 +8,7 @@ import ( "net/http/pprof" "github.com/gorilla/mux" + "github.com/pace/bricks/http/middleware" "github.com/pace/bricks/locale" "github.com/pace/bricks/maintenance/errors" "github.com/pace/bricks/maintenance/health" @@ -22,7 +23,7 @@ import ( func Router() *mux.Router { r := mux.NewRouter() - r.Use(metricsMiddleware) + r.Use(middleware.Metrics) // the logging middleware needs to be registered before the // error middleware to make it possible to send panics to @@ -43,7 +44,7 @@ func Router() *mux.Router { r.Use(locale.Handler()) // makes some infos about the request accessable from the context - r.Use(RequestInContextMiddleware) + r.Use(middleware.RequestInContext) // for prometheus r.Handle("/metrics", metric.Handler()) From caeac41edcf0507f8250e7adcfba0f01d9e1402f Mon Sep 17 00:00:00 2001 From: Vincent Landgraf Date: Mon, 14 Dec 2020 14:00:13 +0100 Subject: [PATCH 2/5] implement external round tripper middleware --- http/middleware/external_dependency.go | 143 ++++++++++++++++++++ http/middleware/external_dependency_test.go | 82 +++++++++++ http/router.go | 3 + pkg/context/transfer.go | 2 +- 4 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 http/middleware/external_dependency.go create mode 100644 http/middleware/external_dependency_test.go diff --git a/http/middleware/external_dependency.go b/http/middleware/external_dependency.go new file mode 100644 index 000000000..2b039ac37 --- /dev/null +++ b/http/middleware/external_dependency.go @@ -0,0 +1,143 @@ +// Copyright © 2020 by PACE Telematics GmbH. All rights reserved. +// Created at 2020/12/14 by Vincent Landgraf + +package middleware + +import ( + "bytes" + "context" + "fmt" + "net/http" + "strconv" + "strings" + "sync" + "time" + + "github.com/pace/bricks/maintenance/log" +) + +// depFormat is the format of a single dependency report +const depFormat = "%s:%d" + +// ExternalDependencyHeaderName name of the HTTP header that is used for reporting +const ExternalDependencyHeaderName = "External-Dependencies" + +// ExternalDependency middleware to report external dependencies +func ExternalDependency(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var edc ExternalDependencyContext + edw := externalDependencyWriter{ + ResponseWriter: w, + edc: &edc, + } + r = r.WithContext(ContextWithExternalDependency(r.Context(), &edc)) + next.ServeHTTP(&edw, r) + }) +} + +func AddExternalDependency(ctx context.Context, name string, dur time.Duration) { + ec := ExternalDependencyContextFromContext(ctx) + if ec == nil { + log.Ctx(ctx).Warn().Msgf("can't add external dependency %q with %s, because context is missing", name, dur) + return + } + ec.AddDependency(name, dur) +} + +type externalDependencyWriter struct { + http.ResponseWriter + header bool + edc *ExternalDependencyContext +} + +// addHeader adds the external dependency header if not done already +func (w *externalDependencyWriter) addHeader() { + if !w.header { + if len(w.edc.dependencies) > 0 { + w.ResponseWriter.Header().Add(ExternalDependencyHeaderName, w.edc.String()) + } + w.header = true + } +} + +func (w *externalDependencyWriter) Write(data []byte) (int, error) { + w.addHeader() + return w.ResponseWriter.Write(data) +} + +func (w *externalDependencyWriter) WriteHeader(statusCode int) { + w.addHeader() + w.ResponseWriter.WriteHeader(statusCode) +} + +// ContextWithExternalDependency creates a contex with the external provided dependencies +func ContextWithExternalDependency(ctx context.Context, edc *ExternalDependencyContext) context.Context { + return context.WithValue(ctx, (*ExternalDependencyContext)(nil), edc) +} + +// ExternalDependencyContextFromContext returns the external dependencies context or nil +func ExternalDependencyContextFromContext(ctx context.Context) *ExternalDependencyContext { + if v := ctx.Value((*ExternalDependencyContext)(nil)); v != nil { + return v.(*ExternalDependencyContext) + } + return nil +} + +// ExternalDependencyContext contains all dependencies that where seed +// during the request livecycle +type ExternalDependencyContext struct { + mu sync.RWMutex + dependencies []externalDependency +} + +func (c *ExternalDependencyContext) AddDependency(name string, duration time.Duration) { + c.mu.Lock() + c.dependencies = append(c.dependencies, externalDependency{ + Name: name, + Duration: duration, + }) + c.mu.Unlock() +} + +// String formats all external dependencies +func (c *ExternalDependencyContext) String() string { + var buf bytes.Buffer + sep := len(c.dependencies) - 1 + for _, dep := range c.dependencies { + buf.WriteString(dep.String()) + if sep > 0 { + buf.WriteByte(',') + sep-- + } + } + return buf.String() +} + +// Parse a external dependency value +func (c *ExternalDependencyContext) Parse(s string) { + values := strings.Split(s, ",") + for _, value := range values { + index := strings.IndexByte(value, ':') + if index == -1 { + continue // ignore the invalid values + } + dur, err := strconv.ParseInt(value[index+1:], 10, 64) + if err != nil { + continue // ignore the invalid values + } + + c.AddDependency(value[:index], time.Millisecond*time.Duration(dur)) + } +} + +// externalDependency represents one external dependency that +// was involved in the process to creating a response +type externalDependency struct { + Name string // canonical name of the source + Duration time.Duration // time spend with the external dependency +} + +// String returns a formated single external dependency +func (r externalDependency) String() string { + return fmt.Sprintf(depFormat, r.Name, r.Duration.Milliseconds()) +} diff --git a/http/middleware/external_dependency_test.go b/http/middleware/external_dependency_test.go new file mode 100644 index 000000000..5c7e27df7 --- /dev/null +++ b/http/middleware/external_dependency_test.go @@ -0,0 +1,82 @@ +// Copyright © 2020 by PACE Telematics GmbH. All rights reserved. +// Created at 2020/12/14 by Vincent Landgraf + +package middleware + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func Test_ExternalDependency_Middleare(t *testing.T) { + AddExternalDependency(context.TODO(), "test", time.Second) // should not fail + t.Run("empty set", func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + + h := ExternalDependency(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + h.ServeHTTP(rec, req) + assert.Nil(t, rec.HeaderMap[ExternalDependencyHeaderName]) + }) + t.Run("one dependency set", func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + + h := ExternalDependency(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + AddExternalDependency(r.Context(), "test", time.Second) + w.Write(nil) // nolint: errcheck + })) + h.ServeHTTP(rec, req) + assert.Equal(t, rec.HeaderMap[ExternalDependencyHeaderName][0], "test:1000") + }) +} + +func Test_ExternalDependencyContext_String(t *testing.T) { + var edc ExternalDependencyContext + + // empty + assert.Empty(t, edc.String()) + + // one dependency + edc.AddDependency("test1", time.Millisecond) + assert.EqualValues(t, "test1:1", edc.String()) + + // multiple dependencies + edc.AddDependency("test2", time.Nanosecond) + assert.EqualValues(t, "test1:1,test2:0", edc.String()) + + edc.AddDependency("test3", time.Second) + assert.EqualValues(t, "test1:1,test2:0,test3:1000", edc.String()) + + edc.AddDependency("test4", time.Millisecond*123) + assert.EqualValues(t, "test1:1,test2:0,test3:1000,test4:123", edc.String()) +} + +func Test_ExternalDependencyContext_Parse(t *testing.T) { + var edc ExternalDependencyContext + + // empty + assert.Empty(t, edc.String()) + + // one dependency + edc.Parse("test1:1") + assert.EqualValues(t, "test1:1", edc.String()) + + // ignore invalid lines + edc.Parse("error") + assert.EqualValues(t, "test1:1", edc.String()) + + // multiple dependencies + edc.Parse("test2:0") + assert.EqualValues(t, "test1:1,test2:0", edc.String()) + + edc.Parse("test3:1000,test4:123") + assert.EqualValues(t, "test1:1,test2:0,test3:1000,test4:123", edc.String()) +} diff --git a/http/router.go b/http/router.go index f6cc29995..b3d65f14e 100644 --- a/http/router.go +++ b/http/router.go @@ -43,6 +43,9 @@ func Router() *mux.Router { r.Use(locale.Handler()) + // report use of external dependencies + r.Use(middleware.ExternalDependency) + // makes some infos about the request accessable from the context r.Use(middleware.RequestInContext) diff --git a/pkg/context/transfer.go b/pkg/context/transfer.go index 318bf37b1..e66c427b0 100644 --- a/pkg/context/transfer.go +++ b/pkg/context/transfer.go @@ -3,7 +3,7 @@ package context import ( "context" - "github.com/pace/bricks/http" + http "github.com/pace/bricks/http/middleware" "github.com/pace/bricks/http/oauth2" "github.com/pace/bricks/locale" "github.com/pace/bricks/maintenance/errors" From 8dd0b4c27c763db9e02b15be128097f9c115cded Mon Sep 17 00:00:00 2001 From: Vincent Landgraf Date: Mon, 14 Dec 2020 14:21:32 +0100 Subject: [PATCH 3/5] fix linter issues --- backend/queue/rmq.go | 3 ++ backend/queue/rmq_test.go | 6 ++- http/jsonapi/errors_test.go | 5 ++- http/jsonapi/generator/generate_helper.go | 2 - http/jsonapi/generator/generate_security.go | 5 ++- http/jsonapi/request.go | 8 ++-- http/jsonapi/request_test.go | 40 ++++++++++++++------ http/jsonapi/response_test.go | 12 ++++-- http/jsonapi/runtime.go | 8 ++-- http/jsonapi/runtime/error.go | 8 ---- http/jsonapi/runtime/standart_params_test.go | 7 +++- http/longpoll/longpoll_test.go | 4 +- http/middleware/external_dependency_test.go | 4 +- http/transport/chainable_test.go | 4 +- http/transport/locale_round_tripper_test.go | 2 +- maintenance/errors/context_test.go | 3 +- maintenance/errors/raven/client.go | 13 ++++--- maintenance/errors/raven/stacktrace.go | 2 +- maintenance/log/handler.go | 3 -- 19 files changed, 85 insertions(+), 54 deletions(-) diff --git a/backend/queue/rmq.go b/backend/queue/rmq.go index 0ad3ba50c..b1364112b 100644 --- a/backend/queue/rmq.go +++ b/backend/queue/rmq.go @@ -63,6 +63,9 @@ func NewQueue(name string, healthyLimit int) (rmq.Queue, error) { return nil, err } queue, err := rmqConnection.OpenQueue(name) + if err != nil { + return nil, err + } if _, ok := queueHealthLimits.Load(name); ok { return queue, nil } diff --git a/backend/queue/rmq_test.go b/backend/queue/rmq_test.go index 684af265c..4f4a6fa71 100644 --- a/backend/queue/rmq_test.go +++ b/backend/queue/rmq_test.go @@ -16,7 +16,8 @@ func TestIntegrationHealthCheck(t *testing.T) { ctx := log.WithContext(context.Background()) q1, err := queue.NewQueue("integrationTestTasks", 1) assert.NoError(t, err) - q1.Publish("nothing here") + err = q1.Publish("nothing here") + assert.NoError(t, err) check := &queue.HealthCheck{IgnoreInterval: true} res := check.HealthCheck(ctx) @@ -24,7 +25,8 @@ func TestIntegrationHealthCheck(t *testing.T) { t.Errorf("Expected health check to be OK for a non-full queue") } - q1.Publish("nothing here either") + err = q1.Publish("nothing here either") + assert.NoError(t, err) res = check.HealthCheck(ctx) if res.State == "OK" { diff --git a/http/jsonapi/errors_test.go b/http/jsonapi/errors_test.go index 3f1e47917..7ee1dab7e 100644 --- a/http/jsonapi/errors_test.go +++ b/http/jsonapi/errors_test.go @@ -50,7 +50,10 @@ func TestMarshalErrorsWritesTheExpectedPayload(t *testing.T) { var writer io.Writer = buffer _ = MarshalErrors(writer, testRow.In) - json.Unmarshal(buffer.Bytes(), &output) + err := json.Unmarshal(buffer.Bytes(), &output) + if err != nil { + t.Fatal(err) + } if !reflect.DeepEqual(output, testRow.Out) { t.Fatalf("Expected: \n%#v \nto equal: \n%#v", output, testRow.Out) diff --git a/http/jsonapi/generator/generate_helper.go b/http/jsonapi/generator/generate_helper.go index 576ec4b48..9d40a8afd 100644 --- a/http/jsonapi/generator/generate_helper.go +++ b/http/jsonapi/generator/generate_helper.go @@ -13,8 +13,6 @@ import ( "github.com/getkin/kin-openapi/openapi3" ) -const pkgRuntime = "github.com/pace/bricks/http/jsonapi/runtime" - func (g *Generator) addGoDoc(typeName, description string) { if description != "" { g.goSource.Comment(fmt.Sprintf("%s %s", typeName, description)) diff --git a/http/jsonapi/generator/generate_security.go b/http/jsonapi/generator/generate_security.go index f4db9e3d0..59064ce08 100644 --- a/http/jsonapi/generator/generate_security.go +++ b/http/jsonapi/generator/generate_security.go @@ -112,7 +112,10 @@ func (g *Generator) buildSecurityConfigs(schema *openapi3.Swagger) error { if e, ok := value.Value.Extensions["openIdConnectUrl"]; ok { var url string if data, ok := e.(json.RawMessage); ok { - json.Unmarshal(data, &url) + err := json.Unmarshal(data, &url) + if err != nil { + return err + } instanceVal[jen.Id("OpenIdConnectURL")] = jen.Lit(url) } } diff --git a/http/jsonapi/request.go b/http/jsonapi/request.go index f6892b26d..37c7189e9 100644 --- a/http/jsonapi/request.go +++ b/http/jsonapi/request.go @@ -274,8 +274,8 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) buf := bytes.NewBuffer(nil) - json.NewEncoder(buf).Encode(data.Relationships[args[1]]) - json.NewDecoder(buf).Decode(relationship) + err = json.NewEncoder(buf).Encode(data.Relationships[args[1]]) // nolint: errcheck + json.NewDecoder(buf).Decode(relationship) // nolint: errcheck data := relationship.Data models := reflect.New(fieldValue.Type()).Elem() @@ -302,10 +302,10 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) buf := bytes.NewBuffer(nil) - json.NewEncoder(buf).Encode( + json.NewEncoder(buf).Encode( // nolint: errcheck data.Relationships[args[1]], ) - json.NewDecoder(buf).Decode(relationship) + json.NewDecoder(buf).Decode(relationship) // nolint: errcheck /* http://jsonapi.org/format/#document-resource-object-relationships diff --git a/http/jsonapi/request_test.go b/http/jsonapi/request_test.go index 5a2c00f5c..7fcfb421b 100644 --- a/http/jsonapi/request_test.go +++ b/http/jsonapi/request_test.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "log" "reflect" "sort" "strings" @@ -202,7 +203,6 @@ func TestUnmarshalToStructWithPointerAttr_BadType_Struct(t *testing.T) { func TestUnmarshalToStructWithPointerAttr_BadType_IntSlice(t *testing.T) { out := new(WithPointer) - type FooStruct struct{ A, B int } in := map[string]json.RawMessage{ "name": json.RawMessage(`[4, 5]`), // This is the wrong type. } @@ -361,7 +361,10 @@ func TestUnmarshalParsesISO8601(t *testing.T) { } in := bytes.NewBuffer(nil) - json.NewEncoder(in).Encode(payload) + err := json.NewEncoder(in).Encode(payload) + if err != nil { + log.Fatal(err) + } out := new(Timestamp) @@ -387,7 +390,10 @@ func TestUnmarshalParsesISO8601TimePointer(t *testing.T) { } in := bytes.NewBuffer(nil) - json.NewEncoder(in).Encode(payload) + err := json.NewEncoder(in).Encode(payload) + if err != nil { + t.Fatal(err) + } out := new(Timestamp) @@ -413,7 +419,10 @@ func TestUnmarshalInvalidISO8601(t *testing.T) { } in := bytes.NewBuffer(nil) - json.NewEncoder(in).Encode(payload) + err := json.NewEncoder(in).Encode(payload) + if err != nil { + t.Fatal(err) + } out := new(Timestamp) @@ -969,7 +978,7 @@ func samplePayload() io.Reader { } out := bytes.NewBuffer(nil) - json.NewEncoder(out).Encode(payload) + json.NewEncoder(out).Encode(payload) // nolint: errcheck return out } @@ -987,7 +996,7 @@ func samplePayloadWithID() io.Reader { } out := bytes.NewBuffer(nil) - json.NewEncoder(out).Encode(payload) + json.NewEncoder(out).Encode(payload) // nolint: errcheck return out } @@ -1002,7 +1011,7 @@ func samplePayloadWithBadTypes(m map[string]json.RawMessage) io.Reader { } out := bytes.NewBuffer(nil) - json.NewEncoder(out).Encode(payload) + json.NewEncoder(out).Encode(payload) // nolint: errcheck return out } @@ -1017,7 +1026,7 @@ func sampleWithPointerPayload(m map[string]json.RawMessage) io.Reader { } out := bytes.NewBuffer(nil) - json.NewEncoder(out).Encode(payload) + json.NewEncoder(out).Encode(payload) // nolint: errcheck return out } @@ -1094,17 +1103,26 @@ func samplePayloadWithSideloaded() io.Reader { testModel := testModel() out := bytes.NewBuffer(nil) - MarshalPayload(out, testModel) + err := MarshalPayload(out, testModel) + if err != nil { + panic(err) + } return out } func sampleSerializedEmbeddedTestModel() *Blog { out := bytes.NewBuffer(nil) - MarshalOnePayloadEmbedded(out, testModel()) + err := MarshalOnePayloadEmbedded(out, testModel()) + if err != nil { + panic(err) + } blog := new(Blog) - UnmarshalPayload(out, blog) + err = UnmarshalPayload(out, blog) + if err != nil { + panic(err) + } return blog } diff --git a/http/jsonapi/response_test.go b/http/jsonapi/response_test.go index f1a91989f..456f0db55 100644 --- a/http/jsonapi/response_test.go +++ b/http/jsonapi/response_test.go @@ -27,7 +27,10 @@ func TestMarshalPayload(t *testing.T) { // One out1 := bytes.NewBuffer(nil) - MarshalPayload(out1, book) + err := MarshalPayload(out1, book) + if err != nil { + t.Fatal(err) + } if strings.Contains(out1.String(), `"9.9999999999999999999"`) { t.Fatalf("decimals should be encoded as number, got: %q", out1.String()) @@ -39,11 +42,14 @@ func TestMarshalPayload(t *testing.T) { if _, ok := jsonData["data"].(map[string]interface{}); !ok { t.Fatalf("data key did not contain an Hash/Dict/Map") } - fmt.Println(string(out1.Bytes())) + fmt.Println(out1.String()) // Many out2 := bytes.NewBuffer(nil) - MarshalPayload(out2, books) + err = MarshalPayload(out2, books) + if err != nil { + t.Fatal(err) + } if err := json.Unmarshal(out2.Bytes(), &jsonData); err != nil { t.Fatal(err) diff --git a/http/jsonapi/runtime.go b/http/jsonapi/runtime.go index 2658017c7..7fd67db16 100644 --- a/http/jsonapi/runtime.go +++ b/http/jsonapi/runtime.go @@ -79,13 +79,13 @@ func (r *Runtime) UnmarshalPayload(reader io.Reader, model interface{}) error { } // UnmarshalManyPayload has docs in request.go for UnmarshalManyPayload. -func (r *Runtime) UnmarshalManyPayload(reader io.Reader, kind reflect.Type) (elems []interface{}, err error) { - r.instrumentCall(UnmarshalStart, UnmarshalStop, func() error { - elems, err = UnmarshalManyPayload(reader, kind) +func (r *Runtime) UnmarshalManyPayload(reader io.Reader, kind reflect.Type) (elements []interface{}, err error) { + err2 := r.instrumentCall(UnmarshalStart, UnmarshalStop, func() error { + elements, err = UnmarshalManyPayload(reader, kind) return err }) - return + return elements, err2 } // MarshalPayload has docs in response.go for MarshalPayload. diff --git a/http/jsonapi/runtime/error.go b/http/jsonapi/runtime/error.go index a67be7ea5..dbd6fbc5d 100644 --- a/http/jsonapi/runtime/error.go +++ b/http/jsonapi/runtime/error.go @@ -111,14 +111,6 @@ func WriteError(w http.ResponseWriter, code int, err error) { log.Logger().Info().Str("req_id", reqID). Err(err).Msg("Unable to send error response to the client") } - - // log all errors send to the client - for _, ei := range errList.List { - ev := log.Logger().Info().Str("req_id", reqID) - if source := ei.Source; source != nil { - ev = ev.Fields(*source) - } - } } // Error objects MUST be returned as an array keyed by errors in the top level of a JSON API document. diff --git a/http/jsonapi/runtime/standart_params_test.go b/http/jsonapi/runtime/standart_params_test.go index 147f9297f..5adda657e 100644 --- a/http/jsonapi/runtime/standart_params_test.go +++ b/http/jsonapi/runtime/standart_params_test.go @@ -67,6 +67,7 @@ func TestIntegrationFilterParameter(t *testing.T) { // Paging r = httptest.NewRequest("GET", "http://abc.de/whatEver?page[number]=3&page[size]=2", nil) urlParams, err = runtime.ReadURLQueryParameters(r, mapper, &testValueSanitizer{}) + assert.NoError(t, err) var modelsPaging []TestModel q = db.Model(&modelsPaging) q = urlParams.AddToQuery(q) @@ -77,6 +78,7 @@ func TestIntegrationFilterParameter(t *testing.T) { // Sorting r = httptest.NewRequest("GET", "http://abc.de/whatEver?sort=-test", nil) urlParams, err = runtime.ReadURLQueryParameters(r, mapper, &testValueSanitizer{}) + assert.NoError(t, err) var modelsSort []TestModel q = db.Model(&modelsSort) q = urlParams.AddToQuery(q) @@ -90,16 +92,19 @@ func TestIntegrationFilterParameter(t *testing.T) { // Combine all r = httptest.NewRequest("GET", "http://abc.de/whatEver?sort=-test&filter[test]=a,b&page[number]=0&page[size]=1", nil) urlParams, err = runtime.ReadURLQueryParameters(r, mapper, &testValueSanitizer{}) + assert.NoError(t, err) var modelsCombined []TestModel q = db.Model(&modelsCombined) q = urlParams.AddToQuery(q) err = q.Select() + assert.NoError(t, err) a.Equal(1, len(modelsCombined)) a.Equal("b", modelsCombined[0].FilterName) // Tear Down - db.DropTable(&TestModel{}, &orm.DropTableOptions{ + err = db.DropTable(&TestModel{}, &orm.DropTableOptions{ IfExists: true, }) + assert.NoError(t, err) } func setupDatabase(a *assert.Assertions) *pg.DB { diff --git a/http/longpoll/longpoll_test.go b/http/longpoll/longpoll_test.go index 266438c24..289860ab9 100644 --- a/http/longpoll/longpoll_test.go +++ b/http/longpoll/longpoll_test.go @@ -14,7 +14,7 @@ func TestLongPollUntilBounds(t *testing.T) { ok, err := Until(context.Background(), -1, func(ctx context.Context) (bool, error) { budget, ok := ctx.Deadline() assert.True(t, ok) - assert.Equal(t, time.Millisecond*999, budget.Sub(time.Now()).Truncate(time.Millisecond)) + assert.Equal(t, time.Millisecond*999, budget.Sub(time.Now()).Truncate(time.Millisecond)) // nolint: gosimple called++ return true, nil }) @@ -26,7 +26,7 @@ func TestLongPollUntilBounds(t *testing.T) { ok, err = Until(context.Background(), time.Hour, func(ctx context.Context) (bool, error) { budget, ok := ctx.Deadline() assert.True(t, ok) - assert.Equal(t, time.Second*59, budget.Sub(time.Now()).Truncate(time.Second)) + assert.Equal(t, time.Second*59, budget.Sub(time.Now()).Truncate(time.Second)) // nolint: gosimple called++ return true, nil }) diff --git a/http/middleware/external_dependency_test.go b/http/middleware/external_dependency_test.go index 5c7e27df7..85a1b965e 100644 --- a/http/middleware/external_dependency_test.go +++ b/http/middleware/external_dependency_test.go @@ -23,7 +23,7 @@ func Test_ExternalDependency_Middleare(t *testing.T) { w.WriteHeader(http.StatusOK) })) h.ServeHTTP(rec, req) - assert.Nil(t, rec.HeaderMap[ExternalDependencyHeaderName]) + assert.Nil(t, rec.Result().Header[ExternalDependencyHeaderName]) }) t.Run("one dependency set", func(t *testing.T) { rec := httptest.NewRecorder() @@ -34,7 +34,7 @@ func Test_ExternalDependency_Middleare(t *testing.T) { w.Write(nil) // nolint: errcheck })) h.ServeHTTP(rec, req) - assert.Equal(t, rec.HeaderMap[ExternalDependencyHeaderName][0], "test:1000") + assert.Equal(t, rec.Result().Header[ExternalDependencyHeaderName][0], "test:1000") }) } diff --git a/http/transport/chainable_test.go b/http/transport/chainable_test.go index aa2c46486..b1aab9d3d 100644 --- a/http/transport/chainable_test.go +++ b/http/transport/chainable_test.go @@ -34,12 +34,12 @@ func TestRoundTripperRace(t *testing.T) { go func() { for i := 0; i < 10; i++ { - client.Get(server.URL + "/test001") + client.Get(server.URL + "/test001") // nolint: errcheck } }() for i := 0; i < 10; i++ { - client.Get(server.URL + "/test002") + client.Get(server.URL + "/test002") // nolint: errcheck } } diff --git a/http/transport/locale_round_tripper_test.go b/http/transport/locale_round_tripper_test.go index b9f5fab7d..8115648eb 100644 --- a/http/transport/locale_round_tripper_test.go +++ b/http/transport/locale_round_tripper_test.go @@ -32,7 +32,7 @@ func TestLocaleRoundTrip(t *testing.T) { r, err := http.NewRequest("GET", "http://example.com/test", nil) require.NoError(t, err) - lrt.RoundTrip(r.WithContext(locale.WithLocale(context.Background(), l))) + lrt.RoundTrip(r.WithContext(locale.WithLocale(context.Background(), l))) // nolint: errcheck lctx, ok := locale.FromCtx(mock.r.Context()) require.True(t, ok) diff --git a/maintenance/errors/context_test.go b/maintenance/errors/context_test.go index 9b0a24dc0..c36720d5c 100644 --- a/maintenance/errors/context_test.go +++ b/maintenance/errors/context_test.go @@ -18,7 +18,8 @@ func TestHide(t *testing.T) { canceledContext, cancel := context.WithCancel(backgroundContext) cancel() - exceededContext, _ := context.WithTimeout(backgroundContext, time.Millisecond) + exceededContext, cancel2 := context.WithTimeout(backgroundContext, time.Millisecond) + defer cancel2() time.Sleep(2 * time.Millisecond) type args struct { diff --git a/maintenance/errors/raven/client.go b/maintenance/errors/raven/client.go index aabcfbd03..1eb4a2bcb 100644 --- a/maintenance/errors/raven/client.go +++ b/maintenance/errors/raven/client.go @@ -13,7 +13,6 @@ import ( "fmt" "io" "io/ioutil" - "log" mrand "math/rand" "net/http" "net/url" @@ -25,6 +24,7 @@ import ( "time" "github.com/certifi/gocertifi" + "github.com/pace/bricks/maintenance/log" pkgErrors "github.com/pkg/errors" ) @@ -338,7 +338,6 @@ func (c *context) interfaces() []Interface { } if c.http != nil { interfaces[i] = c.http - i++ } return interfaces } @@ -371,7 +370,11 @@ func newClient(tags map[string]string) *Client { sampleRate: 1.0, queue: make(chan *outgoingPacket, MaxQueueBuffer), } - client.SetDSN(os.Getenv("SENTRY_DSN")) + dsn := os.Getenv("SENTRY_DSN") + err := client.SetDSN(dsn) + if err != nil && dsn != "" { + log.Warnf("DSN environment was set to %q but failed: %v", dsn, err) + } client.SetRelease(os.Getenv("SENTRY_RELEASE")) client.SetEnvironment(os.Getenv("SENTRY_ENVIRONMENT")) return client @@ -953,7 +956,7 @@ func (t *HTTPTransport) Send(url, authHeader string, packet *Packet) error { if err != nil { return err } - io.Copy(ioutil.Discard, res.Body) + io.Copy(ioutil.Discard, res.Body) // nolint: errcheck res.Body.Close() if res.StatusCode != 200 { return fmt.Errorf("raven: got http status %d", res.StatusCode) @@ -973,7 +976,7 @@ func serializedPacket(packet *Packet) (io.Reader, string, error) { buf := &bytes.Buffer{} b64 := base64.NewEncoder(base64.StdEncoding, buf) deflate, _ := zlib.NewWriterLevel(b64, zlib.BestCompression) - deflate.Write(packetJSON) + deflate.Write(packetJSON) // nolint: errcheck deflate.Close() b64.Close() return buf, "application/octet-stream", nil diff --git a/maintenance/errors/raven/stacktrace.go b/maintenance/errors/raven/stacktrace.go index 7d6c40e90..fd89c04f3 100644 --- a/maintenance/errors/raven/stacktrace.go +++ b/maintenance/errors/raven/stacktrace.go @@ -28,7 +28,7 @@ func (s *Stacktrace) Class() string { return "stacktrace" } func (s *Stacktrace) Culprit() string { for i := len(s.Frames) - 1; i >= 0; i-- { frame := s.Frames[i] - if frame.InApp == true && frame.Module != "" && frame.Function != "" { + if frame.InApp && frame.Module != "" && frame.Function != "" { return frame.Module + "." + frame.Function } } diff --git a/maintenance/log/handler.go b/maintenance/log/handler.go index ecb7d23e3..5f5939529 100644 --- a/maintenance/log/handler.go +++ b/maintenance/log/handler.go @@ -4,7 +4,6 @@ package log import ( - "errors" "net" "net/http" "strings" @@ -100,8 +99,6 @@ func isPrivate(ip net.IP) bool { return len(ip) == net.IPv6len && ip[0]&0xfe == 0xfc } -var noXid = errors.New("no xid") - func RequestIDHandler(fieldKey, headerName string) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 68414d9c2e63b0f39db8989263f056b7c3a9493c Mon Sep 17 00:00:00 2001 From: Vincent Landgraf Date: Tue, 15 Dec 2020 11:22:25 +0100 Subject: [PATCH 4/5] measure and pickup external http dependencies --- http/transport/default_transport.go | 16 +++++ .../external_dependency_round_tripper.go | 51 ++++++++++++++ .../external_dependency_round_tripper_test.go | 68 +++++++++++++++++++ 3 files changed, 135 insertions(+) create mode 100644 http/transport/external_dependency_round_tripper.go create mode 100644 http/transport/external_dependency_round_tripper_test.go diff --git a/http/transport/default_transport.go b/http/transport/default_transport.go index ae5a831a0..5589388c8 100644 --- a/http/transport/default_transport.go +++ b/http/transport/default_transport.go @@ -7,6 +7,22 @@ package transport // If not explicitly finalized via `Final` it uses `http.DefaultTransport` as finalizer. func NewDefaultTransportChain() *RoundTripperChain { return Chain( + &ExternalDependencyRoundTripper{}, + NewDefaultRetryRoundTripper(), + &JaegerRoundTripper{}, + NewDumpRoundTripperEnv(), + &LoggingRoundTripper{}, + &LocaleRoundTripper{}, + &RequestIDRoundTripper{}, + ) +} + +// NewDefaultTransportChain returns a transport chain with retry, jaeger and logging support. +// If not explicitly finalized via `Final` it uses `http.DefaultTransport` as finalizer. +// The passed name is recorded as external dependency +func NewDefaultTransportChainWithExternalName(name string) *RoundTripperChain { + return Chain( + &ExternalDependencyRoundTripper{name: name}, NewDefaultRetryRoundTripper(), &JaegerRoundTripper{}, NewDumpRoundTripperEnv(), diff --git a/http/transport/external_dependency_round_tripper.go b/http/transport/external_dependency_round_tripper.go new file mode 100644 index 000000000..e8cc07fdd --- /dev/null +++ b/http/transport/external_dependency_round_tripper.go @@ -0,0 +1,51 @@ +// Copyright © 2020 by PACE Telematics GmbH. All rights reserved. +// Created at 2020/12/14 by Vincent Landgraf + +package transport + +import ( + "net/http" + "time" + + "github.com/pace/bricks/http/middleware" +) + +// ExternalDependencyRoundTripper greps external dependency headers and +// attach them to the currect context +type ExternalDependencyRoundTripper struct { + name string + transport http.RoundTripper +} + +// Transport returns the RoundTripper to make HTTP requests +func (l *ExternalDependencyRoundTripper) Transport() http.RoundTripper { + return l.transport +} + +// SetTransport sets the RoundTripper to make HTTP requests +func (l *ExternalDependencyRoundTripper) SetTransport(rt http.RoundTripper) { + l.transport = rt +} + +// RoundTrip executes a single HTTP transaction via Transport() +func (l *ExternalDependencyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + start := time.Now() + resp, err := l.Transport().RoundTrip(req) + elapsed := time.Since(start) + + ec := middleware.ExternalDependencyContextFromContext(req.Context()) + if ec != nil { + if l.name != "" { + ec.AddDependency(l.name, elapsed) + } + + if resp != nil { + header := resp.Header.Get(middleware.ExternalDependencyHeaderName) + if header != "" { + ec.Parse(header) + } + } + } + + return resp, err +} diff --git a/http/transport/external_dependency_round_tripper_test.go b/http/transport/external_dependency_round_tripper_test.go new file mode 100644 index 000000000..9c2866127 --- /dev/null +++ b/http/transport/external_dependency_round_tripper_test.go @@ -0,0 +1,68 @@ +// Copyright © 2020 by PACE Telematics GmbH. All rights reserved. +// Created at 2020/12/15 by Vincent Landgraf + +package transport + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/pace/bricks/http/middleware" + "github.com/stretchr/testify/assert" +) + +type edRoundTripperMock struct { + req *http.Request + resp *http.Response +} + +func (m *edRoundTripperMock) RoundTrip(req *http.Request) (*http.Response, error) { + m.req = req + return m.resp, nil +} + +func TestExternalDependencyRoundTripper(t *testing.T) { + var edc middleware.ExternalDependencyContext + ctx := middleware.ContextWithExternalDependency(context.Background(), &edc) + + r := httptest.NewRequest("GET", "http://example.com/test", nil) + r = r.WithContext(ctx) + + mock := &edRoundTripperMock{ + resp: &http.Response{ + Header: http.Header{ + middleware.ExternalDependencyHeaderName: []string{"test1:123,test2:53"}, + }, + }, + } + lrt := &ExternalDependencyRoundTripper{transport: mock} + + _, err := lrt.RoundTrip(r) + assert.NoError(t, err) + + assert.EqualValues(t, "test1:123,test2:53", edc.String()) +} + +func TestExternalDependencyRoundTripperWithName(t *testing.T) { + var edc middleware.ExternalDependencyContext + ctx := middleware.ContextWithExternalDependency(context.Background(), &edc) + + r := httptest.NewRequest("GET", "http://example.com/test", nil) + r = r.WithContext(ctx) + + mock := &edRoundTripperMock{ + resp: &http.Response{ + Header: http.Header{ + middleware.ExternalDependencyHeaderName: []string{"test1:123,test2:53"}, + }, + }, + } + lrt := &ExternalDependencyRoundTripper{name: "ext", transport: mock} + + _, err := lrt.RoundTrip(r) + assert.NoError(t, err) + + assert.EqualValues(t, "ext:0,test1:123,test2:53", edc.String()) +} From 851e1a0a06ccb204c117d19e54ccf3c010b059b1 Mon Sep 17 00:00:00 2001 From: Vincent Landgraf Date: Tue, 15 Dec 2020 13:33:48 +0100 Subject: [PATCH 5/5] address review comments --- http/jsonapi/request.go | 4 ++-- http/middleware/external_dependency.go | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/http/jsonapi/request.go b/http/jsonapi/request.go index 37c7189e9..5e2eff67b 100644 --- a/http/jsonapi/request.go +++ b/http/jsonapi/request.go @@ -274,8 +274,8 @@ func unmarshalNode(data *Node, model reflect.Value, included *map[string]*Node) buf := bytes.NewBuffer(nil) - err = json.NewEncoder(buf).Encode(data.Relationships[args[1]]) // nolint: errcheck - json.NewDecoder(buf).Decode(relationship) // nolint: errcheck + json.NewEncoder(buf).Encode(data.Relationships[args[1]]) // nolint: errcheck + json.NewDecoder(buf).Decode(relationship) // nolint: errcheck data := relationship.Data models := reflect.New(fieldValue.Type()).Elem() diff --git a/http/middleware/external_dependency.go b/http/middleware/external_dependency.go index 2b039ac37..d8728f369 100644 --- a/http/middleware/external_dependency.go +++ b/http/middleware/external_dependency.go @@ -4,7 +4,6 @@ package middleware import ( - "bytes" "context" "fmt" "net/http" @@ -83,7 +82,7 @@ func ExternalDependencyContextFromContext(ctx context.Context) *ExternalDependen return nil } -// ExternalDependencyContext contains all dependencies that where seed +// ExternalDependencyContext contains all dependencies that were seen // during the request livecycle type ExternalDependencyContext struct { mu sync.RWMutex @@ -101,16 +100,16 @@ func (c *ExternalDependencyContext) AddDependency(name string, duration time.Dur // String formats all external dependencies func (c *ExternalDependencyContext) String() string { - var buf bytes.Buffer + var b strings.Builder sep := len(c.dependencies) - 1 for _, dep := range c.dependencies { - buf.WriteString(dep.String()) + b.WriteString(dep.String()) if sep > 0 { - buf.WriteByte(',') + b.WriteByte(',') sep-- } } - return buf.String() + return b.String() } // Parse a external dependency value