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

Add telemetry to application insights #708

Closed
luizfbicalho opened this issue Feb 2, 2021 · 20 comments · Fixed by #1342
Closed

Add telemetry to application insights #708

luizfbicalho opened this issue Feb 2, 2021 · 20 comments · Fixed by #1342
Assignees
Labels
Type: Documentation Improvements or additions to documentation
Milestone

Comments

@luizfbicalho
Copy link

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.

@luizfbicalho luizfbicalho added the Type: Idea This issue is a high-level idea for discussion. label Feb 2, 2021
@Tratcher
Copy link
Member

Tratcher commented Feb 2, 2021

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

@luizfbicalho
Copy link
Author

luizfbicalho commented Feb 2, 2021

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
Copy link
Member

Tratcher commented Feb 2, 2021

@MihaZupan do you think this is because of dotnet/runtime#31261?

@karelz
Copy link
Member

karelz commented Feb 16, 2021

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

@karelz karelz added this to the YARP 1.0.0 milestone Feb 16, 2021
@karelz karelz added Type: Documentation Improvements or additions to documentation and removed Type: Idea This issue is a high-level idea for discussion. labels Feb 16, 2021
@luizfbicalho
Copy link
Author

luizfbicalho commented Mar 10, 2021

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
Copy link
Member

Not currently, we will need to add more instrumentation as described in #776 (comment) 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
Copy link
Author

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
Copy link

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
Copy link
Member

It is actively being worked on in runtime. Relevant issue: dotnet/runtime#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
Copy link
Contributor

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
Copy link

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
Copy link
Member

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.

@MihaZupan MihaZupan removed this from the YARP 1.0.0 milestone Aug 20, 2021
@Tratcher
Copy link
Member

YARP's current default does not match Runtime behavior.

Is this fixable for 1.0?

@MihaZupan
Copy link
Member

MihaZupan commented Aug 20, 2021

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

var activityContextHeaders = context.NewConfig.ActivityContextHeaders.GetValueOrDefault(ActivityContextHeaders.BaggageAndCorrelationContext);

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

@karelz
Copy link
Member

karelz commented Aug 26, 2021

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
Copy link
Member

karelz commented Sep 2, 2021

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

@karelz karelz added this to the YARP 1.0.0 milestone Sep 2, 2021
@MihaZupan
Copy link
Member

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
Copy link
Contributor

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

@samsp-msft samsp-msft added the samsp_list Personal tag used when reviewing issues for further discussion label Oct 8, 2021
@Bouke
Copy link
Contributor

Bouke commented Oct 25, 2021

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
Copy link
Member

@Bouke see #1316

@samsp-msft samsp-msft removed the samsp_list Personal tag used when reviewing issues for further discussion label Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants