open-telemetry / opentelemetry-android

OpenTelemetry Tooling for Android
Apache License 2.0
156 stars 41 forks source link

Add consumer proguard rules #83

Open LikeTheSalad opened 1 year ago

LikeTheSalad commented 1 year ago

To prevent obfuscation to break OTel Java SDK functionality in production apps.

LikeTheSalad commented 1 year ago

This is quite strange, though I just tried to use the OTel Java SDK (version 1.28.0) in an app with code obfuscation and I didn't get any errors (which I remember getting some time ago). So I'm not sure if it's due to some code refactoring in the OTel Java SDK that removed the usage of reflection or something like that. But it seems like we no longer need to add proguard rules, at least not for now.

GerardPaligot commented 1 week ago

Maybe something change from 2023 but today, if I didn't add these proguard rules in my project with OpenTelemetry Android, I got a crash on the obfuscation:

-keep class io.opentelemetry.exporter.** { *; }
-keep class io.grpc.** { *; }
-keep class com.google.protobuf.** { *; }
-keep class com.fasterxml.jackson.core.** { *; }
-dontwarn com.google.auto.value.AutoValue$CopyAnnotations
-dontwarn com.google.auto.value.extension.memoized.Memoized
-dontwarn io.grpc.**
-dontwarn com.fasterxml.jackson.core.**

Can we consider to add them in opentelemetry-android to avoid this configuration in consumers?

LikeTheSalad commented 1 week ago

Thank you for the heads-up, I'll take a look at it.

LikeTheSalad commented 1 week ago

I think the reason might be that they've enabled R8 full mode by default, I'm not fully sure what's the AGP version where the switch was made from disabled by default, though it seems like the tests we've run so far were counting on it being disabled only. I'll make some changes and add some automatic checks for R8 full mode.

marandaneto commented 1 week ago

I think the reason might be that they've enabled R8 full mode by default, I'm not fully sure what's the AGP version where the switch was made from disabled by default, though it seems like the tests we've run so far were counting on it being disabled only. I'll make some changes and add some automatic checks for R8 full mode.

https://developer.android.com/build/releases/past-releases/agp-8-0-0-release-notes#default-changes >= v8 is enabled by default

LikeTheSalad commented 1 week ago

I've created this PR to address it and also to ensure that we get compilation errors on every PR whenever there are some R8 warnings on missing rules.

I also left an open question there which concerns me a bit, so please have a look, I'm open to suggestions!

LikeTheSalad commented 1 week ago

Btw, @GerardPaligot, I've included only the rules that R8 generated during my compilation tests. I'm aware that not all the rules you've provided here are there, I'm not sure if it's because something might have changed in the latest OTel Java versions, which we always keep updated, where some of those rules might no longer be needed, or if I'm missing something, so I'd appreciate if you could double check whether those rules work for your use case.