fix: envelope header has sdkVersion and not sdkInfo (#488)

getsentry · Jul 21, 2020 · 89da1a3 · 89da1a3
1 parent c8dbbad
commit 89da1a3
Show file tree

Hide file tree

Showing 27 changed files with 309 additions and 282 deletions.
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
@@ -13,7 +13,6 @@
 import io.sentry.core.SendFireAndForgetEnvelopeSender;
 import io.sentry.core.SentryLevel;
 import io.sentry.core.SentryOptions;
-import io.sentry.core.protocol.SdkInfo;
 import io.sentry.core.util.Objects;
 import java.io.File;
 import org.jetbrains.annotations.NotNull;
@@ -100,8 +99,6 @@ static void init(
     options.setLogger(logger);
 
     options.setSentryClientName(BuildConfig.SENTRY_CLIENT_NAME + "/" + BuildConfig.VERSION_NAME);
-    options.setSdkInfo(
-        SdkInfo.createSdkInfo(BuildConfig.SENTRY_CLIENT_NAME, BuildConfig.VERSION_NAME));
 
     ManifestMetadataReader.applyMetadata(context, options);
     initializeCacheDirs(context, options);

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java
@@ -33,7 +33,6 @@
 import io.sentry.core.protocol.DebugMeta;
 import io.sentry.core.protocol.Device;
 import io.sentry.core.protocol.OperatingSystem;
-import io.sentry.core.protocol.SdkVersion;
 import io.sentry.core.protocol.SentryThread;
 import io.sentry.core.protocol.User;
 import io.sentry.core.util.ApplyScopeUtils;
@@ -176,7 +175,7 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) {
       event.setDebugMeta(getDebugMeta());
     }
     if (event.getSdk() == null) {
-      event.setSdk(getSdkVersion());
+      event.setSdk(options.getSdkVersion());
     }
 
     PackageInfo packageInfo = ContextUtils.getPackageInfo(context, logger);
@@ -235,7 +234,6 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) {
 
     DebugMeta debugMeta = new DebugMeta();
     debugMeta.setImages(debugImages);
-    debugMeta.setSdkInfo(options.getSdkInfo());
     return debugMeta;
   }
 
@@ -244,21 +242,6 @@ private void setAppExtras(final @NotNull App app) {
     app.setAppStartTime(appStartTime);
   }
 
-  private @NotNull SdkVersion getSdkVersion() {
-    SdkVersion sdkVersion = new SdkVersion();
-
-    sdkVersion.setName("sentry.java.android");
-    String version = BuildConfig.VERSION_NAME;
-    sdkVersion.setVersion(version);
-    sdkVersion.addPackage("maven:sentry-core", version);
-    sdkVersion.addPackage("maven:sentry-android-core", version);
-    if (options.isEnableNdk()) {
-      sdkVersion.addPackage("maven:sentry-android-ndk", version);
-    }
-
-    return sdkVersion;
-  }
-
   @SuppressWarnings("deprecation")
   private @NotNull String getAbi() {
     return Build.CPU_ABI;

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java
@@ -1,7 +1,9 @@
 package io.sentry.android.core;
 
 import io.sentry.core.SentryOptions;
+import io.sentry.core.protocol.SdkVersion;
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
 
 /** Sentry SDK options for Android */
 public final class SentryAndroidOptions extends SentryOptions {
@@ -32,6 +34,25 @@ public final class SentryAndroidOptions extends SentryOptions {
   /** Enable or disable automatic breadcrumbs for App Components Using ComponentCallbacks */
   private boolean enableAppComponentBreadcrumbs = true;
 
+  public SentryAndroidOptions() {
+    setSdkVersion(createSdkVersion());
+  }
+
+  private @NotNull SdkVersion createSdkVersion() {
+    final SdkVersion sdkVersion = new SdkVersion();
+
+    sdkVersion.setName(BuildConfig.SENTRY_CLIENT_NAME);
+    String version = BuildConfig.VERSION_NAME;
+    sdkVersion.setVersion(version);
+
+    // add 2 default packages
+    sdkVersion.addPackage("maven:sentry-android-core", version);
+    // TODO: sentry-core should add itself as the version may mismatch
+    sdkVersion.addPackage("maven:sentry-core", version);
+
+    return sdkVersion;
+  }
+
   /**
    * Checks if ANR (Application Not Responding) is enabled or disabled Default is enabled
    *

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt
@@ -323,22 +323,28 @@ class AndroidOptionsInitializerTest {
     }
 
     @Test
-    fun `init should set SdkInfo`() {
+    fun `init should set SdkVersion`() {
         val sentryOptions = SentryAndroidOptions()
         val mockContext = mock<Context>()
         whenever(mockContext.applicationContext).thenReturn(null)
 
         AndroidOptionsInitializer.init(sentryOptions, mockContext)
 
-        assertNotNull(sentryOptions.sdkInfo)
-        val sdkInfo = sentryOptions.sdkInfo!!
+        assertNotNull(sentryOptions.sdkVersion)
+        val sdkVersion = sentryOptions.sdkVersion!!
 
-        assertEquals("sentry.java.android", sdkInfo.sdkName)
+        assertEquals(BuildConfig.SENTRY_CLIENT_NAME, sdkVersion.name)
+        assertEquals(BuildConfig.VERSION_NAME, sdkVersion.version)
 
-        // versions change on every release, so lets check only if its not null
-        assertNotNull(sdkInfo.versionMajor)
-        assertNotNull(sdkInfo.versionMinor)
-        assertNotNull(sdkInfo.versionPatchlevel)
+        assertTrue(sdkVersion.packages!!.any {
+            it.name == "maven:sentry-android-core"
+            it.version == BuildConfig.VERSION_NAME
+        })
+
+        assertTrue(sdkVersion.packages!!.any {
+            it.name == "maven:sentry-core"
+            it.version == BuildConfig.VERSION_NAME
+        })
     }
 
     private fun createMockContext(): Context {

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt
@@ -14,7 +14,7 @@ import io.sentry.core.SentryLevel
 import io.sentry.core.Session
 import io.sentry.core.protocol.Contexts
 import io.sentry.core.protocol.Device
-import io.sentry.core.protocol.SdkInfo
+import io.sentry.core.protocol.SdkVersion
 import java.io.ByteArrayInputStream
 import java.io.IOException
 import java.io.InputStream
@@ -358,16 +358,24 @@ class AndroidSerializerTest {
     }
 
     @Test
-    fun `When deserializing an Envelope, sdkInfo should be set`() {
-        val jsonEnvelope = FileFromResources.invoke("envelope_session_sdkinfo.txt")
+    fun `When deserializing an Envelope, SdkVersion should be set`() {
+        val jsonEnvelope = FileFromResources.invoke("envelope_session_sdkversion.txt")
         val envelope = serializer.deserializeEnvelope(ByteArrayInputStream(jsonEnvelope.toByteArray(Charsets.UTF_8)))!!
-        assertNotNull(envelope.header.sdkInfo)
-        val sdkInfo = envelope.header.sdkInfo!!
+        assertNotNull(envelope.header.sdkVersion)
+        val sdkInfo = envelope.header.sdkVersion!!
 
-        assertEquals("test", sdkInfo.sdkName)
-        assertEquals(1, sdkInfo.versionMajor)
-        assertEquals(2, sdkInfo.versionMinor)
-        assertEquals(3, sdkInfo.versionPatchlevel)
+        assertEquals("test", sdkInfo.name)
+        assertEquals("1.2.3", sdkInfo.version)
+
+        assertNotNull(sdkInfo.integrations)
+        assertTrue(sdkInfo.integrations!!.any { it == "NdkIntegration" })
+
+        assertNotNull(sdkInfo.packages)
+
+        assertTrue(sdkInfo.packages!!.any {
+            it.name == "maven:sentry-android-core"
+            it.version == "4.5.6"
+        })
     }
 
     @Test
@@ -382,20 +390,33 @@ class AndroidSerializerTest {
     }
 
     @Test
-    fun `When serializing an envelope, sdkInfo should be set`() {
+    fun `When serializing an envelope, SdkVersion should be set`() {
         val session = createSessionMockData()
-        val sentryEnvelope = SentryEnvelope.fromSession(serializer, session, SdkInfo.createSdkInfo("test", "1.2.3"))
+        val version = SdkVersion().apply {
+            name = "test"
+            version = "1.2.3"
+            addIntegration("TestIntegration")
+            addPackage("abc", "4.5.6")
+        }
+        val sentryEnvelope = SentryEnvelope.fromSession(serializer, session, version)
 
         val jsonEnvelope = serializeToString(sentryEnvelope)
         // reversing it so we can assert the values
         val envelope = serializer.deserializeEnvelope(ByteArrayInputStream(jsonEnvelope.toByteArray(Charsets.UTF_8)))!!
-        assertNotNull(envelope.header.sdkInfo)
+        assertNotNull(envelope.header.sdkVersion)
+
+        val sdkVersion = envelope.header.sdkVersion!!
+        assertEquals(version.name, sdkVersion.name)
+        assertEquals(version.version, sdkVersion.version)
+
+        assertNotNull(sdkVersion.integrations)
+        assertTrue(sdkVersion.integrations!!.any { it == "TestIntegration" })
 
-        val sdkInfo = envelope.header.sdkInfo!!
-        assertEquals("test", sdkInfo.sdkName)
-        assertEquals(1, sdkInfo.versionMajor)
-        assertEquals(2, sdkInfo.versionMinor)
-        assertEquals(3, sdkInfo.versionPatchlevel)
+        assertNotNull(sdkVersion.packages)
+        assertTrue(sdkVersion.packages!!.any {
+            it.name == "abc"
+            it.version == "4.5.6"
+        })
     }
 
     private fun assertSessionData(expectedSession: Session?) {

diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt
@@ -18,7 +18,7 @@ import io.sentry.core.DiagnosticLogger
 import io.sentry.core.SentryEvent
 import io.sentry.core.SentryLevel
 import io.sentry.core.SentryOptions
-import io.sentry.core.protocol.SdkInfo
+import io.sentry.core.protocol.SdkVersion
 import io.sentry.core.protocol.SentryThread
 import kotlin.test.BeforeTest
 import kotlin.test.Test
@@ -27,6 +27,7 @@ import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
+import kotlin.test.assertSame
 import kotlin.test.assertTrue
 import org.junit.runner.RunWith
 
@@ -39,7 +40,10 @@ class DefaultAndroidEventProcessorTest {
         val options = SentryOptions().apply {
             isDebug = true
             setLogger(mock())
-            sdkInfo = SdkInfo.createSdkInfo("test", "1.2.3")
+            sdkVersion = SdkVersion().apply {
+                name = "test"
+                version = "1.2.3"
+            }
         }
     }
 
@@ -165,14 +169,11 @@ class DefaultAndroidEventProcessorTest {
     }
 
     @Test
-    fun `Processor sets sdkInfo in the event`() {
+    fun `Processor sets sdkVersion in the event`() {
         val processor = DefaultAndroidEventProcessor(context, fixture.options, fixture.buildInfo, mock())
         val event = SentryEvent()
         processor.process(event, null)
-        assertNotNull(event.debugMeta.sdkInfo)
-        assertEquals("test", event.debugMeta.sdkInfo.sdkName)
-        assertEquals(1, event.debugMeta.sdkInfo.versionMajor)
-        assertEquals(2, event.debugMeta.sdkInfo.versionMinor)
-        assertEquals(3, event.debugMeta.sdkInfo.versionPatchlevel)
+        assertNotNull(event.sdk)
+        assertSame(event.sdk, fixture.options.sdkVersion)
     }
 }
diff --git a/...st/resources/envelope_session_sdkinfo.txt → ...resources/envelope_session_sdkversion.txt b/...st/resources/envelope_session_sdkinfo.txt → ...resources/envelope_session_sdkversion.txt
@@ -1,3 +1,3 @@
-{"sdk_info":{"sdk_name":"test","version_major":1,"version_minor":2,"version_patchlevel":3}}
+{"sdk":{"name":"test","version":"1.2.3","integrations":["NdkIntegration"],"packages":[{"name":"maven:sentry-android-core","version":"4.5.6"}]}}
 {"content_type":"application/json","type":"session","length":306}
 {"sid":"c81d4e2e-bcf2-11e6-869b-7df92533d2db","did":"123","init":true,"started":"2020-02-07T14:16:00Z","status":"ok","seq":123456,"errors":2,"duration":6000.0,"timestamp":"2020-02-07T14:16:00Z","attrs":{"release":"io.sentry@1.0+123","environment":"debug","ip_address":"127.0.0.1","user_agent":"jamesBond"}}
diff --git a/sentry-android-ndk/build.gradle.kts b/sentry-android-ndk/build.gradle.kts
@@ -1,4 +1,5 @@
 import com.novoda.gradle.release.PublishExtension
+import org.jetbrains.kotlin.config.KotlinCompilerVersion
 
 plugins {
     id("com.android.library")
@@ -42,11 +43,6 @@ android {
         }
     }
 
-    buildFeatures {
-        // Determines whether to generate a BuildConfig class.
-        buildConfig = false
-    }
-
     externalNativeBuild {
         cmake {
             version = Config.Android.cmakeVersion
@@ -103,6 +99,9 @@ dependencies {
     api(project(":sentry-android-core"))
 
     compileOnly(Config.CompileOnly.jetbrainsAnnotations)
+
+    testImplementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION))
+    testImplementation(Config.TestLibs.kotlinTestJunit)
 }
 
 val initNative = tasks.register<Exec>("initNative") {

diff --git a/sentry-android-ndk/src/main/java/io/sentry/android/ndk/SentryNdk.java b/sentry-android-ndk/src/main/java/io/sentry/android/ndk/SentryNdk.java
@@ -2,6 +2,7 @@
 
 import io.sentry.core.SentryOptions;
 import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
 
 @ApiStatus.Internal
 public final class SentryNdk {
@@ -19,9 +20,10 @@ private SentryNdk() {}
     System.loadLibrary("sentry-android");
   }
 
-  private static native void initSentryNative(SentryOptions options);
+  private static native void initSentryNative(@NotNull final SentryOptions options);
 
-  public static void init(SentryOptions options) {
+  public static void init(@NotNull final SentryOptions options) {
+    SentryNdkUtil.addPackage(options.getSdkVersion());
     initSentryNative(options);
   }
 }
diff --git a/sentry-android-ndk/src/main/java/io/sentry/android/ndk/SentryNdkUtil.java b/sentry-android-ndk/src/main/java/io/sentry/android/ndk/SentryNdkUtil.java
@@ -0,0 +1,24 @@
+package io.sentry.android.ndk;
+
+import io.sentry.core.protocol.SdkVersion;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Util class to make SentryNdk testable, as SentryNdk inits native libraries and it breaks on init.
+ */
+final class SentryNdkUtil {
+
+  private SentryNdkUtil() {}
+
+  /**
+   * Adds the sentry-android-ndk package into the package list
+   *
+   * @param sdkVersion the SdkVersion object
+   */
+  static void addPackage(@Nullable final SdkVersion sdkVersion) {
+    if (sdkVersion == null) {
+      return;
+    }
+    sdkVersion.addPackage("maven:sentry-android-ndk", BuildConfig.VERSION_NAME);
+  }
+}
diff --git a/sentry-android-ndk/src/test/java/io/sentry/android/ndk/SentryNdkUtilTest.kt b/sentry-android-ndk/src/test/java/io/sentry/android/ndk/SentryNdkUtilTest.kt
@@ -0,0 +1,30 @@
+package io.sentry.android.ndk
+
+import io.sentry.core.SentryOptions
+import io.sentry.core.protocol.SdkVersion
+import kotlin.test.Test
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+class SentryNdkUtilTest {
+
+    @Test
+    fun `SentryNdk adds the Ndk package into the package list`() {
+        val options = SentryOptions().apply {
+            sdkVersion = SdkVersion()
+        }
+        SentryNdkUtil.addPackage(options.sdkVersion)
+        assertTrue(options.sdkVersion!!.packages!!.any {
+            it.name == "maven:sentry-android-ndk"
+            it.version == BuildConfig.VERSION_NAME
+        })
+    }
+
+    @Test
+    fun `SentryNdk do not add the Ndk package into the package list`() {
+        val options = SentryOptions()
+        SentryNdkUtil.addPackage(options.sdkVersion)
+
+        assertNull(options.sdkVersion?.packages)
+    }
+}
diff --git a/sentry-android-timber/build.gradle.kts b/sentry-android-timber/build.gradle.kts
@@ -33,11 +33,6 @@ android {
         }
     }
 
-    buildFeatures {
-        // Determines whether to generate a BuildConfig class.
-        buildConfig = false
-    }
-
     compileOptions {
         sourceCompatibility = JavaVersion.VERSION_1_8
         targetCompatibility = JavaVersion.VERSION_1_8