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

BFF's SocketsHttpHandler configuration out of sync with Yarp's #194

Open
ArturDorochowicz opened this issue May 28, 2024 · 1 comment
Open
Assignees
Labels
area/bff/yarp BFF Yarp integrations
Milestone

Comments

@ArturDorochowicz
Copy link
Contributor

Which version of Duende BFF are you using?
2.2.0

Which version of .NET are you using?
net8.0

Describe the bug

At work we use BFF in an application that uses Microsoft Application Insights (AppInsights). Since introducing BFF, it's always reported telemetry nested incorrectly. What we'd expect is something like this:

  • outgoing Ajax request to BFF host (from browser)
    • incoming request to BFF host (from BFF host)
      • outgoing request to remote api (from BFF host)
        • incoming request to remote api (from remote api)

Instead, what we actually get is this (i.e. incoming requests are siblings and there's no outgoing request):

  • outgoing Ajax request to BFF host
    • incoming request to BFF host
    • incoming request to remote api

This has puzzled me, because Yarp docs say that it works out of the box (https://microsoft.github.io/reverse-proxy/articles/distributed-tracing.html). Only recently have I realized that it is dependent on using Yarp's created SocketsHttpHandler (in ForwarderHttpClientFactory) and BFF doesn't do that.

So it turns out that BFF's configuration for SocketsHttpHandler is out of sync with Yarp.

In Yarp 2.1.0 we have additionally ActivityHeadersPropagator (which is the missing piece behind correct distributed tracing) and ConnectTimeout.
https://github.com/microsoft/reverse-proxy/blob/v2.1.0/src/ReverseProxy/Forwarder/ForwarderHttpClientFactory.cs#L44-L54

And in Yarp 2.2.0 (currently preview) there's also EnableMultipleHttp2Connections.
https://github.com/microsoft/reverse-proxy/blob/v2.2.0-preview1/src/ReverseProxy/Forwarder/ForwarderHttpClientFactory.cs#L44-L55

To Reproduce

Steps to reproduce the behavior.

Expected behavior

A clear and concise description of what you expected to happen.

Log output/exception with stacktrace

data

Additional context

In my opinion BFF should employ Yarp's classes here and not recreate this, but I will create separate issue for that.

@AndersAbel
Copy link
Member

Thank you for your detailed bug report. It indeed looks like we are not propagating the activity headers correctly.

I'm transferring this issue to the BFF repository and marking it as a bug.

@AndersAbel AndersAbel transferred this issue from DuendeSoftware/Support May 30, 2024
@AndersAbel AndersAbel added the bug label May 30, 2024
@brockallen brockallen added this to the 3.0.0 milestone Jun 4, 2024
@brockallen brockallen assigned damianh and unassigned AndersAbel Aug 22, 2024
@damianh damianh added area/bff/yarp BFF Yarp integrations and removed bug labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bff/yarp BFF Yarp integrations
Projects
None yet
Development

No branches or pull requests

5 participants