chore: Add http requests to custom instrumentation

[tommasini]

Description

This PR adds custom instrumentation for http requests.

performance e2e: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/17c729b7-7979-4d02-bc56-de709b38bac8?tab=workflows

Android QA: https://app.bitrise.io/build/fbbcf003-58c5-4771-bfef-391f8e8cc84d?tab=log

Related issues

Fixes:

Manual testing steps

1. Go to this page...

Screenshots/Recordings

IOS:

Android:

Before

After

Pre-merge author checklist

• I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 5398f3e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c9754da4-3f45-4c8b-996e-9fcfc3e4e128

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[MajorLift]

• How would you feel about including the full url in the name like the automatically-generated spans, and then adding the hostname as a tag? That way we can filter by hostname without losing extra information about endpoints and query strings.

• Also, regarding span operation names, Sentry recommends the following:

Sentry maintains a list of well known span operations and it is recommended that you use one of those operations if it is applicable to your span.
https://docs.sentry.io/platforms/react-native/tracing/instrumentation/custom-instrumentation/#adding-span-operations-op

Taking that into consideration, how about e.g. http.client and http.client.no_basic_functionality for the trace operation names? Prepending those with custom. also seems reasonable if it's more readable for the team.

• We could also automatically attach additional information about each request using the span.setAttribute() method as seen here:

https://docs.sentry.io/platforms/javascript/tracing/instrumentation/custom-instrumentation/requests-module/#wrap-http-requests-in-a-span

Edit: The code snippet above stores the hostname as the server.address attribute, so looks like we wouldn't need to use a tag for that.

[tommasini]

Hey @MajorLift , loved your review! More curious to understand how would we use the span.setAttribute in this scenario

I added the operations name suggestion I think it makes super sense!

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e627d42
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6d15a23a-4fba-4ad8-b514-70f25b1dc53f

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[Cal-L]

LGTM

Accident

[Cal-L]

I agree with showing the full url

[MarioAslau]

Looks great @tommasini ! 🎉

[MajorLift]

I did some digging on setAttribute, and looks like Attributes replace Tags in Sentry v8. Tags are now only supposed to be attached to scopes and not spans.

Previously, the model had the concepts of Data, Tags, and Context, which could be used for different things. However, this had two main downsides: Firstly, it was not always clear which concept should be used in a given situation. Secondly, these concepts were not displayed the same way for transactions or spans.

To address these issues, the new model only has Attributes that can be set on spans. Broadly speaking, they map to what was formerly known as Data.

If you still really need to set tags or context, you can do so on the scope before starting a span

— https://docs.sentry.io/platforms/javascript/migration/v7-to-v8/v8-new-performance-api/#attributes-vs-data-vs-tags-vs-context

This explains why the Sentry.Span interface doesn't have a setTag method in extension, which is on v8.

Mobile @sentry/types is v7, while @sentry/react-native is on the latest major version. I'm not sure whether this is a hard breaking change, but it looks like we'll have to move away from using tags eventually.

[tommasini]

I believe the reason why we move tags until now, is because they are queryable on dashboards, nevertheless, I'm not 100% sure of this, will need to re-check!

[MajorLift]

Edit: There is no need to switch to Attributes in this PR. Both extension and mobile are set up to attach Tags to Scopes. We can keep using that set-up for the time being.

I believe the reason why we move tags until now, is because they are queryable on dashboards,

It would appear that Attributes are a full replacement for Tags, meaning they should also be searchable. The docs don't seem fully updated on that point, though.

But more importantly, the migration guide seems to show span.tags and span.setTag as being removed from the Span schema, which would force us to make the switch.

https://docs.sentry.io/platforms/javascript/migration/v7-to-v8/v8-new-performance-api/#the-span-schema

There's also a functional benefit to switching:

Tags over the 200 character limit will become truncated, losing potentially important information.

Instead of using tags, it is recommended to add data to spans as attributes. Using span.setAttribute() allows to add data with arbitrary length to a span. You can add as many attributes to your span as you want.

https://docs.sentry.io/platforms/javascript/tracing/troubleshooting/#control-data-truncation

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud