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

Use DiagnosticsSource instead of middleware #6

Open
SergeyKanzhelev opened this issue Feb 8, 2018 · 3 comments
Open

Use DiagnosticsSource instead of middleware #6

SergeyKanzhelev opened this issue Feb 8, 2018 · 3 comments

Comments

@SergeyKanzhelev
Copy link

Using middleware to track request is not necessary. Diagnostics Source was designed to allow tracking incoming requests without injecting middleware.

@yogiraj07
Copy link
Contributor

Hi @SergeyKanzhelev ,

We appreciate you taking time and providing us feedback. We would like to consider this approach in our future work. We welcome pull request and happy to review it.

Thanks,
Yogi

@travisgosselin
Copy link

Using DiagnosticSource here would be a large improvement to two key areas of XRay:

  1. Usage of DiagnosticSources for System.Net.Http would allow the easy capture of EVERY Http request leaving the application, without having to manually assign an HttpMessageHandler to any component. This would automatically cover most scenarios of HTTP outbound communication requiring the consumer to do nothing be call the middleware they already do for UseXRay. I have begun a POC with this method for a series of internal apps we will be rolling out. So far it is pretty straightforward and working well:
public class XRayHttpDiagnosticListener : IObserver<DiagnosticListener>, IDisposable
    {
        private const string HttpHandlerDiagnosticListenerKey = "HttpHandlerDiagnosticListener";
        private readonly List<IDisposable> _subscription = new List<IDisposable>();

        public XRayHttpDiagnosticListener()
        {
            _subscription.Add(DiagnosticListener.AllListeners.Subscribe(this));
        }

        [DiagnosticName("System.Net.Http.HttpRequestOut")]
        public void IsEnabled()
        {
            // MUST KEEP HERE to enable other diagnostics at the root level
        }

        [DiagnosticName("System.Net.Http.HttpRequestOut.Start")]
        public void OnHttpRequestOutStart(System.Net.Http.HttpRequestMessage request)
        {
            // process this request into xray
            // before we can process it, must ensure async local is in context
            if (AWSXRayRecorder.Instance.IsEntityPresent() && !request.RequestUri.ToString().Contains("amazonaws.com"))
            {
                RequestUtil.ProcessRequest(request);
            }
        }

        [DiagnosticName("System.Net.Http.HttpRequestOut.Stop")]
        public void OnHttpRequestOutStop(System.Net.Http.HttpRequestMessage request, System.Net.Http.HttpResponseMessage response, TaskStatus requestTaskStatus)
        {
            // before we can process it, must ensure async local is in context
            if (AWSXRayRecorder.Instance.IsEntityPresent() && !request.RequestUri.ToString().Contains("amazonaws.com"))
            {
                if (response != null)
                {
                    // request did not throw an exception, so process the response
                    RequestUtil.ProcessResponse(response);
                }

                // always end the segment no matter what
                AWSXRayRecorder.Instance.EndSubsegment();
            }
        }

        [DiagnosticName("System.Net.Http.Exception")]
        public void OnHttpRequestOutException(Exception exception, System.Net.Http.HttpRequestMessage request)
        {
            if (exception != null && AWSXRayRecorder.Instance.IsEntityPresent() && !request.RequestUri.ToString().Contains("amazonaws.com"))
            {
                AWSXRayRecorder.Instance.AddException(exception);
            }
        }

        public void Dispose()
        {
            foreach (var sub in _subscription)
            {
                sub.Dispose();
            }
        }

        void IObserver<DiagnosticListener>.OnCompleted()
        {

        }

        void IObserver<DiagnosticListener>.OnError(Exception error)
        {

        }

        void IObserver<DiagnosticListener>.OnNext(DiagnosticListener value)
        {
            if (value.Name.Equals(HttpHandlerDiagnosticListenerKey))
            {
                _subscription.Add(value.SubscribeWithAdapter(this));
            }
        }
    }
  1. DiagnosticSources can also be used to easily capture all SqlClient activity as well. No need to manually use a TraceableSqlCommand object, again just plugin with middleware, and capture all requests automatically. This is great has it is a simple solution to get XRay working with Dapper. That being said there is a bit more complexity to handle here regarding connection opening and closing, and command execution. Here is a start for something:
public class XRaySqlClientDiagnosticLogger : IObserver<DiagnosticListener>, IDisposable
    {
        private const string SqlClientDiagnosticListenerKey = "SqlClientDiagnosticListener";
        private readonly List<IDisposable> _subscription = new List<IDisposable>();

        public XRaySqlClientDiagnosticLogger()
        {
            _subscription.Add(DiagnosticListener.AllListeners.Subscribe(this));
        }

        [DiagnosticName("System.Data.SqlClient")]
        public void IsEnabled()
        {
            // MUST KEEP HERE to enable other diagnostics at the root level
        }

        [DiagnosticName("System.Data.SqlClient.WriteCommandBefore")]
        public void SqlClientWriteCommandBefore(Guid operationId, string operation, Guid connectionId, SqlCommand command)
        {
            var recorder = AWSXRayRecorder.Instance;
            if (recorder.IsEntityPresent() && command != null)
            {
                var connectionBuilder = new SqlConnectionStringBuilder(command.Connection.ConnectionString);
                connectionBuilder.Remove("Password");
                recorder.BeginSubsegment($"{ connectionBuilder.InitialCatalog }@{ connectionBuilder.DataSource }");
                recorder.SetNamespace("remote");
                recorder.AddMetadata("database_type", "sqlserver");
                recorder.AddMetadata("connection_string", connectionBuilder.ToString());
                recorder.AddMetadata("sql_user", connectionBuilder.UserID);
                recorder.AddMetadata("sql_query", command.CommandText);
            }
        }

        [DiagnosticName("System.Data.SqlClient.WriteCommandAfter")]
        public void SqlClientWriteCommandAfter(Guid operationId, string operation, Guid connectionId, SqlCommand command)
        {
            var recorder = AWSXRayRecorder.Instance;
            if (recorder.IsEntityPresent() && command != null)
            {
                AWSXRayRecorder.Instance.EndSubsegment();
            }
        }

        [DiagnosticName("System.Data.SqlClient.WriteCommandError")]
        public void SqlClientWriteCommandError(Guid operationId, string operation, Guid connectionId, SqlCommand command, Exception exception)
        {
            var recorder = AWSXRayRecorder.Instance;
            if (recorder.IsEntityPresent() && exception != null)
            {
                AWSXRayRecorder.Instance.AddException(exception);
            }
        }

        public void Dispose()
        {
            foreach (var sub in _subscription)
            {
                sub.Dispose();
            }
        }

        void IObserver<DiagnosticListener>.OnCompleted()
        {

        }

        void IObserver<DiagnosticListener>.OnError(Exception error)
        {

        }

        void IObserver<DiagnosticListener>.OnNext(DiagnosticListener value)
        {
            if (value.Name == SqlClientDiagnosticListenerKey)
            {
                _subscription.Add(value.SubscribeWithAdapter(this));
            }
        }
    }

Hope that helps (helped me a lot). I'd love to be able to put in a PR... will see if time allows me to spend some time putting something together!

@yogiraj07
Copy link
Contributor

Hi @travisgosselin ,
Thank you for presenting the approaches. I will take a look at it soon and also welcome you to submit PR.

Best,
Yogi

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

No branches or pull requests

3 participants