-
Notifications
You must be signed in to change notification settings - Fork 333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Is it better to move ErrorOccurred into TracingContext #542
Comments
Or public interface ITracingContext
{
// ...
TracingConfig TracingConfig { get; }
}
public static void ErrorOccurred(this ITracingContext tracingContext, SegmentContext context, Exception exception)
{
if (context == null || context.Span == null)
return;
context.Span.IsError = true;
if (exception == null)
return;
var stackTrace = exception.HasInnerExceptions() ? exception.ToDemystifiedString(tracingContext.TracingConfig.ExceptionMaxDepth) : exception.StackTrace;
context.Span.AddLog(LogEvent.Event("error"),
LogEvent.ErrorKind(exception.GetType().FullName),
LogEvent.Message(exception.Message),
LogEvent.ErrorStack(stackTrace));
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Please answer these questions before submitting your issue.
Question
Currently, when calling
SegmentSpan.ErrorOccurred(Exception, TracingConfig)
, you need to pass in theTracingConfig
parameter. Is it better to move the logic ofErrorOccurred
intoTracingContext
?TracingContext
can hold aTracingConfig
object, which is much more convenient than getting aTracingConfig
every timeErrorOccurred
is called.现在我们调用
SegmentSpan.ErrorOccurred(Exception, TracingConfig)
方法的时候需要传入TracingConfig
参数,将ErrorOccurred
的逻辑移动到TracingContext
中是否更好?TracingContext
可以持有一个TracingConfig
对象,相比每次调用ErrorOccurred
都去获取一个TracingConfig
会方便很多。Bug
Which version of SkyWalking, OS and .NET Runtime?
Which company or project?
What happen?
If possible, provide a way for reproducing the error. e.g. demo application, component version.
Requirement or improvement
The text was updated successfully, but these errors were encountered: