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.

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

Cannot remove default integrations, i.e. TempSensorBreadcrumbsIntegration #2355

Closed
fzyzcjy opened this issue Nov 12, 2022 · 17 comments
Closed

Comments

@fzyzcjy
Copy link

fzyzcjy commented Nov 12, 2022

Integration

sentry-android

Build System

Gradle

AGP Version

latest

Proguard

Enabled

Version

latest

Steps to Reproduce

Hi, I have to remove TempSensorBreadcrumbsIntegration integration, because otherwise app is rejected by stores since it reads privacy data. However, I have not seen a way to remove it (there is no things as removeIntegration etc).

Expected Result

Actual Result

@romtsn
Copy link
Member

romtsn commented Nov 14, 2022

I think you can simply do

SentryAndroid.init(context) { options ->
  options.integrations.removeIf { it is TempSensorBreadcrumbsIntegration }
}

Also, I'm curious which stores are rejecting the app? Is it Google Play or some others? We might need to remove it from default integrations then

@fzyzcjy
Copy link
Author

fzyzcjy commented Nov 14, 2022

I will have a try, thanks! (Not checked it yet so not sure whether integrations is mutable or not)

Also, I'm curious which stores are rejecting the app? Is it Google Play or some others? We might need to remove it from default integrations then

Others, indeed tencent store, which seem to have >1 billion target users (where google play is banned)

@romtsn
Copy link
Member

romtsn commented Nov 14, 2022

yep, it's mutable.

Chinese stores are very restrictive, we have a few issues opened about reading too much data 🙈 Maybe we need to think of a chinese flavour of the SDK without all those sensitive default integrations/data collectors, if there's a demand. I might open an issue to collect feedback 👍

Gonna close this one, since it should be resolved by using remove/removeIf.

@romtsn romtsn closed this as not planned Won't fix, can't repro, duplicate, stale Nov 14, 2022
Repository owner moved this from Needs More Information to Done in Mobile & Cross Platform SDK Nov 14, 2022
@fzyzcjy
Copy link
Author

fzyzcjy commented Nov 14, 2022

Thank you!

@marandaneto
Copy link
Contributor

Dupe of getsentry/sentry-dart#1129
@fzyzcjy next time I'd ask you to open an issue on a single repo otherwise you trigger multiple teams for the very same issue, thanks for understanding.

@fzyzcjy
Copy link
Author

fzyzcjy commented Nov 14, 2022

@marandaneto Get it, sorry for that. Originally I thought this is Android while that is Flutter...

By the way, the two issues have different answers that are both helpful and needed to be combined indeed :)

@marandaneto
Copy link
Contributor

@marandaneto Get it, sorry for that. Originally I thought this is Android while that is Flutter...

By the way, the two issues have different answers that are both helpful and needed to be combined indeed :)

That's what I meant by:

remove the SystemEventsBreadcrumbsIntegration from the integrations list manually on the Android SDK.

Here it is the very same answer, but with a code snippet =P

@fzyzcjy
Copy link
Author

fzyzcjy commented Nov 14, 2022

@marandaneto You are right =P

P.S. My mind seemed to be wrong to Android before, because IIRC the dart integrations are not removeable (use something like UnmodifiableListView the last time I check it) and I wrongly thought it was Android

@marandaneto
Copy link
Contributor

@marandaneto You are right =P

P.S. My mind seemed to be wrong to Android before, because IIRC the dart integrations are not removeable (use something like UnmodifiableListView the last time I check it) and I wrongly thought it was Android

You can mutate the Flutter integrations as well.
SentryOptions#removeIntegration(integration)

@fzyzcjy
Copy link
Author

fzyzcjy commented Nov 14, 2022

Totally agree. Not sure why I was so stupid when checking it :)

@fzyzcjy
Copy link
Author

fzyzcjy commented Feb 15, 2023

I think you can simply do

SentryAndroid.init(context) { options ->
  options.integrations.removeIf { it is TempSensorBreadcrumbsIntegration }
}

Also, I'm curious which stores are rejecting the app? Is it Google Play or some others? We might need to remove it from default integrations then

Well, there is error on some devices:

E/AndroidRuntime(18564): Caused by: java.lang.UnsupportedOperationException
E/AndroidRuntime(18564): 	at java.util.concurrent.CopyOnWriteArrayList$CowIterator.remove(CopyOnWriteArrayList.java:814)
E/AndroidRuntime(18564): 	at java.util.Collection.removeIf(Collection.java:402)

So just a reminder for those who wants to use this API :)

@romtsn
Copy link
Member

romtsn commented Feb 15, 2023

oh 🙈 removeIf is probably using iterator under the hood, which is not allowed in CopyOnWriteArrayList. Then the code would be slightly different to make it work:

SentryAndroid.init(context) { options ->
  val tempSensorIntegration = options.integrations.indexOfFirst { it is TempSensorBreadcrumbsIntegration }
  options.integrations.removeAt(tempSensorIntegration)
}

@fzyzcjy
Copy link
Author

fzyzcjy commented Feb 15, 2023

I used removeAll by builtin kotlin and it worked well as well. removeIf seems to have bug because it is not supported in android <24 (while ok for >=24)

@fzyzcjy
Copy link
Author

fzyzcjy commented Jul 6, 2023

@marandaneto Well the fix only lasts for 4 days... and it does not work: #2821

@marandaneto
Copy link
Contributor

Oh no, definitely a regression, @romtsn @markushi ^

@stefanosiano
Copy link
Member

Hey @fzyzcjy this issue was fixed in the release 6.25.1.
Feel free to reopen this issue in case something goes wrong

@fzyzcjy
Copy link
Author

fzyzcjy commented Jul 12, 2023

@stefanosiano Looks great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants