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.

Sign up for GitHub

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

Jump to bottom

Make span eligible for Requests #3334

Closed
bitsandfoxes opened this issue Apr 26, 2024 · 2 comments · Fixed by #3357
Closed

Make span eligible for Requests #3334

bitsandfoxes opened this issue Apr 26, 2024 · 2 comments · Fixed by #3357
Assignees
@jamescrosswell
Labels
Product: Performance

Comments

@bitsandfoxes
Copy link
Contributor

bitsandfoxes commented Apr 26, 2024 •
edited by jamescrosswell

https://docs.sentry.io/product/performance/requests/#span-eligibility

Review

SentryHttpMessageHandler / SentryGraphQLHttpMessageHandler

  • ✅ The op field is set to "http.client"
  • ✅ The description field contains the HTTP method and the full URL of the request (e.g., "GET http://my.service.io/some-data").
  • ❌ If the request is to a relative URL (e.g., "GET /data.json"), the server.address span data value should be set to the server address (e.g., "my.service.io").
  • ✅ The http.response.status_code contains the HTTP response status code (e.g., "200").

OpenTelemetry instrumentation

  • ❌ The op field is set to http.client
    • The operation name is being set by OpenTelemetry to Microsoft.AspNetCore.Hosting.HttpRequestIn
  • ❌ The description field contains the HTTP method and the full URL of the request (e.g., "GET http://my.service.io/some-data").
  • ✅ If the request is to a relative URL (e.g., "GET /data.json"), the server.address span data value should be set to the server address (e.g., "my.service.io").
  • ✅ The http.response.status_code contains the HTTP response status code (e.g., "200").
@jamescrosswell
Copy link
Collaborator

@bitsandfoxes added some details to the issue description.

@bitsandfoxes bitsandfoxes changed the title Validate span eligibility for Requests Make span eligible for Requests May 6, 2024
@jamescrosswell
Copy link
Collaborator

jamescrosswell commented May 9, 2024 •
edited

So it turns out this scenario isn't actually possible in .NET:

❌ If the request is to a relative URL (e.g., "GET /data.json"), the server.address span data value should be set to the server address (e.g., "my.service.io").

The only way to use Relative URLs with HttpClient is like this:

using var client = new HttpClient(sentryHandler);
client.BaseAddress = new Uri("http://localhost");
var uri = new Uri("/hello", UriKind.Relative);
await client.GetAsync(uri);

The call to await client.GetAsync(uri) will throw an exception if you're using a Uri with UriKind.Relative and you haven't set the HttpClient.BaseAddress. On the other hand, if you provide the base address, this gets combined with the relative url to create an absolute Uri.

No matter which way you do it then, by the time we see the Uri in the SentryMessageHandler, it will be an absolute URI.

I've opted to always set the server.address span data, which I think is what's happening when the spans are instrumented with OpenTelemetry.

@jamescrosswell jamescrosswell mentioned this issue May 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jamescrosswell jamescrosswell

Labels
Product: Performance
Projects
.NET & Gaming
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure outgoing spans show on the Request dashboard in Sentry getsentry/sentry-dotnet
2 participants
@jamescrosswell @bitsandfoxes