Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

fix: envelope header has sdkVersion and not sdkInfo #488

Merged
merged 4 commits into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
}

PackageInfo packageInfo = ContextUtils.getPackageInfo(context, logger);
Expand Down Expand Up @@ -235,7 +234,6 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) {

DebugMeta debugMeta = new DebugMeta();
debugMeta.setImages(debugImages);
debugMeta.setSdkInfo(options.getSdkInfo());
return debugMeta;
}

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

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


return sdkVersion;
}

/**
* Checks if ANR (Application Not Responding) is enabled or disabled Default is enabled
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,22 +323,28 @@ class AndroidOptionsInitializerTest {
}

@Test
fun `init should set SdkInfo`() {
fun `init should set SdkVersion`() {
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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"
}
}
}

Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
@@ -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"}}
9 changes: 4 additions & 5 deletions sentry-android-ndk/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import com.novoda.gradle.release.PublishExtension
import org.jetbrains.kotlin.config.KotlinCompilerVersion

plugins {
id("com.android.library")
Expand Down Expand Up @@ -42,11 +43,6 @@ android {
}
}

buildFeatures {
// Determines whether to generate a BuildConfig class.
buildConfig = false
}

externalNativeBuild {
cmake {
version = Config.Android.cmakeVersion
Expand Down Expand Up @@ -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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.sentry.android.ndk

import io.sentry.core.SentryOptions
import io.sentry.core.protocol.SdkVersion
import kotlin.test.Test
import kotlin.test.assertTrue

class SentryNdkUtilTest {
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

@Test
fun `SentryNdk adds the Ndk into 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
})
}
}
5 changes: 0 additions & 5 deletions sentry-android-timber/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading