Skip to content

Commit d007225

Browse files
lblodergetsentry-botadinauer
authored
Resolve spring properties in @SentryCheckIn annotation (#3194)
* resolve spring properties in @SentryCheckIn annotation * format * add changelog * guard against exceptions when resolving property, add test * Format code * Update sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java Co-authored-by: Alexander Dinauer <[email protected]> * Format code * cr changes, fix tests --------- Co-authored-by: Sentry Github Bot <[email protected]> Co-authored-by: Alexander Dinauer <[email protected]>
1 parent 95a98b5 commit d007225

7 files changed

Lines changed: 278 additions & 7 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- Add new threshold parameters to monitor config ([#3181](https://github.com/getsentry/sentry-java/pull/3181))
88
- Report process init time as a span for app start performance ([#3159](https://github.com/getsentry/sentry-java/pull/3159))
99
- (perf-v2): Calculate frame delay on a span level ([#3197](https://github.com/getsentry/sentry-java/pull/3197))
10+
- Resolve spring properties in @SentryCheckIn annotation ([#3194](https://github.com/getsentry/sentry-java/pull/3194))
1011

1112
### Fixes
1213

‎sentry-spring-jakarta/api/sentry-spring-jakarta.api‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,11 @@ public abstract interface annotation class io/sentry/spring/jakarta/checkin/Sent
9494
public abstract fun value ()Ljava/lang/String;
9595
}
9696

97-
public class io/sentry/spring/jakarta/checkin/SentryCheckInAdvice : org/aopalliance/intercept/MethodInterceptor {
97+
public class io/sentry/spring/jakarta/checkin/SentryCheckInAdvice : org/aopalliance/intercept/MethodInterceptor, org/springframework/context/EmbeddedValueResolverAware {
9898
public fun <init> ()V
9999
public fun <init> (Lio/sentry/IHub;)V
100100
public fun invoke (Lorg/aopalliance/intercept/MethodInvocation;)Ljava/lang/Object;
101+
public fun setEmbeddedValueResolver (Lorg/springframework/util/StringValueResolver;)V
101102
}
102103

103104
public class io/sentry/spring/jakarta/checkin/SentryCheckInAdviceConfiguration {

‎sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/checkin/SentryCheckInAdvice.java‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
import org.jetbrains.annotations.NotNull;
1818
import org.jetbrains.annotations.Nullable;
1919
import org.springframework.aop.support.AopUtils;
20+
import org.springframework.context.EmbeddedValueResolverAware;
2021
import org.springframework.core.annotation.AnnotationUtils;
2122
import org.springframework.util.ObjectUtils;
23+
import org.springframework.util.StringValueResolver;
2224

2325
/**
2426
* Reports execution of every bean method annotated with {@link SentryCheckIn} as a monitor
@@ -27,9 +29,11 @@
2729
@ApiStatus.Internal
2830
@ApiStatus.Experimental
2931
@Open
30-
public class SentryCheckInAdvice implements MethodInterceptor {
32+
public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueResolverAware {
3133
private final @NotNull IHub hub;
3234

35+
private @Nullable StringValueResolver resolver;
36+
3337
public SentryCheckInAdvice() {
3438
this(HubAdapter.getInstance());
3539
}
@@ -51,7 +55,25 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
5155
}
5256

5357
final boolean isHeartbeatOnly = checkInAnnotation.heartbeat();
54-
final @Nullable String monitorSlug = checkInAnnotation.value();
58+
59+
@Nullable String monitorSlug = checkInAnnotation.value();
60+
61+
if (resolver != null) {
62+
try {
63+
monitorSlug = resolver.resolveStringValue(checkInAnnotation.value());
64+
} catch (Throwable e) {
65+
// When resolving fails, we fall back to the original string which may contain unresolved
66+
// expressions. Testing shows this can also happen if properties cannot be resolved (without
67+
// an exception being thrown). Sentry should alert the user about missed checkins in this
68+
// case since the monitor slug won't match what is configured in Sentry.
69+
hub.getOptions()
70+
.getLogger()
71+
.log(
72+
SentryLevel.WARNING,
73+
"Slug for method annotated with @SentryCheckIn could not be resolved from properties.",
74+
e);
75+
}
76+
}
5577

5678
if (ObjectUtils.isEmpty(monitorSlug)) {
5779
hub.getOptions()
@@ -85,4 +107,9 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
85107
hub.popScope();
86108
}
87109
}
110+
111+
@Override
112+
public void setEmbeddedValueResolver(StringValueResolver resolver) {
113+
this.resolver = resolver;
114+
}
88115
}

‎sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryCheckInAdviceTest.kt‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,24 @@ import org.mockito.kotlin.times
2121
import org.mockito.kotlin.verify
2222
import org.mockito.kotlin.whenever
2323
import org.springframework.beans.factory.annotation.Autowired
24+
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory
2425
import org.springframework.context.annotation.Bean
2526
import org.springframework.context.annotation.Configuration
2627
import org.springframework.context.annotation.EnableAspectJAutoProxy
2728
import org.springframework.context.annotation.Import
29+
import org.springframework.context.support.PropertySourcesPlaceholderConfigurer
30+
import org.springframework.test.context.TestPropertySource
2831
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig
2932
import org.springframework.test.context.junit4.SpringRunner
30-
import kotlin.RuntimeException
33+
import org.springframework.util.StringValueResolver
3134
import kotlin.test.BeforeTest
3235
import kotlin.test.Test
3336
import kotlin.test.assertEquals
3437
import kotlin.test.assertNotNull
3538

3639
@RunWith(SpringRunner::class)
3740
@SpringJUnitConfig(SentryCheckInAdviceTest.Config::class)
41+
@TestPropertySource(properties = ["my.cron.slug = mypropertycronslug"])
3842
class SentryCheckInAdviceTest {
3943

4044
@Autowired
@@ -46,6 +50,9 @@ class SentryCheckInAdviceTest {
4650
@Autowired
4751
lateinit var sampleServiceHeartbeat: SampleServiceHeartbeat
4852

53+
@Autowired
54+
lateinit var sampleServiceSpringProperties: SampleServiceSpringProperties
55+
4956
@Autowired
5057
lateinit var hub: IHub
5158

@@ -157,6 +164,66 @@ class SentryCheckInAdviceTest {
157164
verify(hub, never()).popScope()
158165
}
159166

167+
@Test
168+
fun `when @SentryCheckIn is passed a spring property it is resolved correctly`() {
169+
val checkInId = SentryId()
170+
val checkInCaptor = argumentCaptor<CheckIn>()
171+
whenever(hub.captureCheckIn(checkInCaptor.capture())).thenReturn(checkInId)
172+
val result = sampleServiceSpringProperties.hello()
173+
assertEquals(1, result)
174+
assertEquals(1, checkInCaptor.allValues.size)
175+
176+
val doneCheckIn = checkInCaptor.lastValue
177+
assertEquals("mypropertycronslug", doneCheckIn.monitorSlug)
178+
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
179+
assertNotNull(doneCheckIn.duration)
180+
181+
val order = inOrder(hub)
182+
order.verify(hub).pushScope()
183+
order.verify(hub).captureCheckIn(any())
184+
order.verify(hub).popScope()
185+
}
186+
187+
@Test
188+
fun `when @SentryCheckIn is passed a spring property that does not exist, raw value is used`() {
189+
val checkInId = SentryId()
190+
val checkInCaptor = argumentCaptor<CheckIn>()
191+
whenever(hub.captureCheckIn(checkInCaptor.capture())).thenReturn(checkInId)
192+
val result = sampleServiceSpringProperties.helloUnresolvedProperty()
193+
assertEquals(1, result)
194+
assertEquals(1, checkInCaptor.allValues.size)
195+
196+
val doneCheckIn = checkInCaptor.lastValue
197+
assertEquals("\${my.cron.unresolved.property}", doneCheckIn.monitorSlug)
198+
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
199+
assertNotNull(doneCheckIn.duration)
200+
201+
val order = inOrder(hub)
202+
order.verify(hub).pushScope()
203+
order.verify(hub).captureCheckIn(any())
204+
order.verify(hub).popScope()
205+
}
206+
207+
@Test
208+
fun `when @SentryCheckIn is passed a spring property that causes an exception, raw value is used`() {
209+
val checkInId = SentryId()
210+
val checkInCaptor = argumentCaptor<CheckIn>()
211+
whenever(hub.captureCheckIn(checkInCaptor.capture())).thenReturn(checkInId)
212+
val result = sampleServiceSpringProperties.helloExceptionProperty()
213+
assertEquals(1, result)
214+
assertEquals(1, checkInCaptor.allValues.size)
215+
216+
val doneCheckIn = checkInCaptor.lastValue
217+
assertEquals("\${my.cron.exception.property}", doneCheckIn.monitorSlug)
218+
assertEquals(CheckInStatus.OK.apiName(), doneCheckIn.status)
219+
assertNotNull(doneCheckIn.duration)
220+
221+
val order = inOrder(hub)
222+
order.verify(hub).pushScope()
223+
order.verify(hub).captureCheckIn(any())
224+
order.verify(hub).popScope()
225+
}
226+
160227
@Configuration
161228
@EnableAspectJAutoProxy(proxyTargetClass = true)
162229
@Import(SentryCheckInAdviceConfiguration::class, SentryCheckInPointcutConfiguration::class)
@@ -171,12 +238,21 @@ class SentryCheckInAdviceTest {
171238
@Bean
172239
open fun sampleServiceHeartbeat() = SampleServiceHeartbeat()
173240

241+
@Bean
242+
open fun sampleServiceSpringProperties() = SampleServiceSpringProperties()
243+
174244
@Bean
175245
open fun hub(): IHub {
176246
val hub = mock<IHub>()
177247
Sentry.setCurrentHub(hub)
178248
return hub
179249
}
250+
251+
companion object {
252+
@Bean
253+
@JvmStatic
254+
fun propertySourcesPlaceholderConfigurer() = MyPropertyPlaceholderConfigurer()
255+
}
180256
}
181257

182258
open class SampleService {
@@ -206,4 +282,33 @@ class SentryCheckInAdviceTest {
206282
throw RuntimeException("thrown on purpose")
207283
}
208284
}
285+
286+
open class SampleServiceSpringProperties {
287+
288+
@SentryCheckIn("\${my.cron.slug}", heartbeat = true)
289+
open fun hello() = 1
290+
291+
@SentryCheckIn("\${my.cron.unresolved.property}", heartbeat = true)
292+
open fun helloUnresolvedProperty() = 1
293+
294+
@SentryCheckIn("\${my.cron.exception.property}", heartbeat = true)
295+
open fun helloExceptionProperty() = 1
296+
}
297+
298+
class MyPropertyPlaceholderConfigurer : PropertySourcesPlaceholderConfigurer() {
299+
300+
override fun doProcessProperties(
301+
beanFactoryToProcess: ConfigurableListableBeanFactory,
302+
valueResolver: StringValueResolver
303+
) {
304+
val wrappedResolver = StringValueResolver { strVal: String ->
305+
if ("\${my.cron.exception.property}".equals(strVal)) {
306+
throw IllegalArgumentException("Cannot resolve property: $strVal")
307+
} else {
308+
valueResolver.resolveStringValue(strVal)
309+
}
310+
}
311+
super.doProcessProperties(beanFactoryToProcess, wrappedResolver)
312+
}
313+
}
209314
}

‎sentry-spring/api/sentry-spring.api‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,11 @@ public abstract interface annotation class io/sentry/spring/checkin/SentryCheckI
9494
public abstract fun value ()Ljava/lang/String;
9595
}
9696

97-
public class io/sentry/spring/checkin/SentryCheckInAdvice : org/aopalliance/intercept/MethodInterceptor {
97+
public class io/sentry/spring/checkin/SentryCheckInAdvice : org/aopalliance/intercept/MethodInterceptor, org/springframework/context/EmbeddedValueResolverAware {
9898
public fun <init> ()V
9999
public fun <init> (Lio/sentry/IHub;)V
100100
public fun invoke (Lorg/aopalliance/intercept/MethodInvocation;)Ljava/lang/Object;
101+
public fun setEmbeddedValueResolver (Lorg/springframework/util/StringValueResolver;)V
101102
}
102103

103104
public class io/sentry/spring/checkin/SentryCheckInAdviceConfiguration {

‎sentry-spring/src/main/java/io/sentry/spring/checkin/SentryCheckInAdvice.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
import org.jetbrains.annotations.NotNull;
1818
import org.jetbrains.annotations.Nullable;
1919
import org.springframework.aop.support.AopUtils;
20+
import org.springframework.context.EmbeddedValueResolverAware;
2021
import org.springframework.core.annotation.AnnotationUtils;
2122
import org.springframework.util.ObjectUtils;
23+
import org.springframework.util.StringValueResolver;
2224

2325
/**
2426
* Reports execution of every bean method annotated with {@link SentryCheckIn} as a monitor
@@ -27,9 +29,11 @@
2729
@ApiStatus.Internal
2830
@ApiStatus.Experimental
2931
@Open
30-
public class SentryCheckInAdvice implements MethodInterceptor {
32+
public class SentryCheckInAdvice implements MethodInterceptor, EmbeddedValueResolverAware {
3133
private final @NotNull IHub hub;
3234

35+
private @Nullable StringValueResolver resolver;
36+
3337
public SentryCheckInAdvice() {
3438
this(HubAdapter.getInstance());
3539
}
@@ -51,7 +55,28 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
5155
}
5256

5357
final boolean isHeartbeatOnly = checkInAnnotation.heartbeat();
54-
final @Nullable String monitorSlug = checkInAnnotation.value();
58+
59+
@Nullable String monitorSlug = checkInAnnotation.value();
60+
61+
if (resolver != null) {
62+
try {
63+
monitorSlug = resolver.resolveStringValue(checkInAnnotation.value());
64+
} catch (Throwable e) {
65+
// When resolving fails, we fall back to the original string which may contain unresolved
66+
// expressions.
67+
// Testing shows this can also happen if properties cannot be resolved (without an exception
68+
// being thrown).
69+
// Sentry should alert the user about missed checkins in this case since the monitor slug
70+
// won't match
71+
// what is configured in Sentry.
72+
hub.getOptions()
73+
.getLogger()
74+
.log(
75+
SentryLevel.WARNING,
76+
"Slug for method annotated with @SentryCheckIn could not be resolved from properties.",
77+
e);
78+
}
79+
}
5580

5681
if (ObjectUtils.isEmpty(monitorSlug)) {
5782
hub.getOptions()
@@ -85,4 +110,9 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
85110
hub.popScope();
86111
}
87112
}
113+
114+
@Override
115+
public void setEmbeddedValueResolver(StringValueResolver resolver) {
116+
this.resolver = resolver;
117+
}
88118
}

0 commit comments

Comments
 (0)