Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Log SenderException errors in RemoteReporter #653

Closed
backjo opened this issue Sep 10, 2019 · 9 comments · Fixed by #662
Closed

Log SenderException errors in RemoteReporter #653

backjo opened this issue Sep 10, 2019 · 9 comments · Fixed by #662

Comments

@backjo
Copy link
Contributor

backjo commented Sep 10, 2019

Requirement - what kind of business use case are you trying to solve?

Troubleshooting errors in the trace pipeline can be difficult. The metrics emitted by Jaeger tell me there is an error, but I'm unable to see any information in the logs about that error.

Problem - what in Jaeger blocks you from solving the requirement?

SenderException is swallowed in RemoteReporter

Proposal - what do you suggest to solve the problem or improve the existing situation?

Log the exception at error level

@jpkrohling
Copy link
Collaborator

Instead of blindly logging the exceptions, we should probably have the ability for the application to be called back when an exception occurs, so that it can decide what to do.

@pavolloffay
Copy link
Member

How would be propagate failures to the app?

We don't want to throw exceptions and interrupt the execution. I have run into similar problems in past, the logging should help to pinpoint the issue.

@jpkrohling
Copy link
Collaborator

Callbacks. The Tracer could have a onError() method, accepting instances of an error handler. When an exception happens, the tracer calls all error handlers registered with the onError().

Something like (untested):

// public API
public interface ErrorHandler {
    public void handle(Throwable t);
}

public class JaegerTracer {
    private List<ErrorHandler> errorHandlers = new ArrayList<ErrorHandler>();

    // public API
    public void onError(ErrorHandler handler) {
        this.errorHandlers.add(handler);
    }

    // private API, to be called by Senders/Reporters when an exception happens
    public void notifyError(Throwable t) {
      for (ErrorHandler handler : errorHandlers) {
          handler.handle(t);
      }
    }
}

@yurishkuro
Copy link
Member

my preference would be to log errors rather than maintain an extra API

@quaff
Copy link
Contributor

quaff commented Oct 9, 2019

+1 for logging, errors can be alerted by log collector such as ELK, or ignored by customize logger level in configuration file such as log4j2.xml.

@esukram
Copy link
Contributor

esukram commented Oct 16, 2019

Will give it a try and let you know, if there are any open questions....!

@esukram
Copy link
Contributor

esukram commented Oct 16, 2019

Callbacks. The Tracer could have a onError() method, accepting instances of an error handler. When an exception happens, the tracer calls all error handlers registered with the onError().
The default implementation could be the logger - hat do you think?

It would be easier of course to follow @pavolloffay and @yurishkuro suggestion to just log it 😃

@xificurC
Copy link

just add a log.error() call please

@jpkrohling
Copy link
Collaborator

Yes, follow their suggestion ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants