Skip to content

Commit

Permalink
Merge branch 'main' into lcian/feat/apollo-4
Browse files Browse the repository at this point in the history
  • Loading branch information
lcian authored Feb 24, 2025
2 parents de7eeb9 + 37b98dc commit b584414
Show file tree
Hide file tree
Showing 28 changed files with 726 additions and 49 deletions.
4 changes: 0 additions & 4 deletions .craft.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
minVersion: 0.29.3
changelogPolicy: auto
targets:
- name: symbol-collector
includeNames: /libsentry(-android)?\.so/
batchType: android
bundleIdPrefix: sentry-android-ndk-
- name: maven
includeNames: /^sentry.*$/
gradleCliPath: ./gradlew
Expand Down
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
### Features

- The `ignoredErrors` option is now configurable via the manifest property `io.sentry.traces.ignored-errors` ([#4178](https://github.com/getsentry/sentry-java/pull/4178))
- Add Apollo 4 integration ([#4166](https://github.com/getsentry/sentry-java/pull/4166))
- A list of active Spring profiles is attached to payloads sent to Sentry (errors, traces, etc.) and displayed in the UI when using our Spring or Spring Boot integrations ([#4147](https://github.com/getsentry/sentry-java/pull/4147))
- This consists of an empty list when only the default profile is active
- Move to a single NetworkCallback listener to reduce number of IPC calls on Android ([#4164](https://github.com/getsentry/sentry-java/pull/4164))
- Add GraphQL Apollo Kotlin 4 integration ([#4166](https://github.com/getsentry/sentry-java/pull/4166))

### Fixes

Expand Down
30 changes: 30 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,36 @@ To run the build and tests:
make compile
```

# Format

To format the changed code and make CI happy you can run:

```shell
make format
```

or

```shell
./gradlew spotlessApply
```

# Binary compatibility validation

To prevent breaking ABI changes and exposing things we should not, we make use of https://github.com/Kotlin/binary-compatibility-validator. If your change intended to introduce a new public method/property or modify the existing one you can overwrite the API declarations to make CI happy as follows (overwrites them from scratch):

```shell
make api
```

or

```shell
./gradlew apiDump
```

However, if your change did not intend to modify the public API, consider changing the method/property visibility or removing the change altogether.

# CI

Build and tests are automatically run against branches and pull requests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,19 @@
import android.annotation.SuppressLint;
import android.content.Context;
import android.net.ConnectivityManager;
import android.net.ConnectivityManager.NetworkCallback;
import android.net.Network;
import android.net.NetworkCapabilities;
import android.os.Build;
import androidx.annotation.NonNull;
import io.sentry.IConnectionStatusProvider;
import io.sentry.ILogger;
import io.sentry.ISentryLifecycleToken;
import io.sentry.SentryLevel;
import io.sentry.android.core.BuildInfoProvider;
import io.sentry.android.core.ContextUtils;
import java.util.HashMap;
import java.util.Map;
import io.sentry.util.AutoClosableReentrantLock;
import java.util.ArrayList;
import java.util.List;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -31,8 +33,9 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP
private final @NotNull Context context;
private final @NotNull ILogger logger;
private final @NotNull BuildInfoProvider buildInfoProvider;
private final @NotNull Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
registeredCallbacks;
private final @NotNull List<IConnectionStatusObserver> connectionStatusObservers;
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
private volatile @Nullable NetworkCallback networkCallback;

public AndroidConnectionStatusProvider(
@NotNull Context context,
Expand All @@ -41,7 +44,7 @@ public AndroidConnectionStatusProvider(
this.context = ContextUtils.getApplicationContext(context);
this.logger = logger;
this.buildInfoProvider = buildInfoProvider;
this.registeredCallbacks = new HashMap<>();
this.connectionStatusObservers = new ArrayList<>();
}

@Override
Expand All @@ -65,40 +68,64 @@ public AndroidConnectionStatusProvider(

@Override
public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
connectionStatusObservers.add(observer);
}

final ConnectivityManager.NetworkCallback callback =
new ConnectivityManager.NetworkCallback() {
@Override
public void onAvailable(@NonNull Network network) {
observer.onConnectionStatusChanged(getConnectionStatus());
}

@Override
public void onLosing(@NonNull Network network, int maxMsToLive) {
observer.onConnectionStatusChanged(getConnectionStatus());
}

@Override
public void onLost(@NonNull Network network) {
observer.onConnectionStatusChanged(getConnectionStatus());
}

@Override
public void onUnavailable() {
observer.onConnectionStatusChanged(getConnectionStatus());
if (networkCallback == null) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
if (networkCallback == null) {
final @NotNull NetworkCallback newNetworkCallback =
new NetworkCallback() {
@Override
public void onAvailable(final @NotNull Network network) {
updateObservers();
}

@Override
public void onUnavailable() {
updateObservers();
}

@Override
public void onLost(final @NotNull Network network) {
updateObservers();
}

public void updateObservers() {
final @NotNull ConnectionStatus status = getConnectionStatus();
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
for (final @NotNull IConnectionStatusObserver observer :
connectionStatusObservers) {
observer.onConnectionStatusChanged(status);
}
}
}
};

if (registerNetworkCallback(context, logger, buildInfoProvider, newNetworkCallback)) {
networkCallback = newNetworkCallback;
return true;
} else {
return false;
}
};

registeredCallbacks.put(observer, callback);
return registerNetworkCallback(context, logger, buildInfoProvider, callback);
}
}
}
// networkCallback is already registered, so we can safely return true
return true;
}

@Override
public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) {
final @Nullable ConnectivityManager.NetworkCallback callback =
registeredCallbacks.remove(observer);
if (callback != null) {
unregisterNetworkCallback(context, logger, callback);
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
connectionStatusObservers.remove(observer);
if (connectionStatusObservers.isEmpty()) {
if (networkCallback != null) {
unregisterNetworkCallback(context, logger, networkCallback);
networkCallback = null;
}
}
}
}

Expand Down Expand Up @@ -281,7 +308,7 @@ public static boolean registerNetworkCallback(
final @NotNull Context context,
final @NotNull ILogger logger,
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull ConnectivityManager.NetworkCallback networkCallback) {
final @NotNull NetworkCallback networkCallback) {
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
return false;
Expand All @@ -307,7 +334,7 @@ public static boolean registerNetworkCallback(
public static void unregisterNetworkCallback(
final @NotNull Context context,
final @NotNull ILogger logger,
final @NotNull ConnectivityManager.NetworkCallback networkCallback) {
final @NotNull NetworkCallback networkCallback) {

final ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
if (connectivityManager == null) {
Expand All @@ -322,8 +349,13 @@ public static void unregisterNetworkCallback(

@TestOnly
@NotNull
public Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
getRegisteredCallbacks() {
return registeredCallbacks;
public List<IConnectionStatusObserver> getStatusObservers() {
return connectionStatusObservers;
}

@TestOnly
@Nullable
public NetworkCallback getNetworkCallback() {
return networkCallback;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue

Expand Down Expand Up @@ -258,9 +259,12 @@ class AndroidConnectionStatusProviderTest {
val observer = IConnectionStatusProvider.IConnectionStatusObserver { }
val addResult = connectionStatusProvider.addConnectionStatusObserver(observer)
assertTrue(addResult)
assertEquals(1, connectionStatusProvider.statusObservers.size)
assertNotNull(connectionStatusProvider.networkCallback)

connectionStatusProvider.removeConnectionStatusObserver(observer)
assertTrue(connectionStatusProvider.registeredCallbacks.isEmpty())
assertTrue(connectionStatusProvider.statusObservers.isEmpty())
assertNull(connectionStatusProvider.networkCallback)
}

@Test
Expand All @@ -269,18 +273,16 @@ class AndroidConnectionStatusProviderTest {

var callback: NetworkCallback? = null
whenever(connectivityManager.registerDefaultNetworkCallback(any())).then { invocation ->
callback = invocation.getArgument(0, NetworkCallback::class.java)
callback = invocation.getArgument(0, NetworkCallback::class.java) as NetworkCallback
Unit
}
val observer = mock<IConnectionStatusProvider.IConnectionStatusObserver>()
connectionStatusProvider.addConnectionStatusObserver(observer)
callback!!.onAvailable(mock<Network>())
callback!!.onUnavailable()
callback!!.onLosing(mock<Network>(), 0)
callback!!.onLost(mock<Network>())
callback!!.onUnavailable()
connectionStatusProvider.removeConnectionStatusObserver(observer)

verify(observer, times(5)).onConnectionStatusChanged(any())
verify(observer, times(3)).onConnectionStatusChanged(any())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.sentry.spring.jakarta.SentryUserFilter;
import io.sentry.spring.jakarta.SentryUserProvider;
import io.sentry.spring.jakarta.SentryWebConfiguration;
import io.sentry.spring.jakarta.SpringProfilesEventProcessor;
import io.sentry.spring.jakarta.SpringSecuritySentryUserProvider;
import io.sentry.spring.jakarta.checkin.SentryCheckInAdviceConfiguration;
import io.sentry.spring.jakarta.checkin.SentryCheckInPointcutConfiguration;
Expand Down Expand Up @@ -68,6 +69,7 @@
import org.springframework.context.annotation.Import;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.core.env.Environment;
import org.springframework.graphql.execution.DataFetcherExceptionResolverAdapter;
import org.springframework.scheduling.quartz.SchedulerFactoryBean;
import org.springframework.security.core.context.SecurityContextHolder;
Expand Down Expand Up @@ -470,4 +472,14 @@ private static class SentryTracesSampleRateCondition {}
@SuppressWarnings("UnusedNestedClass")
private static class SentryTracesSamplerBeanCondition {}
}

@Configuration(proxyBeanMethods = false)
@Open
static class SpringProfilesEventProcessorConfiguration {
@Bean
public @NotNull SpringProfilesEventProcessor springProfilesEventProcessor(
final Environment environment) {
return new SpringProfilesEventProcessor(environment);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import io.sentry.spring.jakarta.HttpServletRequestSentryUserProvider
import io.sentry.spring.jakarta.SentryExceptionResolver
import io.sentry.spring.jakarta.SentryUserFilter
import io.sentry.spring.jakarta.SentryUserProvider
import io.sentry.spring.jakarta.SpringProfilesEventProcessor
import io.sentry.spring.jakarta.SpringSecuritySentryUserProvider
import io.sentry.spring.jakarta.tracing.SentryTracingFilter
import io.sentry.spring.jakarta.tracing.SpringServletTransactionNameProvider
Expand Down Expand Up @@ -925,6 +926,14 @@ class SentryAutoConfigurationTest {
}
}

@Test
fun `registers SpringProfilesEventProcessor on SentryOptions`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it.getBean(SentryOptions::class.java).eventProcessors).anyMatch { processor -> processor.javaClass == SpringProfilesEventProcessor::class.java }
}
}

@Configuration(proxyBeanMethods = false)
open class CustomSchedulerFactoryBeanCustomizerConfiguration {
class MyJobListener : JobListener {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.sentry.spring.SentryUserFilter;
import io.sentry.spring.SentryUserProvider;
import io.sentry.spring.SentryWebConfiguration;
import io.sentry.spring.SpringProfilesEventProcessor;
import io.sentry.spring.SpringSecuritySentryUserProvider;
import io.sentry.spring.boot.graphql.SentryGraphqlAutoConfiguration;
import io.sentry.spring.checkin.SentryCheckInAdviceConfiguration;
Expand Down Expand Up @@ -67,6 +68,7 @@
import org.springframework.context.annotation.Import;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.core.env.Environment;
import org.springframework.graphql.execution.DataFetcherExceptionResolverAdapter;
import org.springframework.scheduling.quartz.SchedulerFactoryBean;
import org.springframework.security.core.context.SecurityContextHolder;
Expand Down Expand Up @@ -439,4 +441,14 @@ private static class SentryTracesSampleRateCondition {}
@SuppressWarnings("UnusedNestedClass")
private static class SentryTracesSamplerBeanCondition {}
}

@Configuration(proxyBeanMethods = false)
@Open
static class SpringProfilesEventProcessorConfiguration {
@Bean
public @NotNull SpringProfilesEventProcessor springProfilesEventProcessor(
final Environment environment) {
return new SpringProfilesEventProcessor(environment);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import io.sentry.spring.HttpServletRequestSentryUserProvider
import io.sentry.spring.SentryExceptionResolver
import io.sentry.spring.SentryUserFilter
import io.sentry.spring.SentryUserProvider
import io.sentry.spring.SpringProfilesEventProcessor
import io.sentry.spring.SpringSecuritySentryUserProvider
import io.sentry.spring.tracing.SentryTracingFilter
import io.sentry.spring.tracing.SpringServletTransactionNameProvider
Expand Down Expand Up @@ -865,6 +866,14 @@ class SentryAutoConfigurationTest {
}
}

@Test
fun `registers SpringProfilesEventProcessor on SentryOptions`() {
contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj")
.run {
assertThat(it.getBean(SentryOptions::class.java).eventProcessors).anyMatch { processor -> processor.javaClass == SpringProfilesEventProcessor::class.java }
}
}

@Configuration(proxyBeanMethods = false)
open class CustomSchedulerFactoryBeanCustomizerConfiguration {
class MyJobListener : JobListener {
Expand Down
7 changes: 7 additions & 0 deletions sentry-spring-jakarta/api/sentry-spring-jakarta.api
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ public class io/sentry/spring/jakarta/SentryWebConfiguration {
public fun httpServletRequestSentryUserProvider (Lio/sentry/SentryOptions;)Lio/sentry/spring/jakarta/HttpServletRequestSentryUserProvider;
}

public final class io/sentry/spring/jakarta/SpringProfilesEventProcessor : io/sentry/EventProcessor {
public fun <init> (Lorg/springframework/core/env/Environment;)V
public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
public fun process (Lio/sentry/SentryReplayEvent;Lio/sentry/Hint;)Lio/sentry/SentryReplayEvent;
public fun process (Lio/sentry/protocol/SentryTransaction;Lio/sentry/Hint;)Lio/sentry/protocol/SentryTransaction;
}

public final class io/sentry/spring/jakarta/SpringSecuritySentryUserProvider : io/sentry/spring/jakarta/SentryUserProvider {
public fun <init> (Lio/sentry/SentryOptions;)V
public fun provideUser ()Lio/sentry/protocol/User;
Expand Down
Loading

0 comments on commit b584414

Please sign in to comment.