microsoft / reverse-proxy

A toolkit for developing high-performance HTTP reverse proxy applications.
https://microsoft.github.io/reverse-proxy
MIT License
8.48k stars 835 forks source link

Add telemetry to application insights #708

Closed luizfbicalho closed 2 years ago

luizfbicalho commented 3 years ago

What should we add or change to make your life better?

Would be interesting to make integtation with application insights easier, like adding all the telemetry to azure application inghts

Why is this important to you?

Because we can use all of the appliction insights infrastructure to monitor the proxy.

Tratcher commented 3 years ago

YARP operates as an ASP.NET Core component. What do you need from YARP beyond Application Insights' existing ASP.NET Core integration?

luizfbicalho commented 3 years ago

The application insights log doens't show the proxy request

For example, my proxy calls an api to get a valid token for this request, if I open the application insights log, it show the token request, but it doesn't show the proxy request to the destination.

Maybe I did something wrong in the telemetry configuration

            services.AddReverseProxy()
                .LoadFromConfig(Configuration.GetSection("ReverseProxy"));
            services.AddTelemetryListeners();

            services.AddApplicationInsightsTelemetry(Configuration["APPINSIGHTS_CONNECTIONSTRING"]);
Tratcher commented 3 years ago

@MihaZupan do you think this is because of https://github.com/dotnet/runtime/issues/31261?

karelz commented 3 years ago

Triage: We will need some guidance/docs by @MihaZupan

luizfbicalho commented 3 years ago

I didn't understand @karelz and @MihaZupan , is there something that I can do to log all of the information about the proxy information on the fowarded request? What does Type: documentation means?

MihaZupan commented 3 years ago

Not currently, we will need to add more instrumentation as described in https://github.com/microsoft/reverse-proxy/issues/776#issuecomment-786241459 to support AppInsights out of the box.

Out-of-the-box you will currently only see the incoming requests to the proxy, and we will forward the trace information in the request to the backend, so if you have other services participating in the distributed trace, those will light up as well.

luizfbicalho commented 3 years ago

Thanks, if it doesn't work out of the box, is there something that I can do? some extension would help? I could try to develop something if I had the direction

brentnewbury-work commented 3 years ago

Is there any update to this? I'd also like to know if there's anything we can do now to light up dependency tracking of proxied requests in AppInsights.

MihaZupan commented 3 years ago

It is actively being worked on in runtime. Relevant issue: https://github.com/dotnet/runtime/issues/50658

If you are blocked, see outgoing dependencies tracking docs on how you could instrument your handler. AppInsights-specific example Handler.cs (untested)

samsp-msft commented 3 years ago

Stepping back a bit here, are you looking for distributed tracing - which is what most of this thread is about, or other logging from the proxy being consumed by app-insights?

brentnewbury-work commented 3 years ago

For me, I'm looking for dependency tracing. I want to be able to go into App Insights in the proxy, and see the proxied requests, see what database calls were made in the backend, etc. We can normally do that with our other App Services, but for some reason the proxy isn't doing this at all.

MihaZupan commented 3 years ago

I believe all distributed tracing needs can be addressed via the DistributedContextPropagator APIs added in 6.0.

I recommend we remove the ActivityContextPropagator from YARP when moving to 6.0 (#1055) as DistributedContextPropagator exposes a superset of functionality and YARP's current default does not match Runtime behavior.

Do we care about enhancing ActivityContextPropagator for < 6.0? Workarounds exist and it would ultimately be deprecated.

Removing the milestone to bring this to attention in next triage.

Tratcher commented 3 years ago

YARP's current default does not match Runtime behavior.

Is this fixable for 1.0?

MihaZupan commented 3 years ago

YARP's current default does not match Runtime behavior.

Is this fixable for 1.0?

Yes, the default I had in mind specifically was BaggageAndCorrelationContext here https://github.com/microsoft/reverse-proxy/blob/bbd9dbcd844dfa01a7a3d1a46c0c599d974199c3/src/ReverseProxy/Forwarder/ForwarderHttpClientFactory.cs#L131

Default runtime behavior (at least for now) is closer to ActivityContextHeaders.CorrelationContext.

karelz commented 3 years ago

Folks affected by this: Is it possible for you to migrate to .NET 6.0 where this is solved? Note: that .NET 6 is near RC1 which will have go-live license (support with some fine print).

karelz commented 3 years ago

Triage: Some docs may be still needed -- @MihaZupan to check it out, then close.

MihaZupan commented 3 years ago

Note: From some manual testing, YARP will have to remove tracing headers from the request in order for the E2E to work well (HttpClient internals will no-op if already present).

samsp-msft commented 2 years ago

It may make sense to be a 1.1 feature to go with .NET 6?

Bouke commented 2 years ago

Folks affected by this: Is it possible for you to migrate to .NET 6.0 where this is solved? Note: that .NET 6 is near RC1 which will have go-live license (support with some fine print).

Yarp RC1 no longer includes the traceparent header in requests to backends. Is this expected behaviour? And if so: how can I get it back in projects targeting .NET 5?

Tratcher commented 2 years ago

@Bouke see https://github.com/microsoft/reverse-proxy/discussions/1316