-
Notifications
You must be signed in to change notification settings - Fork 828
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
Add Android API-friendliness checks #4505
Changes from 1 commit
bfef129
4038af0
5eb6c83
87d5930
b100913
b21032a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
plugins { | ||
`java-library` | ||
|
||
id("ru.vyarus.animalsniffer") | ||
} | ||
|
||
dependencies { | ||
add("signature", "com.toasttab.android:gummy-bears-api-21:0.3.0:coreLib@signature") | ||
} | ||
|
||
animalsniffer { | ||
sourceSets = listOf(java.sourceSets.main.get()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
import ru.vyarus.gradle.plugin.animalsniffer.AnimalSniffer | ||
|
||
plugins { | ||
id("org.xbib.gradle.plugin.jflex") | ||
|
||
id("otel.java-conventions") | ||
id("otel.animalsniffer-conventions") | ||
id("otel.jacoco-conventions") | ||
id("otel.japicmp-conventions") | ||
id("otel.publish-conventions") | ||
|
@@ -66,4 +69,25 @@ tasks { | |
isFailOnNoMatchingTests = false | ||
} | ||
} | ||
|
||
withType<AnimalSniffer>().configureEach { | ||
// we catch NoClassDefFoundError when use caffeine3 is not available on classpath and fall back to caffeine2 | ||
exclude("**/internal/shaded/caffeine3/**") | ||
|
||
ignoreClasses = listOf( | ||
// ignore shaded caffeine3 references | ||
"io.opentelemetry.instrumentation.api.internal.shaded.caffeine3.cache.Caffeine", | ||
"io.opentelemetry.instrumentation.api.internal.shaded.caffeine3.cache.Cache", | ||
|
||
// these standard Java classes seem to be desugared properly | ||
"java.lang.ClassValue", | ||
"java.util.concurrent.CompletableFuture", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not aware of this working on API21 still (how are you verifying it desugars properly?). I think it's ok because we don't call any caffeine API using it though so that's the comment we should add I guess? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm basing it on a very simple (probably too simple 😅 ) assumption -- if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for clarifying - indeed that looks like API level 21 https://github.com/signalfx/splunk-otel-android/blob/main/splunk-otel-android/build.gradle And if it's working it seems likely that it's because we don't call it indeed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I looked through all the ignored classes and it does seem that most of them come from not used code paths.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's quite dangerous to put this here, since you might use it, and if it is actually used, it will break Android compatibility. Same with any of these overrides. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can quite easily remove the I don't have any good idea about what to do with the caffeine classes though. |
||
"java.util.concurrent.CompletionException", | ||
"java.util.concurrent.CompletionStage", | ||
"java.util.concurrent.ConcurrentHashMap", | ||
mateuszrzeszutek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"java.util.concurrent.ForkJoinPool", | ||
"java.util.concurrent.atomic.LongAdder", | ||
"sun.misc.Unsafe" | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
plugins { | ||
id("otel.library-instrumentation") | ||
id("otel.animalsniffer-conventions") | ||
} | ||
|
||
val grpcVersion = "1.6.0" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call this conventions something like
android
orandroid-compatible
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I don't mind - I used
otel.animalsniffer-conventions
because the SDK file is named the same way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
android-compatibility
? (and we can update in SDK repo too)