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

support two modes of okhttp instrumentation via plugin + wire up release pipeline #144

Merged
merged 5 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions .github/workflows/integrations_android.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ on:
required: true
type: string
jobs:
build-capture-timber:
name: Build Capture Timber
build-gradle-libraries:
name: Build Gradle Libraries
runs-on: ubuntu-latest
defaults:
run:
Expand All @@ -35,13 +35,23 @@ jobs:
run: ./gradlew :capture-timber:publish -PVERSION_NAME="${{ inputs.version }}" --info
env:
SKIP_PROTO_GEN: 1
- name: Compress artifacts
- name: Compress Timber artifacts
run: |
readonly dir=$(pwd)
(cd capture-timber/build/repos/releases/io/bitdrift/capture-timber/${{ inputs.version }} && zip -r "$dir/capture-timber.zip" ./*)
- name: Compress Plugin artifacts
run: |
readonly dir=$(pwd)
(cd capture-plugin/build/repos/releases/io/bitdrift/capture-plugin/${{ inputs.version }} && zip -r "$dir/capture-plugin.zip" ./*)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure the upload artifacts job already compresses all the files in a folder, not sure why we had this compression step to being with

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also aren't you missing a step to run run: ./gradlew :capture-plugin:publish ??

- name: Upload artifacts
uses: actions/upload-artifact@v4
with:
name: capture-timber.zip
path: platform/jvm/capture-timber.zip
if-no-files-found: error
- name: Upload artifacts
uses: actions/upload-artifact@v4
with:
name: capture-plugin.zip
path: platform/jvm/capture-plugin.zip
if-no-files-found: error
6 changes: 5 additions & 1 deletion .github/workflows/release_gh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ jobs:
- uses: actions/download-artifact@v4
with:
name: capture-timber.zip
- uses: actions/download-artifact@v4
with:
name: capture-plugin.zip
- name: Prepare Android artifacts
run: ./ci/gh_prepare_android_artifacts.sh "$VERSION"
env:
Expand All @@ -223,7 +226,8 @@ jobs:
"Capture-$VERSION.android.zip" \
"example-apps.ios.zip" \
"example-apps.android.zip" \
"capture-timber-$VERSION.android.zip"
"capture-timber-$VERSION.android.zip" \
"capture-plugin-$VERSION.android.zip" \
env:
VERSION: ${{ inputs.version }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
3 changes: 2 additions & 1 deletion .github/workflows/release_public.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ jobs:
run: |
gh release download "v$VERSION" -p 'Capture*.android.zip'
gh release download "v$VERSION" -p 'capture-timber*.android.zip'
gh release download "v$VERSION" -p 'capture-plugin*.android.zip'
env:
VERSION: ${{ inputs.version }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload Android Artifacts to aws bucket
run: ./ci/capture_android_release.sh ${{ inputs.version }} "Capture-${{ inputs.version }}.android.zip" "capture-timber-${{ inputs.version }}.android.zip"
run: ./ci/capture_android_release.sh ${{ inputs.version }} "Capture-${{ inputs.version }}.android.zip" "capture-timber-${{ inputs.version }}.android.zip" "capture-plugin-${{ inputs.version }}.android.zip"

39 changes: 16 additions & 23 deletions ci/capture_android_release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ readonly remote_location_root_prefix="s3://bitdrift-public-dl/sdk/android-maven/
readonly version="$1"
readonly capture_archive="$2"
readonly capture_timber_archive="$3"
readonly capture_plugin_archive="$4"

function upload_file() {
local -r location="$1"
Expand Down Expand Up @@ -78,36 +79,28 @@ function release_capture_sdk() {
popd
}

function release_capture_timber() {
function release_gradle_library() {
local -r library_name="$1"
local -r archive="$2"

echo "+++ dl.bitdrift.io Android Capture Timber artifacts upload"

# We get a zip containing the artifacts named per Maven conventions.
local -r remote_location_prefix="$remote_location_root_prefix/$library_name"

pushd "$(mktemp -d)"
unzip -o "$sdk_repo/$capture_timber_archive"

echo "+++ Uploading artifacts to s3 bucket"
unzip -o "$archive"

# Make sure we update the top-level maven-metadata.xml file with the new release version
aws s3 cp maven-metadata.xml* "$remote_location_prefix/" --region us-east-1

local -r remote_location_prefix="$remote_location_root_prefix/capture-timber"
local -r name="capture-timber-$version"
# Update the per-version files
aws s3 cp "$sdk_repo/ci/LICENSE.txt" "$remote_location_prefix/$version/LICENSE.txt" --region us-east-1
aws s3 cp "$sdk_repo/ci/NOTICE.txt" "$remote_location_prefix/$version/NOTICE.txt" --region us-east-1

files=(\
"$sdk_repo/ci/LICENSE.txt" \
"$sdk_repo/ci/NOTICE.txt" \
"$name.pom" \
"$name-javadoc.jar" \
"$name-sources.jar" \
"$name.module" \
"$name.aar" \
)

for file in "${files[@]}"; do
upload_file "$remote_location_prefix/$version" "$file"
done

generate_maven_file "$remote_location_prefix"
aws s3 cp "$version" "$remote_location_prefix/$version/" --recursive --region us-east-1
popd
}

release_capture_sdk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong method call removed

release_gradle_library "capture-timber" "$capture_timber_archive"
release_gradle_library "capture-plugin" "$capture_plugin_archive"
release_capture_timber
30 changes: 6 additions & 24 deletions ci/gh_prepare_android_artifacts.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,33 +43,15 @@ function prepare_capture_sdk() {
function prepare_capture_timber() {
echo "+++ Preparing Android Capture Timber library artifacts for '$version' version"

pushd "$(mktemp -d)"
local -r out_artifacts_dir="capture-timber-out"

unzip "$sdk_repo/capture-timber.zip"

mkdir "$out_artifacts_dir"

local -r name="capture-timber-$version"

files=(\
"$name.aar" \
"$name.module" \
"$name.pom" \
"$name-javadoc.jar" \
"$name-sources.jar" \
"$sdk_repo/ci/LICENSE.txt" \
"$sdk_repo/ci/NOTICE.txt" \
)
cp "$sdk_repo/capture-timber.zip" "$sdk_repo/capture-timber-$version.android.zip"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the implication that we will now include checksum files in the GH release zips for timber/plugin which isn't ideal but I don't think anyone relies on the GH releases so seems fine until we can simplify the deployment pipeline further

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that should be fine, arguably any artifact bundle can benefit from including checksums

}

for file in "${files[@]}"; do
filename=$(basename "$file")
mv "$file" "$out_artifacts_dir/$filename"
done
function prepare_capture_plugin() {
echo "+++ Preparing Android Capture Timber library artifacts for '$version' version"

(cd "$out_artifacts_dir" && zip -r "$sdk_repo/capture-timber-$version.android.zip" ./*)
popd
cp "$sdk_repo/capture-plugin.zip" "$sdk_repo/capture-plugin-$version.android.zip"
}

prepare_capture_sdk
prepare_capture_timber
prepare_capture_plugin
46 changes: 41 additions & 5 deletions platform/jvm/capture-plugin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
id("java-gradle-plugin")
}

group = "io.bitdrift"

dependencies {
compileOnly("com.android.tools.build:gradle:7.4.0")
compileOnly("org.ow2.asm:asm-commons:9.4")
Expand All @@ -22,17 +24,51 @@ dependencies {
gradlePlugin {
plugins {
create("capturePlugin") {
id = "io.bitdrift.capture.capture-plugin"
id = "io.bitdrift.capture-plugin"
implementationClass = "io.bitdrift.capture.CapturePlugin"
}
}
}

mavenPublishing {
configureBasedOnAppliedPlugins()

pom {
name.set("CapturePlugin")
description.set("Official Capture Gradle plugin.")
url.set("https://bitdrift.io")
licenses {
license {
name.set("BITDRIFT SOFTWARE DEVELOPMENT KIT LICENSE AGREEMENT")
url.set("https://dl.bitdrift.io/sdk/android-maven/io/bitdrift/capture-plugin/${findProperty("VERSION_NAME")}/LICENSE.txt")
distribution.set("repo")
}
license {
name.set("NOTICE")
url.set("https://dl.bitdrift.io/sdk/android-maven/io/bitdrift/capture-plugin/${findProperty("VERSION_NAME")}/NOTICE.txt")
distribution.set("repo")
}
}
developers {
developer {
id.set("bitdriftlabs")
name.set("Bitdrift, Inc.")
url.set("https://github.com/bitdriftlabs")
email.set("[email protected]")
}
scm {
connection.set("scm:git:git://github.com/bitdriftlabs/capture-sdk.git")
developerConnection.set("scm:git:ssh://[email protected]:bitdriftlabs/capture-sdk.git")
url.set("https://github.com/bitdriftlabs/capture-sdk")
}
}
}
}

publishing {
repositories {
mavenLocal()
maven {
url = uri(layout.buildDirectory.dir("repos/releases"))
}
}
}

group = "io.bitdrift.capture.capture-plugin"
version = "0.1.0"
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ fun AndroidComponentsExtension<*, *, *>.configure(
FramesComputationMode.COMPUTE_FRAMES_FOR_INSTRUMENTED_METHODS,
) { params ->
params.tmpDir.set(tmpDir)
params.debug.set(false)
params.debug.set(extension.instrumentation.debug)
params.proxyOkHttpEventListener.set(extension.instrumentation.proxyOkHttpEventListener)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ open class InstrumentationExtension @Inject constructor(objects: ObjectFactory)
val debug: Property<Boolean> = objects.property(Boolean::class.java).convention(
false
)

val proxyOkHttpEventListener: Property<Boolean> = objects.property(Boolean::class.java).convention(false)
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ abstract class SpanAddingClassVisitorFactory : AsmClassVisitorFactory<SpanAdding
@get:Input
val debug: Property<Boolean>

@get:Input
val proxyOkHttpEventListener: Property<Boolean>

@get:Internal
val tmpDir: Property<File>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,19 @@ class OkHttpEventListener(
override val fqName: String get() = "okhttp3.OkHttpClient"

override fun getVisitor(
instrumentableContext: ClassContext,
apiVersion: Int,
originalVisitor: ClassVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters
instrumentableContext: ClassContext,
apiVersion: Int,
originalVisitor: ClassVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters
): ClassVisitor = CommonClassVisitor(
apiVersion = apiVersion,
classVisitor = originalVisitor,
className = fqName.substringAfterLast('.'),
methodInstrumentables = listOf(
OkHttpEventListenerMethodInstrumentable(
)
),
parameters = parameters
apiVersion = apiVersion,
classVisitor = originalVisitor,
className = fqName.substringAfterLast('.'),
methodInstrumentables = listOf(
OkHttpEventListenerMethodInstrumentable(
)
),
parameters = parameters
)
}

Expand All @@ -69,14 +69,15 @@ class OkHttpEventListenerMethodInstrumentable(
override val fqName: String get() = "<init>"

override fun getVisitor(
instrumentableContext: MethodContext,
apiVersion: Int,
originalVisitor: MethodVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters
instrumentableContext: MethodContext,
apiVersion: Int,
originalVisitor: MethodVisitor,
parameters: SpanAddingClassVisitorFactory.SpanAddingParameters
): MethodVisitor = OkHttpEventListenerMethodVisitor(
apiVersion = apiVersion,
originalVisitor = originalVisitor,
instrumentableContext = instrumentableContext,
apiVersion = apiVersion,
originalVisitor = originalVisitor,
instrumentableContext = instrumentableContext,
proxyEventListener = parameters.proxyOkHttpEventListener.get()
)

override fun isInstrumentable(data: MethodContext): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,68 @@ import org.objectweb.asm.Opcodes
import org.objectweb.asm.commons.AdviceAdapter

class OkHttpEventListenerMethodVisitor(
apiVersion: Int,
originalVisitor: MethodVisitor,
instrumentableContext: MethodContext,
apiVersion: Int,
originalVisitor: MethodVisitor,
instrumentableContext: MethodContext,
val proxyEventListener: Boolean
) : AdviceAdapter(
apiVersion,
originalVisitor,
instrumentableContext.access,
instrumentableContext.name,
instrumentableContext.descriptor
apiVersion,
originalVisitor,
instrumentableContext.access,
instrumentableContext.name,
instrumentableContext.descriptor
) {

private val captureOkHttpEventListenerFactory =
"io/bitdrift/capture/network/okhttp/CaptureOkHttpEventListenerFactory"
"io/bitdrift/capture/network/okhttp/CaptureOkHttpEventListenerFactory"

override fun onMethodEnter() {
super.onMethodEnter()

if (proxyEventListener) {
addProxyingEventListener()
} else {
addOverwritingEventListener()
}
}

private fun addOverwritingEventListener() {
// Add the following call at the beginning of the constructor with the Builder parameter:
// builder.eventListenerFactory(new CaptureOkHttpEventListenerFactory());

// OkHttpClient.Builder is the parameter, retrieved here
visitVarInsn(Opcodes.ALOAD, 1)

// Let's declare the CaptureOkHttpEventListenerFactory variable
visitTypeInsn(Opcodes.NEW, captureOkHttpEventListenerFactory)

// The CaptureOkHttpEventListenerFactory constructor, which is called later, will consume the
// element without pushing anything back to the stack (<init> returns void).
// Dup will give a reference to the CaptureOkHttpEventListenerFactory after the constructor call
visitInsn(Opcodes.DUP)

// Call CaptureOkHttpEventListenerFactory constructor passing "eventListenerFactory" as parameter
visitMethodInsn(
Opcodes.INVOKESPECIAL,
captureOkHttpEventListenerFactory,
"<init>",
"()V",
false
)

// Call "eventListener" function of OkHttpClient.Builder passing CaptureOkHttpEventListenerFactory
visitMethodInsn(
Opcodes.INVOKEVIRTUAL,
"okhttp3/OkHttpClient\$Builder",
"eventListenerFactory",
"(Lokhttp3/EventListener\$Factory;)Lokhttp3/OkHttpClient\$Builder;",
false
)
}

private fun addProxyingEventListener() {
// Add the following call at the beginning of the constructor with the Builder parameter:
// builder.eventListener(new CaptureOkHttpEventListener(builder.eventListenerFactory));
// builder.eventListenerFactory(new CaptureOkHttpEventListenerFactory(builder.eventListenerFactory));

// OkHttpClient.Builder is the parameter, retrieved here
visitVarInsn(Opcodes.ALOAD, 1)
Expand Down
Loading