From d9fe3257c8c68f5433488dfbdad2ba51e95df19c Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Mon, 26 Aug 2024 15:46:48 -0400 Subject: [PATCH 1/9] saving --- api/transport/propagation.go | 21 +++++++++++++++++++++ internal/observability/call.go | 10 ++++++++++ 2 files changed, 31 insertions(+) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 7c27ba106..b06e3feac 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" @@ -37,6 +38,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( @@ -119,3 +125,18 @@ func UpdateSpanWithErr(span opentracing.Span, err error) error { } return err } + +// UpdateSpanWithoutErrMsg sets the error tag and status code on a span. +// The error message is intentionally omitted to prevent exposing +// personally identifiable information (PII) in tracing systems. +// Returns the given error +func UpdateSpanWithoutErrMsg(span opentracing.Span, err error, errCode yarpcerrors.Code) error { + if err != nil { + span.SetTag("error", true) + span.SetTag(TracingTagStatusCode, errCode) + span.LogFields( + opentracinglog.String("event", "error"), + ) + } + return err +} diff --git a/internal/observability/call.go b/internal/observability/call.go index b9fe0014e..64653ca0e 100644 --- a/internal/observability/call.go +++ b/internal/observability/call.go @@ -23,6 +23,7 @@ package observability import ( "context" "fmt" + "github.com/opentracing/opentracing-go" "time" "go.uber.org/yarpc/api/transport" @@ -135,6 +136,8 @@ func (c call) endWithAppError( res callResult, extraLogFields ...zap.Field) { elapsed := _timeNow().Sub(c.started) + // Emit application error to span tag if applicable + c.emitSpanErrorTags(res) c.endLogs(elapsed, res.err, res.isApplicationError, res.applicationErrorMeta, extraLogFields...) c.endStats(elapsed, res) } @@ -504,3 +507,10 @@ func errToMetricString(err error) string { } return "unknown_internal_yarpc" } + +// emitSpanErrorTags sets the error information as tags on the current span +func (c call) emitSpanErrorTags(res callResult) { + if span := opentracing.SpanFromContext(c.ctx); span != nil { + transport.UpdateSpanWithoutErrMsg(span, res.err, yarpcerrors.FromError(res.err).Code()) + } +} From c3e0cbefaeb0a2cf9943e59ac65d028633100a18 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Mon, 26 Aug 2024 18:01:33 -0400 Subject: [PATCH 2/9] saving --- internal/observability/call.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/observability/call.go b/internal/observability/call.go index 64653ca0e..3daad7138 100644 --- a/internal/observability/call.go +++ b/internal/observability/call.go @@ -137,7 +137,7 @@ func (c call) endWithAppError( extraLogFields ...zap.Field) { elapsed := _timeNow().Sub(c.started) // Emit application error to span tag if applicable - c.emitSpanErrorTags(res) + c.emitSpanErrorTag(res) c.endLogs(elapsed, res.err, res.isApplicationError, res.applicationErrorMeta, extraLogFields...) c.endStats(elapsed, res) } @@ -508,8 +508,8 @@ func errToMetricString(err error) string { return "unknown_internal_yarpc" } -// emitSpanErrorTags sets the error information as tags on the current span -func (c call) emitSpanErrorTags(res callResult) { +// emitSpanErrorTag sets the error information as tags on the current span +func (c call) emitSpanErrorTag(res callResult) { if span := opentracing.SpanFromContext(c.ctx); span != nil { transport.UpdateSpanWithoutErrMsg(span, res.err, yarpcerrors.FromError(res.err).Code()) } From 8d2c6cf27d65e6ab5f51189b4ffc45e9078888cd Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Mon, 26 Aug 2024 18:11:16 -0400 Subject: [PATCH 3/9] saving --- internal/observability/call.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/observability/call.go b/internal/observability/call.go index 3daad7138..37014008e 100644 --- a/internal/observability/call.go +++ b/internal/observability/call.go @@ -499,6 +499,15 @@ func (c call) logStreamEvent(err error, success bool, succMsg, errMsg string, ex ce.Write(fields...) } +// emitSpanErrorTag sets the error information as tags on the current span +func (c call) emitSpanErrorTag(res callResult) { + if span := opentracing.SpanFromContext(c.ctx); span != nil { + if res.isApplicationError { + transport.UpdateSpanWithoutErrMsg(span, res.err, yarpcerrors.FromError(res.err).Code()) + } + } +} + // inteded for metric tags, this returns the yarpcerrors.Status error code name // or "unknown_internal_yarpc" func errToMetricString(err error) string { @@ -507,10 +516,3 @@ func errToMetricString(err error) string { } return "unknown_internal_yarpc" } - -// emitSpanErrorTag sets the error information as tags on the current span -func (c call) emitSpanErrorTag(res callResult) { - if span := opentracing.SpanFromContext(c.ctx); span != nil { - transport.UpdateSpanWithoutErrMsg(span, res.err, yarpcerrors.FromError(res.err).Code()) - } -} From a41057e7d353a289d15457cc18a82f10d66a020c Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Mon, 26 Aug 2024 18:16:36 -0400 Subject: [PATCH 4/9] saving --- internal/observability/call.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/observability/call.go b/internal/observability/call.go index 37014008e..2ee8ac242 100644 --- a/internal/observability/call.go +++ b/internal/observability/call.go @@ -502,6 +502,7 @@ func (c call) logStreamEvent(err error, success bool, succMsg, errMsg string, ex // emitSpanErrorTag sets the error information as tags on the current span func (c call) emitSpanErrorTag(res callResult) { if span := opentracing.SpanFromContext(c.ctx); span != nil { + // emit application error to span tag if applicable if res.isApplicationError { transport.UpdateSpanWithoutErrMsg(span, res.err, yarpcerrors.FromError(res.err).Code()) } From 502479ef3df9a84aaa529af058bea1e2659dd2e0 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Mon, 26 Aug 2024 18:22:51 -0400 Subject: [PATCH 5/9] saving --- internal/observability/call.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/observability/call.go b/internal/observability/call.go index 2ee8ac242..1c62e7c54 100644 --- a/internal/observability/call.go +++ b/internal/observability/call.go @@ -504,7 +504,8 @@ func (c call) emitSpanErrorTag(res callResult) { if span := opentracing.SpanFromContext(c.ctx); span != nil { // emit application error to span tag if applicable if res.isApplicationError { - transport.UpdateSpanWithoutErrMsg(span, res.err, yarpcerrors.FromError(res.err).Code()) + transport.UpdateSpanWithoutErrMsg(span, + res.err, yarpcerrors.FromError(res.err).Code()) } } } From 67ff9f6453475c91ce3b064452e547bb892dad21 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Mon, 26 Aug 2024 18:33:18 -0400 Subject: [PATCH 6/9] saving --- api/transport/propagation.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index b06e3feac..114b5d96f 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -130,7 +130,7 @@ func UpdateSpanWithErr(span opentracing.Span, err error) error { // The error message is intentionally omitted to prevent exposing // personally identifiable information (PII) in tracing systems. // Returns the given error -func UpdateSpanWithoutErrMsg(span opentracing.Span, err error, errCode yarpcerrors.Code) error { +func UpdateSpanWithoutErrMsg(span opentracing.Span, err error, errCode yarpcerrors.Code) { if err != nil { span.SetTag("error", true) span.SetTag(TracingTagStatusCode, errCode) @@ -138,5 +138,4 @@ func UpdateSpanWithoutErrMsg(span opentracing.Span, err error, errCode yarpcerro opentracinglog.String("event", "error"), ) } - return err } From 464cf6445078eba4ebe23555533620e5684fb474 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Wed, 4 Sep 2024 21:53:02 -0400 Subject: [PATCH 7/9] saving --- api/transport/propagation.go | 9 +++++---- internal/observability/call.go | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 114b5d96f..52e0f1032 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -22,12 +22,12 @@ package transport import ( "context" - "go.uber.org/yarpc/yarpcerrors" "time" "github.com/opentracing/opentracing-go" "github.com/opentracing/opentracing-go/ext" opentracinglog "github.com/opentracing/opentracing-go/log" + "go.uber.org/yarpc/yarpcerrors" ) // CreateOpenTracingSpan creates a new context with a started span @@ -39,7 +39,7 @@ type CreateOpenTracingSpan struct { } const ( - //TracingTagStatusCode is the span tag key for the YAPRC status code. + // TracingTagStatusCode is the span tag key for the YAPRC status code. TracingTagStatusCode = "rpc.yarpc.status_code" ) @@ -126,11 +126,11 @@ func UpdateSpanWithErr(span opentracing.Span, err error) error { return err } -// UpdateSpanWithoutErrMsg sets the error tag and status code on a span. +// UpdateSpanWithApplicationErr sets the error tag and status code on a span for application error. // The error message is intentionally omitted to prevent exposing // personally identifiable information (PII) in tracing systems. // Returns the given error -func UpdateSpanWithoutErrMsg(span opentracing.Span, err error, errCode yarpcerrors.Code) { +func UpdateSpanWithApplicationErr(span opentracing.Span, err error, errCode yarpcerrors.Code) error { if err != nil { span.SetTag("error", true) span.SetTag(TracingTagStatusCode, errCode) @@ -138,4 +138,5 @@ func UpdateSpanWithoutErrMsg(span opentracing.Span, err error, errCode yarpcerro opentracinglog.String("event", "error"), ) } + return err } diff --git a/internal/observability/call.go b/internal/observability/call.go index 1c62e7c54..69611387e 100644 --- a/internal/observability/call.go +++ b/internal/observability/call.go @@ -137,7 +137,7 @@ func (c call) endWithAppError( extraLogFields ...zap.Field) { elapsed := _timeNow().Sub(c.started) // Emit application error to span tag if applicable - c.emitSpanErrorTag(res) + c.emitSpanApplicationErrTag(res) c.endLogs(elapsed, res.err, res.isApplicationError, res.applicationErrorMeta, extraLogFields...) c.endStats(elapsed, res) } @@ -499,14 +499,16 @@ func (c call) logStreamEvent(err error, success bool, succMsg, errMsg string, ex ce.Write(fields...) } -// emitSpanErrorTag sets the error information as tags on the current span -func (c call) emitSpanErrorTag(res callResult) { +// emitSpanApplicationErrTag sets the application error information as tags on the current span +func (c call) emitSpanApplicationErrTag(res callResult) { + // Check if application error is applicable first + if !res.isApplicationError { + return + } if span := opentracing.SpanFromContext(c.ctx); span != nil { - // emit application error to span tag if applicable - if res.isApplicationError { - transport.UpdateSpanWithoutErrMsg(span, - res.err, yarpcerrors.FromError(res.err).Code()) - } + // Emit application error to span tag if applicable + transport.UpdateSpanWithApplicationErr(span, + res.err, yarpcerrors.FromError(res.err).Code()) } } From 6ccb1d02042dd3e9f176c699b5c811f86bda17a8 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Wed, 4 Sep 2024 22:31:42 -0400 Subject: [PATCH 8/9] saving --- api/transport/propagation.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 52e0f1032..6c81b1ff6 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -138,5 +138,4 @@ func UpdateSpanWithApplicationErr(span opentracing.Span, err error, errCode yarp opentracinglog.String("event", "error"), ) } - return err } From bdfb7f029e332a5dc29837bcd94a7ec1160290a7 Mon Sep 17 00:00:00 2001 From: Kexiong Liu Date: Wed, 4 Sep 2024 23:55:48 -0400 Subject: [PATCH 9/9] saving --- api/transport/propagation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/transport/propagation.go b/api/transport/propagation.go index 6c81b1ff6..f33a837dc 100644 --- a/api/transport/propagation.go +++ b/api/transport/propagation.go @@ -130,7 +130,7 @@ func UpdateSpanWithErr(span opentracing.Span, err error) error { // The error message is intentionally omitted to prevent exposing // personally identifiable information (PII) in tracing systems. // Returns the given error -func UpdateSpanWithApplicationErr(span opentracing.Span, err error, errCode yarpcerrors.Code) error { +func UpdateSpanWithApplicationErr(span opentracing.Span, err error, errCode yarpcerrors.Code) { if err != nil { span.SetTag("error", true) span.SetTag(TracingTagStatusCode, errCode)