From 3819afe64b16f026678b813b62a041757889cedf Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Thu, 15 Aug 2024 23:53:57 -0400 Subject: [PATCH 01/10] saving --- transport/http/handler.go | 31 +++++++++++++++++++++---------- transport/http/outbound.go | 12 +++++------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/transport/http/handler.go b/transport/http/handler.go index 58969c0e2..56dec5fa5 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -55,6 +55,11 @@ type handler struct { logger *zap.Logger } +const ( + //TracingTagStatusCode is the span tag key for the YAPRC status code. + TracingTagStatusCode = "rpc.yarpc.status_code" +) + func (h handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { responseWriter := newResponseWriter(w) service := popHeader(req.Header, ServiceHeader) @@ -117,12 +122,20 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, Body: req.Body, BodySize: int(req.ContentLength), } + + ctx := req.Context() + ctx, cancel, parseTTLErr := parseTTL(ctx, treq, popHeader(req.Header, TTLMSHeader)) + // parseTTLErr != nil is a problem only if the request is unary. + defer cancel() + ctx, span := h.createSpan(ctx, req, treq, start) + for header := range h.grabHeaders { if value := req.Header.Get(header); value != "" { treq.Headers = treq.Headers.With(header, value) } } if err := transport.ValidateRequest(treq); err != nil { + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return err } defer func() { @@ -133,22 +146,18 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, } }() - ctx := req.Context() - ctx, cancel, parseTTLErr := parseTTL(ctx, treq, popHeader(req.Header, TTLMSHeader)) - // parseTTLErr != nil is a problem only if the request is unary. - defer cancel() - ctx, span := h.createSpan(ctx, req, treq, start) - spec, err := h.router.Choose(ctx, treq) if err != nil { - updateSpanWithErr(span, err) + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return err } if parseTTLErr != nil { + updateSpanWithErr(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) return parseTTLErr } if err := transport.ValidateRequestContext(ctx); err != nil { + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return err } switch spec.Type() { @@ -171,7 +180,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, err = yarpcerrors.Newf(yarpcerrors.CodeUnimplemented, "transport http does not handle %s handlers", spec.Type().String()) } - updateSpanWithErr(span, err) + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return err } @@ -185,6 +194,7 @@ func handleOnewayRequest( // returning from the request var buff bytes.Buffer if _, err := iopool.Copy(&buff, treq.Body); err != nil { + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return err } treq.Body = &buff @@ -203,15 +213,16 @@ func handleOnewayRequest( Handler: onewayHandler, Logger: logger, }) - updateSpanWithErr(span, err) + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) }() return nil } -func updateSpanWithErr(span opentracing.Span, err error) { +func updateSpanWithErr(span opentracing.Span, err error, errCode yarpcerrors.Code) { if err != nil { span.SetTag("error", true) span.LogFields(opentracinglog.String("event", err.Error())) + span.SetTag(TracingTagStatusCode, errCode) } } diff --git a/transport/http/outbound.go b/transport/http/outbound.go index bb34e04ac..30fd79067 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -34,7 +34,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" - opentracinglog "github.com/opentracing/opentracing-go/log" "go.uber.org/yarpc" "go.uber.org/yarpc/api/peer" "go.uber.org/yarpc/api/transport" @@ -300,27 +299,26 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor ttl := deadline.Sub(start) hreq, err := o.createRequest(treq) + ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) if err != nil { + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return nil, err } - ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) + defer span.Finish() if err != nil { + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return nil, err } - defer span.Finish() hreq = o.withCoreHeaders(hreq, treq, ttl) hreq = hreq.WithContext(ctx) response, err := o.roundTrip(hreq, treq, start, o.client) if err != nil { - span.SetTag("error", true) - span.LogFields(opentracinglog.String("event", err.Error())) + updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) return nil, err } - span.SetTag("http.status_code", response.StatusCode) - // Service name match validation, return yarpcerrors.CodeInternal error if not match if match, resSvcName := checkServiceMatch(treq.Service, response.Header); !match { if err = response.Body.Close(); err != nil { From 3e18665cc79cd729ba09a1223d4b575ec0ffe9f8 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Sun, 18 Aug 2024 14:15:33 -0400 Subject: [PATCH 02/10] saving --- tracing.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tracing.go b/tracing.go index 5f9ee3e07..df84c0c8e 100644 --- a/tracing.go +++ b/tracing.go @@ -26,8 +26,14 @@ import ( opentracing "github.com/opentracing/opentracing-go" ) +const ( + //TracingComponentName helps determine the attribution of a span. + TracingComponentName = "yarpc" +) + // OpentracingTags are tags with YARPC metadata. var OpentracingTags = opentracing.Tags{ "yarpc.version": Version, "go.version": runtime.Version(), + "component": TracingComponentName, } From c912a33643855e5cc7b02063f2821bd94be9f038 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Sun, 18 Aug 2024 14:40:05 -0400 Subject: [PATCH 03/10] saving --- transport/http/outbound.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/transport/http/outbound.go b/transport/http/outbound.go index 30fd79067..9b325e598 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -299,6 +299,10 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor ttl := deadline.Sub(start) hreq, err := o.createRequest(treq) + if err != nil { + // TODO: Find a way to emit this error to span tag + return nil, err + } ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) if err != nil { updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) From db8a123533ef5a2e8aa4e70d7924eb8ce2c77d01 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Thu, 22 Aug 2024 11:28:58 -0400 Subject: [PATCH 04/10] saving --- api/transport/propagation.go | 5 +++++ transport/http/handler.go | 34 +++++++++++++++++++++------------- transport/http/outbound.go | 9 +++++---- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 7c27ba106..ed3c45bba 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -37,6 +37,11 @@ type CreateOpenTracingSpan struct { ExtraTags opentracing.Tags } +const ( + //TracingTagStatusCode is the span tag key for the YAPRC status code. + TracingTagStatusCode = "rpc.yarpc.status_code" +) + // Do creates a new context that has a reference to the started span. // This should be called before a Outbound makes a call func (c *CreateOpenTracingSpan) Do( diff --git a/transport/http/handler.go b/transport/http/handler.go index 56dec5fa5..d5b8cf033 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -55,10 +55,10 @@ type handler struct { logger *zap.Logger } -const ( - //TracingTagStatusCode is the span tag key for the YAPRC status code. - TracingTagStatusCode = "rpc.yarpc.status_code" -) +//const ( +// //TracingTagStatusCode is the span tag key for the YAPRC status code. +// TracingTagStatusCode = "rpc.yarpc.status_code" +//) func (h handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { responseWriter := newResponseWriter(w) @@ -135,7 +135,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, } } if err := transport.ValidateRequest(treq); err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } defer func() { @@ -148,16 +148,16 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, spec, err := h.router.Choose(ctx, treq) if err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } if parseTTLErr != nil { - updateSpanWithErr(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) + UpdateSpanWithErrAndCode(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) return parseTTLErr } if err := transport.ValidateRequestContext(ctx); err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } switch spec.Type() { @@ -180,7 +180,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, err = yarpcerrors.Newf(yarpcerrors.CodeUnimplemented, "transport http does not handle %s handlers", spec.Type().String()) } - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } @@ -194,7 +194,7 @@ func handleOnewayRequest( // returning from the request var buff bytes.Buffer if _, err := iopool.Copy(&buff, treq.Body); err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } treq.Body = &buff @@ -213,19 +213,27 @@ func handleOnewayRequest( Handler: onewayHandler, Logger: logger, }) - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) }() return nil } -func updateSpanWithErr(span opentracing.Span, err error, errCode yarpcerrors.Code) { +func updateSpanWithErr(span opentracing.Span, err error) { if err != nil { span.SetTag("error", true) span.LogFields(opentracinglog.String("event", err.Error())) - span.SetTag(TracingTagStatusCode, errCode) } } +// UpdateSpanWithErrAndCode sets the error tag with errcode on a span, if an error is given. +// Returns the given error +func UpdateSpanWithErrAndCode(span opentracing.Span, err error, errCode yarpcerrors.Code) { + if err != nil { + span.SetTag(transport.TracingTagStatusCode, errCode) + } + updateSpanWithErr(span, err) +} + func (h handler) createSpan(ctx context.Context, req *http.Request, treq *transport.Request, start time.Time) (context.Context, opentracing.Span) { // Extract opentracing etc baggage from headers // Annotate the inbound context with a trace span diff --git a/transport/http/outbound.go b/transport/http/outbound.go index 9b325e598..871afd0f3 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -300,17 +300,16 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor hreq, err := o.createRequest(treq) if err != nil { - // TODO: Find a way to emit this error to span tag return nil, err } ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) if err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err } defer span.Finish() if err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err } @@ -319,10 +318,12 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor response, err := o.roundTrip(hreq, treq, start, o.client) if err != nil { - updateSpanWithErr(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err } + span.SetTag("http.response.status_code", response.StatusCode) + // Service name match validation, return yarpcerrors.CodeInternal error if not match if match, resSvcName := checkServiceMatch(treq.Service, response.Header); !match { if err = response.Body.Close(); err != nil { From 0935c0688f9f9293d39670b2ea286fb3f82956bd Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Sun, 25 Aug 2024 01:35:23 -0400 Subject: [PATCH 05/10] saving --- transport/http/handler.go | 5 ++++- transport/http/outbound.go | 13 ++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/transport/http/handler.go b/transport/http/handler.go index d5b8cf033..c934de9e2 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -221,7 +221,10 @@ func handleOnewayRequest( func updateSpanWithErr(span opentracing.Span, err error) { if err != nil { span.SetTag("error", true) - span.LogFields(opentracinglog.String("event", err.Error())) + span.LogFields( + opentracinglog.String("event", "error"), + opentracinglog.String("message", err.Error()), + ) } } diff --git a/transport/http/outbound.go b/transport/http/outbound.go index 871afd0f3..4a59cfc3f 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -303,10 +303,6 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor return nil, err } ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) - if err != nil { - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) - return nil, err - } defer span.Finish() if err != nil { UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) @@ -349,14 +345,14 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor bothResponseError := response.Header.Get(BothResponseErrorHeader) == AcceptTrue if bothResponseError && o.bothResponseError { if response.StatusCode >= 300 { - return getYARPCErrorFromResponse(tres, response, true) + return getYARPCErrorFromResponse(span, tres, response, true) } return tres, nil } if response.StatusCode >= 200 && response.StatusCode < 300 { return tres, nil } - return getYARPCErrorFromResponse(tres, response, false) + return getYARPCErrorFromResponse(span, tres, response, false) } func getYARPCApplicationErrorCode(code string) *yarpcerrors.Code { @@ -481,7 +477,7 @@ func (o *Outbound) withCoreHeaders(req *http.Request, treq *transport.Request, t return req } -func getYARPCErrorFromResponse(tres *transport.Response, response *http.Response, bothResponseError bool) (*transport.Response, error) { +func getYARPCErrorFromResponse(span opentracing.Span, tres *transport.Response, response *http.Response, bothResponseError bool) (*transport.Response, error) { var contents string var details []byte if bothResponseError { @@ -527,6 +523,9 @@ func getYARPCErrorFromResponse(tres *transport.Response, response *http.Response ).WithDetails(details) if bothResponseError { + if response.StatusCode >= 400 { + UpdateSpanWithErrAndCode(span, yarpcErr, yarpcerrors.FromError(yarpcErr).Code()) + } return tres, yarpcErr } return nil, yarpcErr From f921ab0b6ed298a2165afee4a83668c40ee1f344 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Sun, 25 Aug 2024 01:42:45 -0400 Subject: [PATCH 06/10] saving --- transport/http/handler.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/transport/http/handler.go b/transport/http/handler.go index c934de9e2..fcd656101 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -55,11 +55,6 @@ type handler struct { logger *zap.Logger } -//const ( -// //TracingTagStatusCode is the span tag key for the YAPRC status code. -// TracingTagStatusCode = "rpc.yarpc.status_code" -//) - func (h handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { responseWriter := newResponseWriter(w) service := popHeader(req.Header, ServiceHeader) From 4ce9dea4b36081bf8ddd9d1d964a335d4ab30a00 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Wed, 28 Aug 2024 02:24:03 -0400 Subject: [PATCH 07/10] saving --- transport/http/handler.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/transport/http/handler.go b/transport/http/handler.go index fcd656101..3a3424dfa 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -117,18 +117,16 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, Body: req.Body, BodySize: int(req.ContentLength), } - - ctx := req.Context() - ctx, cancel, parseTTLErr := parseTTL(ctx, treq, popHeader(req.Header, TTLMSHeader)) - // parseTTLErr != nil is a problem only if the request is unary. - defer cancel() - ctx, span := h.createSpan(ctx, req, treq, start) - for header := range h.grabHeaders { if value := req.Header.Get(header); value != "" { treq.Headers = treq.Headers.With(header, value) } } + ctx := req.Context() + ctx, cancel, parseTTLErr := parseTTL(ctx, treq, popHeader(req.Header, TTLMSHeader)) + // parseTTLErr != nil is a problem only if the request is unary. + defer cancel() + ctx, span := h.createSpan(ctx, req, treq, start) if err := transport.ValidateRequest(treq); err != nil { UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err From 0fe900c318c0d8d58ffd208b2f530e0db9c982eb Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Thu, 29 Aug 2024 23:44:40 -0400 Subject: [PATCH 08/10] saving --- transport/http/outbound.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/transport/http/outbound.go b/transport/http/outbound.go index 4a59cfc3f..260762c75 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -303,7 +303,9 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor return nil, err } ctx, hreq, span, err := o.withOpentracingSpan(ctx, hreq, treq, start) - defer span.Finish() + if span != nil { + defer span.Finish() + } if err != nil { UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err @@ -313,9 +315,12 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor hreq = hreq.WithContext(ctx) response, err := o.roundTrip(hreq, treq, start, o.client) - if err != nil { + if err != nil || (response.StatusCode >= 400 && response.StatusCode < 600) { + // If there's an error or the status code indicates an error, update the span UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) - return nil, err + if err != nil { + return nil, err + } } span.SetTag("http.response.status_code", response.StatusCode) @@ -345,14 +350,14 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor bothResponseError := response.Header.Get(BothResponseErrorHeader) == AcceptTrue if bothResponseError && o.bothResponseError { if response.StatusCode >= 300 { - return getYARPCErrorFromResponse(span, tres, response, true) + return getYARPCErrorFromResponse(tres, response, true) } return tres, nil } if response.StatusCode >= 200 && response.StatusCode < 300 { return tres, nil } - return getYARPCErrorFromResponse(span, tres, response, false) + return getYARPCErrorFromResponse(tres, response, false) } func getYARPCApplicationErrorCode(code string) *yarpcerrors.Code { @@ -477,7 +482,7 @@ func (o *Outbound) withCoreHeaders(req *http.Request, treq *transport.Request, t return req } -func getYARPCErrorFromResponse(span opentracing.Span, tres *transport.Response, response *http.Response, bothResponseError bool) (*transport.Response, error) { +func getYARPCErrorFromResponse(tres *transport.Response, response *http.Response, bothResponseError bool) (*transport.Response, error) { var contents string var details []byte if bothResponseError { @@ -523,9 +528,6 @@ func getYARPCErrorFromResponse(span opentracing.Span, tres *transport.Response, ).WithDetails(details) if bothResponseError { - if response.StatusCode >= 400 { - UpdateSpanWithErrAndCode(span, yarpcErr, yarpcerrors.FromError(yarpcErr).Code()) - } return tres, yarpcErr } return nil, yarpcErr From a009c9fd41ba1e3f66c7b7c25c05d2a8c791ece3 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Fri, 30 Aug 2024 00:12:00 -0400 Subject: [PATCH 09/10] saving --- api/transport/propagation.go | 20 +++++++++++++++++--- transport/http/handler.go | 34 +++++++--------------------------- transport/http/outbound.go | 4 ++-- 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index ed3c45bba..7527d90ab 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -22,6 +22,7 @@ package transport import ( "context" + "go.uber.org/yarpc/yarpcerrors" "time" "github.com/opentracing/opentracing-go" @@ -115,12 +116,25 @@ func (e *ExtractOpenTracingSpan) Do( return ctx, span } -// UpdateSpanWithErr sets the error tag on a span, if an error is given. -// Returns the given error +// UpdateSpanWithErr logs an error to the span. Prefer UpdateSpanWithErrAndCode +// for including an error code in addition to the error message. +// Deprecated: Use UpdateSpanWithErrAndCode instead. func UpdateSpanWithErr(span opentracing.Span, err error) error { if err != nil { span.SetTag("error", true) - span.LogFields(opentracinglog.String("event", err.Error())) + span.LogFields( + opentracinglog.String("event", "error"), + opentracinglog.String("message", err.Error()), + ) } return err } + +// UpdateSpanWithErrAndCode sets the error tag with errcode on a span, if an error is given. +// Returns the given error +func UpdateSpanWithErrAndCode(span opentracing.Span, err error, errCode yarpcerrors.Code) error { + if err != nil { + span.SetTag(TracingTagStatusCode, errCode) + } + return UpdateSpanWithErr(span, err) +} diff --git a/transport/http/handler.go b/transport/http/handler.go index 3a3424dfa..5e5f2c12e 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -30,7 +30,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" - opentracinglog "github.com/opentracing/opentracing-go/log" "go.uber.org/yarpc" "go.uber.org/yarpc/api/transport" "go.uber.org/yarpc/internal/bufferpool" @@ -128,7 +127,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, defer cancel() ctx, span := h.createSpan(ctx, req, treq, start) if err := transport.ValidateRequest(treq); err != nil { - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } defer func() { @@ -141,16 +140,16 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, spec, err := h.router.Choose(ctx, treq) if err != nil { - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } if parseTTLErr != nil { - UpdateSpanWithErrAndCode(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) + transport.UpdateSpanWithErrAndCode(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) return parseTTLErr } if err := transport.ValidateRequestContext(ctx); err != nil { - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } switch spec.Type() { @@ -173,7 +172,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, err = yarpcerrors.Newf(yarpcerrors.CodeUnimplemented, "transport http does not handle %s handlers", spec.Type().String()) } - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } @@ -187,7 +186,7 @@ func handleOnewayRequest( // returning from the request var buff bytes.Buffer if _, err := iopool.Copy(&buff, treq.Body); err != nil { - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } treq.Body = &buff @@ -206,30 +205,11 @@ func handleOnewayRequest( Handler: onewayHandler, Logger: logger, }) - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) }() return nil } -func updateSpanWithErr(span opentracing.Span, err error) { - if err != nil { - span.SetTag("error", true) - span.LogFields( - opentracinglog.String("event", "error"), - opentracinglog.String("message", err.Error()), - ) - } -} - -// UpdateSpanWithErrAndCode sets the error tag with errcode on a span, if an error is given. -// Returns the given error -func UpdateSpanWithErrAndCode(span opentracing.Span, err error, errCode yarpcerrors.Code) { - if err != nil { - span.SetTag(transport.TracingTagStatusCode, errCode) - } - updateSpanWithErr(span, err) -} - func (h handler) createSpan(ctx context.Context, req *http.Request, treq *transport.Request, start time.Time) (context.Context, opentracing.Span) { // Extract opentracing etc baggage from headers // Annotate the inbound context with a trace span diff --git a/transport/http/outbound.go b/transport/http/outbound.go index 260762c75..8de3a7b67 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -307,7 +307,7 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor defer span.Finish() } if err != nil { - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err } @@ -317,7 +317,7 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor response, err := o.roundTrip(hreq, treq, start, o.client) if err != nil || (response.StatusCode >= 400 && response.StatusCode < 600) { // If there's an error or the status code indicates an error, update the span - UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) if err != nil { return nil, err } From 5d6b6cb9a3774a7c0654c1780fc81b7610342674 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Fri, 30 Aug 2024 00:25:33 -0400 Subject: [PATCH 10/10] saving --- api/transport/propagation.go | 20 +++----------------- transport/http/handler.go | 34 +++++++++++++++++++++++++++------- transport/http/outbound.go | 4 ++-- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 7527d90ab..ed3c45bba 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -22,7 +22,6 @@ package transport import ( "context" - "go.uber.org/yarpc/yarpcerrors" "time" "github.com/opentracing/opentracing-go" @@ -116,25 +115,12 @@ func (e *ExtractOpenTracingSpan) Do( return ctx, span } -// UpdateSpanWithErr logs an error to the span. Prefer UpdateSpanWithErrAndCode -// for including an error code in addition to the error message. -// Deprecated: Use UpdateSpanWithErrAndCode instead. +// UpdateSpanWithErr sets the error tag on a span, if an error is given. +// Returns the given error func UpdateSpanWithErr(span opentracing.Span, err error) error { if err != nil { span.SetTag("error", true) - span.LogFields( - opentracinglog.String("event", "error"), - opentracinglog.String("message", err.Error()), - ) + span.LogFields(opentracinglog.String("event", err.Error())) } return err } - -// UpdateSpanWithErrAndCode sets the error tag with errcode on a span, if an error is given. -// Returns the given error -func UpdateSpanWithErrAndCode(span opentracing.Span, err error, errCode yarpcerrors.Code) error { - if err != nil { - span.SetTag(TracingTagStatusCode, errCode) - } - return UpdateSpanWithErr(span, err) -} diff --git a/transport/http/handler.go b/transport/http/handler.go index 5e5f2c12e..3a3424dfa 100644 --- a/transport/http/handler.go +++ b/transport/http/handler.go @@ -30,6 +30,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" + opentracinglog "github.com/opentracing/opentracing-go/log" "go.uber.org/yarpc" "go.uber.org/yarpc/api/transport" "go.uber.org/yarpc/internal/bufferpool" @@ -127,7 +128,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, defer cancel() ctx, span := h.createSpan(ctx, req, treq, start) if err := transport.ValidateRequest(treq); err != nil { - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } defer func() { @@ -140,16 +141,16 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, spec, err := h.router.Choose(ctx, treq) if err != nil { - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } if parseTTLErr != nil { - transport.UpdateSpanWithErrAndCode(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) + UpdateSpanWithErrAndCode(span, parseTTLErr, yarpcerrors.FromError(parseTTLErr).Code()) return parseTTLErr } if err := transport.ValidateRequestContext(ctx); err != nil { - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } switch spec.Type() { @@ -172,7 +173,7 @@ func (h handler) callHandler(responseWriter *responseWriter, req *http.Request, err = yarpcerrors.Newf(yarpcerrors.CodeUnimplemented, "transport http does not handle %s handlers", spec.Type().String()) } - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } @@ -186,7 +187,7 @@ func handleOnewayRequest( // returning from the request var buff bytes.Buffer if _, err := iopool.Copy(&buff, treq.Body); err != nil { - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return err } treq.Body = &buff @@ -205,11 +206,30 @@ func handleOnewayRequest( Handler: onewayHandler, Logger: logger, }) - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) }() return nil } +func updateSpanWithErr(span opentracing.Span, err error) { + if err != nil { + span.SetTag("error", true) + span.LogFields( + opentracinglog.String("event", "error"), + opentracinglog.String("message", err.Error()), + ) + } +} + +// UpdateSpanWithErrAndCode sets the error tag with errcode on a span, if an error is given. +// Returns the given error +func UpdateSpanWithErrAndCode(span opentracing.Span, err error, errCode yarpcerrors.Code) { + if err != nil { + span.SetTag(transport.TracingTagStatusCode, errCode) + } + updateSpanWithErr(span, err) +} + func (h handler) createSpan(ctx context.Context, req *http.Request, treq *transport.Request, start time.Time) (context.Context, opentracing.Span) { // Extract opentracing etc baggage from headers // Annotate the inbound context with a trace span diff --git a/transport/http/outbound.go b/transport/http/outbound.go index 8de3a7b67..260762c75 100644 --- a/transport/http/outbound.go +++ b/transport/http/outbound.go @@ -307,7 +307,7 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor defer span.Finish() } if err != nil { - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) return nil, err } @@ -317,7 +317,7 @@ func (o *Outbound) call(ctx context.Context, treq *transport.Request) (*transpor response, err := o.roundTrip(hreq, treq, start, o.client) if err != nil || (response.StatusCode >= 400 && response.StatusCode < 600) { // If there's an error or the status code indicates an error, update the span - transport.UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) + UpdateSpanWithErrAndCode(span, err, yarpcerrors.FromError(err).Code()) if err != nil { return nil, err }