-
Notifications
You must be signed in to change notification settings - Fork 10k
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 information to indicate a SignalR request is long running #29699
Comments
Thanks for contacting us. |
We're also considering using response headers (and possibly request headers if we want to wait to flush response headers for long polling or something) to indicate that a request is expected to be long-running. An upside to using a header instead of (or maybe in addition to) adding info to the current Activity is that it could also be used by reverse proxies.
|
My current proposal is that we don't take the current PR to add a response header. Given that Application Insights didn't find it useful and I know of nobody else clamoring for it, I don't think it's worth it. I have two proposals on how to proceed here listed in the order of my preference: Close the span prematurelyFor example if When SignalR now uses the established WebSocket as a channel, it can create a new Span for each RPC invocation (with possibly a Span link to the original associated with the incoming HTTP request). While this bit this optional for now, this is how envision it working if we went all the way and made SignalR span-aware. endpoints.MapGet("/", async context =>
{
using var webSocket = await context.WebSockets.AcceptWebSocketAsync();
context.Features.Get<IActivityFeature>().Activity.Stop();
await webSocket.ReceiveAsync(...);
// do more stuff
}); For non-websocket transports, close the span whenever the determination is made that this request will be long-running. While this helps Add a new attribute (
|
@noahfalk Do you know the GitHub aliases of other APM vendors? Can we tag them on here for feedback? |
@davmason would know the best : ) |
Off the top of my head @pjanotti @macrogreg @zacharycmontoya @discostu105 @d-schneider are all people I engage with frequently. It would make sense to add a post at dotnet/runtime#9305 too if you want more APM vendors to see it. |
@halter73, @BrennanConroy, @davidfowl, and I met to discuss this. We've decided:
|
To allow APMs (e.g., Application Insights) to treat long running SignalR requests differently from the their request statistics, we should set some information (I presume via an
Activity.Tag
?) on current Activity to indicate a request is long running.Edit: After more discussion with App Insights, we think headers will work.
cc @cijothomas
The text was updated successfully, but these errors were encountered: