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

[AbhiPrasad]

Project Board

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

Description

In https://github.com/getsentry/team-webplatform-meta/issues/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
- [x] Python SDK
- [x] JS SDKs
- [x] PHP SDKs
- [ ] https://github.com/getsentry/sentry-go/issues/713
- [ ] https://github.com/getsentry/sentry-dotnet/issues/2606
- [ ] https://github.com/getsentry/sentry-ruby/issues/2105
- [ ] https://github.com/getsentry/sentry-java/pull/2896
- [x] Android SDK (solved with ^)
- [ ] https://github.com/getsentry/sentry-dart/issues/1609
- [ ] https://github.com/getsentry/sentry-cocoa/issues/3234
- [ ] https://github.com/getsentry/sentry-php/pull/1581
- [ ] https://github.com/getsentry/sentry-symfony/pull/756

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 https://github.com/getsentry/team-sdks/issues/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 https://github.com/getsentry/relay/pull/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 🤣 .