Skip to content
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

Open
1 of 4 tasks
inversionhourglass opened this issue Feb 22, 2023 · 1 comment
Open
1 of 4 tasks

Is it better to move ErrorOccurred into TracingContext #542

inversionhourglass opened this issue Feb 22, 2023 · 1 comment
Assignees

Comments

@inversionhourglass
Copy link
Contributor

Please answer these questions before submitting your issue.

  • Why do you submit this issue?
  • Question or discussion
  • Bug
  • Requirement
  • Feature or performance improvement

Question

  • What do you want to know?
    Currently, when calling SegmentSpan.ErrorOccurred(Exception, TracingConfig), you need to pass in the TracingConfig parameter. Is it better to move the logic of ErrorOccurred into TracingContext? TracingContext can hold a TracingConfig object, which is much more convenient than getting a TracingConfig every time ErrorOccurred is called.
    现在我们调用SegmentSpan.ErrorOccurred(Exception, TracingConfig)方法的时候需要传入TracingConfig参数,将ErrorOccurred的逻辑移动到TracingContext中是否更好?TracingContext可以持有一个TracingConfig对象,相比每次调用ErrorOccurred都去获取一个TracingConfig会方便很多。
public interface ITracingContext
{
    // ...

    void ErrorOccurred(SegmentContext context, Exception exception);
}

public class TracingContext : ITracingContext
{
    // ....

    public void ErrorOccurred(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(_tracingConfig.ExceptionMaxDepth) : exception.StackTrace;
        context.Span.AddLog(LogEvent.Event("error"),
            LogEvent.ErrorKind(exception.GetType().FullName),
            LogEvent.Message(exception.Message),
            LogEvent.ErrorStack(stackTrace));
    }
}

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

  • Please describe about your requirements or improvement suggestions.
@inversionhourglass
Copy link
Contributor Author

Or TracingContext exposes a TracingConfig property, and then adds ErrorOccurred method to TracingContext by way of extension method.
或者TracingContext暴露一个TracingConfig属性,然后通过扩展方法的方式为TracingContext增加ErrorOccurred方法。

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants