Return transaction for Sentry.getSpan when global hub mode is enabled by markushi · Pull Request #2855 · getsentry/sentry-java

markushi · 2023-07-20T06:54:39Z

📜 Description

💡 Motivation and Context

Just like Cocoa, always return the root transaction instead of the last active span. This is only applied if globalHubMode is enabled.

💚 How did you test it?

📝 Checklist

I reviewed the submitted code.
I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

github-actions · 2023-07-20T06:55:50Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against a1914e9

github-actions · 2023-07-20T07:11:33Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	358.48 ms	361.78 ms	3.30 ms
Size	1.72 MiB	2.26 MiB	547.97 KiB

Previous results on branch: feat/get-span-root-span

Startup times

Revision	Plain	With Sentry	Diff
`015f93f`	314.22 ms	348.98 ms	34.75 ms
`bfee4ed`	342.45 ms	369.92 ms	27.47 ms
`e9072f6`	295.55 ms	371.12 ms	75.57 ms
`3d502fb`	316.02 ms	358.81 ms	42.79 ms
`6cd0641`	312.44 ms	359.16 ms	46.72 ms
`9ef1fa6`	324.19 ms	374.00 ms	49.81 ms
`e068d53`	307.73 ms	340.84 ms	33.11 ms

App size

Revision	Plain	With Sentry	Diff
`015f93f`	1.72 MiB	2.26 MiB	547.97 KiB
`bfee4ed`	1.72 MiB	2.29 MiB	575.30 KiB
`e9072f6`	1.72 MiB	2.29 MiB	575.26 KiB
`3d502fb`	1.72 MiB	2.29 MiB	576.58 KiB
`6cd0641`	1.72 MiB	2.26 MiB	547.97 KiB
`9ef1fa6`	1.72 MiB	2.29 MiB	576.28 KiB
`e068d53`	1.72 MiB	2.29 MiB	575.30 KiB

codecov · 2023-07-20T07:27:15Z

Codecov Report

Patch coverage is 87.53% of modified lines.

Files Changed	Coverage
...racing/SentrySpanClientHttpRequestInterceptor.java	`0.00%`
...ebflux/SentryWebFilterWithThreadLocalAccessor.java	`0.00%`
...racing/SentrySpanClientHttpRequestInterceptor.java	`0.00%`
sentry/src/main/java/io/sentry/Breadcrumb.java	`ø`
sentry/src/main/java/io/sentry/HostnameCache.java	`ø`
sentry/src/main/java/io/sentry/IHub.java	`ø`
.../src/main/java/io/sentry/JsonObjectSerializer.java	`ø`
sentry/src/main/java/io/sentry/NoOpHub.java	`0.00%`
...ry/src/main/java/io/sentry/ProfilingTraceData.java	`ø`
.../main/java/io/sentry/ProfilingTransactionData.java	`ø`
... and 89 more

📢 Thoughts on this report? Let us know!.

romtsn · 2023-07-25T18:35:09Z

I think simply changing this on the Sentry.java level is probably not enough, because we are using Hub.getSpan() in our own auto-instrumentation, and those will still be returning the latest span and not the root one.

Also, we've talked about introducing another method getLatestSpan or something like that, which we can use in our DB instrumentation to not break the parent-child relationship for Room+SQLite spans, can we also do that in this PR please?

markushi · 2023-07-26T09:42:48Z

I think simply changing this on the Sentry.java level is probably not enough, because we are using Hub.getSpan() in our own auto-instrumentation, and those will still be returning the latest span and not the root one.

As of now globalhub mode is not stored anywhere but Sentry.java so the hub itself doesn't know if it's global or not - let me look into this.

Also, we've talked about introducing another method getLatestSpan or something like that, which we can use in our DB instrumentation to not break the parent-child relationship for Room+SQLite spans, can we also do that in this PR please?

Yes, we thought about this, on it...

romtsn · 2023-07-26T13:17:11Z

As of now globalhub mode is not stored anywhere but Sentry.java so the hub itself doesn't know if it's global or not - let me look into this.

We could just use hub.getTransaction() in our integrations maybe, where applicable

romtsn

LGTM, but I think we also have to do it for Apollo, at least on Android? Is the apollo integration used server-side as well @adinauer? But we have the same problem for the okhttp integration now anyway, if someone is using it on the server alongside the Java SDK

adinauer · 2023-08-09T13:26:29Z

Is the apollo integration used server-side as well

It's a client only, we have support for graphql-java on server side which is used by Netflix DGS as well as spring-graphql.

Or do you mean can this be used in a backend application? That's a yes.

Not really sure about details here, maybe we can do a quick call if you need my input here so I can better grasp implications.

markushi · 2023-08-09T14:28:55Z

Let's change the Sentry.getSpan() behavior for Android only. In case this comes up for Desktop/Cli let's deal with this at a later point.

sentry/src/main/java/io/sentry/Sentry.java

…-java into feat/get-span-root-span

romtsn · 2023-08-30T08:10:30Z

@markushi I think the PR is still missing SentryApolloInterceptor and SentryApollo3Interceptor from the list 😅

romtsn

looks good now, thanks for addressing back-and-forths 🎉

markushi added 2 commits July 20, 2023 08:50

Return transaction for Sentry.getSpan when global hub mode is enabled

473c665

Merge branch 'feat/7.0.0' into feat/get-span-root-span

2c1ab67

markushi requested review from adinauer, romtsn and stefanosiano as code owners July 20, 2023 06:54

Fix changelog

c4df37e

Add missing tests

c934c6f

markushi mentioned this pull request Jul 28, 2023

Use Hub.getSpan instead of Sentry.getSpan getsentry/sentry-android-gradle-plugin#535

Closed

4 tasks

markushi added 3 commits August 4, 2023 08:06

Merge branch 'feat/7.0.0' into feat/get-span-root-span

54b9ae1

Use root span for OkHttp+FileIo integrations

348fb56

Merge branch 'feat/7.0.0' into feat/get-span-root-span

6dd3b53

romtsn approved these changes Aug 9, 2023

View reviewed changes

markushi added 2 commits August 11, 2023 09:04

return root span only on Android

909262a

Merge branch 'feat/7.0.0' into feat/get-span-root-span

a9dadc4

romtsn reviewed Aug 11, 2023

View reviewed changes

sentry/src/main/java/io/sentry/Sentry.java Show resolved Hide resolved

markushi added 4 commits August 29, 2023 08:32

Merge branch 'feat/7.0.0' into feat/get-span-root-span

e902006

Merge branch 'feat/get-span-root-span' of github.com:getsentry/sentry…

61d67d4

…-java into feat/get-span-root-span

Use hub.transaction on Android for FileIO/OkHttp

a1fc5ac

Fix flaky tests

aa6e1af

Use root transaction for Apollo spans on Android

a1914e9

romtsn approved these changes Aug 31, 2023

View reviewed changes

markushi merged commit cd268a3 into feat/7.0.0 Aug 31, 2023

markushi deleted the feat/get-span-root-span branch August 31, 2023 12:15

romtsn mentioned this pull request Sep 6, 2023

Align getSpan method from Scope on Android/Java and iOS #1828

Closed

Uh oh!

Comments

Conversation

markushi commented Jul 20, 2023

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

github-actions bot commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Previous results on branch: feat/get-span-root-span

Startup times

App size

Uh oh!

codecov bot commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

romtsn commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markushi commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romtsn commented Jul 26, 2023

Uh oh!

romtsn left a comment

Choose a reason for hiding this comment

Uh oh!

adinauer commented Aug 9, 2023

Uh oh!

markushi commented Aug 9, 2023

Uh oh!

Uh oh!

romtsn commented Aug 30, 2023

Uh oh!

romtsn left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jul 20, 2023 •

edited

Loading

github-actions bot commented Jul 20, 2023 •

edited

Loading

codecov bot commented Jul 20, 2023 •

edited

Loading

romtsn commented Jul 25, 2023 •

edited

Loading

markushi commented Jul 26, 2023 •

edited

Loading