Azure / diagnostics-correlation

Microsoft Diagnostics Correlation
MIT License
15 stars 8 forks source link

CorrelationContextInjector exception when using HttpClient outside of ASP.NET #4

Open ahoka opened 7 years ago

ahoka commented 7 years ago

Hi,

I have a problem where I want to use a library which calls HttpClient.SendAsync outside of ASP, but after registering Microsoft.Diagnostics.Correlation to the application server. The problem seems to be that there is no CorrelationContext in that situation:

Unhandled Exception: System.AggregateException: One or more errors occurred. (Value cannot be null.
Parameter name: context) ---> System.ArgumentNullException: Value cannot be null.
Parameter name: context
   at Microsoft.Diagnostics.Correlation.Common.Http.CorrelationContextInjector.UpdateRequest(CorrelationContext context, HttpRequestMessage request)
   at Microsoft.Diagnostics.Correlation.AspNetCore.Instrumentation.Internal.HttpDiagnosticListenerObserver`1.OnNext(KeyValuePair`2 value)
   at System.Diagnostics.DiagnosticListener.Write(String name, Object value)
   at System.Net.Http.HttpHandlerDiagnosticListenerExtensions.LogHttpRequestCore(DiagnosticListener diagnosticListener, HttpRequestMessage request)
   at System.Net.Http.HttpHandlerDiagnosticListenerExtensions.LogHttpRequest(DiagnosticListener this, HttpRequestMessage request)
   at System.Net.Http.CurlHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpMessageInvoker.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at Consul.PutRequest`1.<Execute>d__7.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Common.ServiceDiscovery.Consul.ConsulServiceDiscovery.<Register>d__3.MoveNext()
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at AssetService.Host.Kestrel.Program.Main(String[] args)

I think that it should just silently ignore it in this case.

lmolkova commented 7 years ago

Hi,

I'm sorry for the delay, I was out of the office for several days. Could you please share how you configure your application to use correlation?

Correlation provides 3 parts: parsing context from the incoming request, injecting it to outgoing request and retaining it for the request life-cycle. And probably the problem is that first part is missing: nothing handles incoming requests and creates context. In this case there is nothing to inject to outgoing header. In non-ASP.NET case, we expect that context will be set by user code. You may create CorrelationContext and set it with ContextResolver. I think correlation should throw more meaningful exception if no context is set, to indicate that it's not supported. In your case, why would you want to use correlation without incoming requests handling?

ahoka commented 7 years ago

Hi,

I have the following code in ASP's Configure method:

            var configuration = new AspNetCoreCorrelationConfiguration();
            ContextTracingInstrumentation.Enable(configuration);

I think the problem here is that not all outgoing requests come from an incoming request. In my case I would like to register my micro-service during application startup over HTTP.

I think Diagnostics.Correlation should just quietly ignore the HttpClient in these cases, because this way it breaks 3rd party clients used in my application.