Project: [Starfish] Add http.request.method to span data

[AbhiPrasad]

Project Board

https://github.com/orgs/getsentry/projects/150/views/1

Description

In getsentry/team-webplatform-meta#60 we added the necessary span data changes to the Python and JS SDKs for starfish. This issue tracks adding these fields to the remaining SDKs.

These include the following:

1. HTTP information

The primary require is to add a new field to the span data field, http.request.method, representing the http method (GET, POST, PUT).

If possible, adding any of the OpenTelemetry common http attributes should be attempted, but is not a requirement.

SDKs to update

Beta Give feedback

1. Python SDK
2. JS SDKs
3. PHP SDKs
4. [Starfish] Add starfish v1 attribute http.request.method to span data sentry-go#713
   Status: Backlog +1
5. [Starfish] Add starfish v1 attribute http.request.method to span data sentry-dotnet#2606
   +0
   
6. [Starfish] Add starfish v1 attribute http.request.method to span data sentry-ruby#2105
   +0
   
7. Send http.request.method in span data sentry-java#2896
   +0
8. Android SDK (solved with ^)
9. [Starfish] Add starfish v1 attributes to http span data sentry-dart#1609
   Platform: Dart +1
   
10. [Starfish] Add starfish v1 http attributes to span data sentry-cocoa#3234
    Platform: Cocoa +1
11. Add starfish v1 attributes to span data/breadcrumbs sentry-php#1581
    Type: Improvement +1
    
12. Add starfish v1 attributes to span data/breadcrumbs sentry-symfony#756
    Type: Improvement +1
    

RFC

No response

Slack-Channel

#discuss-starfish

Notion Document(s)

https://www.notion.so/sentry/0333024900d14a3f8dd398a36ddb55bd?v=681d28a5fa2a4c4aa6437e49a087a241

Stakeholder(s)

@alexjillard

Team(s)

Web Backend

[AbhiPrasad]

Previously this task had the requirement to add cache attributes like below (documenting for posterity). Because we are prioritizing the the database module, this requirement is now being dropped. Cache information will be added to another task.

3. Cache information

Add cache.hit and cache.item_size as per: https://develop.sentry.dev/sdk/performance/span-data-conventions/#web-server

Any call to cache that doesn't return data (null) is treated as a miss. We define a span that has a cache hit as having the span data field cache.hit as true. Might require SDK to patch cache abstractions of framework.

Also, add a field to cache spans that defines how big the item that is being get/set: cache.item_size. This is an integer and should be in bytes.

[AbhiPrasad]

moved db.system task to #19 so that all database specific attributes were scoped there.

[AbhiPrasad]

otel ended up renaming http.method to http.request.method. Updated the task for this, and opened getsentry/relay#2396 to make relay recognize it.

[adinauer]

@AbhiPrasad do we only need http.request.method for HTTP clients? If we want this for server side transactions as well, where should we put the method? I don't see method in https://github.com/getsentry/relay/blob/master/relay-general/src/protocol/contexts/response.rs . We recently had a similar question regarding status code and that's where it ended up for transactions.

[AbhiPrasad]

do we only need http.request.method for HTTP clients

I think it makes sense to add method to the Response context. Let me open up a PR to Relay.

[AbhiPrasad]

Actually for a response the status code should be good enough for this. We already have the method on the request context https://github.com/getsentry/relay/blob/fd955bc85ccd80fb4763bb04254b4d4a7c9d7200/relay-general/src/protocol/request.rs#L443-L444

[adinauer]

Turns out we already put it on the request in the Java SDK 🤣 .