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

Consider removing TempSensorBreadcrumbsIntegration from default integrations #2395

Closed
romtsn opened this issue Nov 28, 2022 · 7 comments
Closed

Comments

@romtsn
Copy link
Member

romtsn commented Nov 28, 2022

Description

We had a few reports about some Chinese stores flagging this integration as violating their policies, and therefore not allowing the apps to the stores.

We could consider removing it from default integrations if the impact is high, or we could introduce a flag which specifically disables this integration (at the moment it sits behind enableSystemBreadcrumbs flag, which either disabled or enables all integrations related to system events)

@markushi
Copy link
Member

In a first version, let's provide a flag to disable the integration. Also add a section to the trouble shooting guide.

@Sky24n
Copy link

Sky24n commented Mar 22, 2023

@romtsn Why use a TYPE_AMBIENT_TEMPERATURE?

@romtsn
Copy link
Member Author

romtsn commented Mar 22, 2023

@Sky24n I don't know, but my guess would be, developers might want to know if ambient temperature was high, which can explain high battery temperature as well. But I'll defer this to @marandaneto as he was the one implementing it

@marandaneto
Copy link
Contributor

https://developer.android.com/reference/android/hardware/Sensor#TYPE_TEMPERATURE

This constant was deprecated in API level 15.
use Sensor.TYPE_AMBIENT_TEMPERATURE instead.

@Sky24n
Copy link

Sky24n commented Mar 22, 2023

https://developer.android.com/reference/android/hardware/Sensor#TYPE_TEMPERATURE

This constant was deprecated in API level 15.
use Sensor.TYPE_AMBIENT_TEMPERATURE instead.

Why introduce the function of obtaining temperature? TempSensorBreadcrumbsIntegration

@marandaneto
Copy link
Contributor

Maybe the solution here is going with https://developer.android.com/reference/android/os/BatteryManager#EXTRA_TEMPERATURE
I have tested this in the past and most devices didn't return a value, using https://developer.android.com/reference/android/hardware/Sensor#TYPE_TEMPERATURE had better results but it was deprecated in favor of https://developer.android.com/reference/android/hardware/Sensor#TYPE_AMBIENT_TEMPERATURE

@Sky24n feel free to submit a PR adding a flag as suggested by #2395 (comment)

@marandaneto
Copy link
Contributor

marandaneto commented Mar 22, 2023

We already use extra_temperature btw

private @Nullable Float getBatteryTemperature(final @NotNull Intent batteryIntent) {
try {
int temperature = batteryIntent.getIntExtra(EXTRA_TEMPERATURE, -1);
if (temperature != -1) {
return ((float) temperature) / 10; // celsius
}
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, "Error getting battery temperature.", e);
}
return null;
}
but for the battery and not the device itself.

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

No branches or pull requests

6 participants