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

Remove SentryTransaction from public API #1304

Merged
merged 29 commits into from
Mar 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ab204bd
WIP
maciejwalkowiak Mar 3, 2021
4202dbb
Merge remote-tracking branch 'origin/main' into sentry-tracer
maciejwalkowiak Mar 4, 2021
9a364ec
Refactor Spring modules.
maciejwalkowiak Mar 4, 2021
0445c5d
Deprecate methods.
maciejwalkowiak Mar 4, 2021
8128e8c
Polish.
maciejwalkowiak Mar 4, 2021
e25f346
Revert sentry-native changes
maciejwalkowiak Mar 4, 2021
41a6ba4
Fix android tests.
maciejwalkowiak Mar 4, 2021
89b311b
Polish.
maciejwalkowiak Mar 4, 2021
f810653
Apply scope to transaction.
maciejwalkowiak Mar 4, 2021
319ca38
Fix Span serialization.
maciejwalkowiak Mar 4, 2021
04ee739
Create separate protocol class for span.
maciejwalkowiak Mar 5, 2021
0f6d9a5
Polish.
maciejwalkowiak Mar 5, 2021
a053914
Polish.
maciejwalkowiak Mar 5, 2021
43c260c
Merge branch 'sentry-tracer' of https://github.com/getsentry/sentry-j…
maciejwalkowiak Mar 5, 2021
b0aea8f
Merge remote-tracking branch 'origin/main' into sentry-tracer
maciejwalkowiak Mar 5, 2021
9583179
Remove transient modifier from Span fields.
maciejwalkowiak Mar 5, 2021
e5dcd7b
Add serialization tests.
maciejwalkowiak Mar 5, 2021
395e5a2
Add tests.
maciejwalkowiak Mar 5, 2021
e9b00f1
Deprecate `ITransaction#getContexts`.
maciejwalkowiak Mar 5, 2021
63f75d6
Changelog.
maciejwalkowiak Mar 5, 2021
791bdaa
Fix clearing transaction.
maciejwalkowiak Mar 5, 2021
27f80a1
Add test for fixing clearing transaction.
maciejwalkowiak Mar 5, 2021
5cdee1e
Polish
maciejwalkowiak Mar 5, 2021
389f909
Put request and contexts on SentryTracer.
maciejwalkowiak Mar 8, 2021
ed3e6fb
Polish Javadocs.
maciejwalkowiak Mar 8, 2021
eeb8ed5
Make finish return void.
maciejwalkowiak Mar 8, 2021
1dc5e6e
Remove tag created for keeping transaction name.
maciejwalkowiak Mar 9, 2021
e495ba3
Spotless.
maciejwalkowiak Mar 9, 2021
f2ff5b0
Polish.
maciejwalkowiak Mar 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Fix: Initialize Sentry in Logback appender when DSN is not set in XML config (#1296)
* Fix: Fix JUL integration SDK name (#1293)
* Feat: Activity tracing auto instrumentation
* Ref: Separate user facing and protocol classes in the Performance feature (#1304)

# 4.2.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ import io.sentry.Hub
import io.sentry.Scope
import io.sentry.SentryLevel
import io.sentry.SentryOptions
import io.sentry.SentryTransaction
import io.sentry.SentryTracer
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import io.sentry.protocol.SentryTransaction
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
Expand All @@ -31,7 +33,7 @@ class ActivityLifecycleIntegrationTest {
val options = SentryAndroidOptions()
val activity = mock<Activity>()
val bundle = mock<Bundle>()
val transaction = SentryTransaction("name", "op", hub)
val transaction = SentryTracer(TransactionContext("name", "op"), hub)

fun getSut(): ActivityLifecycleIntegration {
whenever(hub.startTransaction(any<String>(), any())).thenReturn(transaction)
Expand Down Expand Up @@ -256,7 +258,7 @@ class ActivityLifecycleIntegrationTest {

whenever(fixture.hub.configureScope(any())).thenAnswer {
val scope = Scope(fixture.options)
val previousTransaction = SentryTransaction("name", "op", fixture.hub)
val previousTransaction = SentryTracer(TransactionContext("name", "op"), fixture.hub)
scope.setTransaction(previousTransaction)

sut.applyScope(scope, fixture.transaction)
Expand All @@ -278,7 +280,7 @@ class ActivityLifecycleIntegrationTest {

verify(fixture.hub).captureTransaction(check<SentryTransaction> {
assertEquals(SpanStatus.OK, it.status)
}, eq(null))
})
}

@Test
Expand All @@ -295,7 +297,7 @@ class ActivityLifecycleIntegrationTest {

verify(fixture.hub).captureTransaction(check<SentryTransaction> {
assertEquals(SpanStatus.UNKNOWN_ERROR, it.status)
}, eq(null))
})
}

@Test
Expand Down Expand Up @@ -330,7 +332,7 @@ class ActivityLifecycleIntegrationTest {
sut.onActivityPreCreated(fixture.activity, fixture.bundle)
sut.onActivityDestroyed(fixture.activity)

verify(fixture.hub).captureTransaction(any<SentryTransaction>(), eq(null))
verify(fixture.hub).captureTransaction(any<SentryTransaction>())
}

@Test
Expand Down Expand Up @@ -366,6 +368,6 @@ class ActivityLifecycleIntegrationTest {
sut.onActivityPreCreated(activity, mock())

sut.onActivityPreCreated(fixture.activity, fixture.bundle)
verify(fixture.hub).captureTransaction(any<SentryTransaction>(), eq(null))
verify(fixture.hub).captureTransaction(any<SentryTransaction>())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import com.nhaarman.mockitokotlin2.whenever
import io.sentry.IHub
import io.sentry.Scope
import io.sentry.SentryOptions
import io.sentry.SentryTransaction
import io.sentry.SpanContext
import io.sentry.SentryTracer
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import kotlin.test.Test
import org.assertj.core.api.Assertions.assertThat
import org.springframework.http.HttpMethod
Expand All @@ -23,7 +23,7 @@ class SentrySpanRestTemplateCustomizerTest {
val hub = mock<IHub>()
val restTemplate = RestTemplate()
var mockServer = MockRestServiceServer.createServer(restTemplate)
val transaction = SentryTransaction("aTransaction", SpanContext("op", true), hub)
val transaction = SentryTracer(TransactionContext("aTransaction", "op", true), hub)
internal val customizer = SentrySpanRestTemplateCustomizer(hub)

fun getSut(isTransactionActive: Boolean, status: HttpStatus = HttpStatus.OK): RestTemplate {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.sentry.spring.tracing;

import io.sentry.protocol.SentryTransaction;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
Expand All @@ -8,8 +9,8 @@

/**
* Makes annotated method execution or a method execution within a class annotated with {@link
* SentrySpan} executed within running {@link io.sentry.SentryTransaction} to get wrapped into
* {@link io.sentry.Span}.
* SentrySpan} executed within running {@link SentryTransaction} to get wrapped into {@link
* io.sentry.Span}.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.TYPE})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ protected void doFilterInternal(
if (hub.isEnabled()) {
final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER);

hub.configureScope(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ITransaction#setRequest is deprecated now as these kind of properties should be set on the scope and then applied to transaction in SentryClient.

scope -> {
scope.setRequest(requestResolver.resolveSentryRequest(httpRequest));
});

// at this stage we are not able to get real transaction name
final ITransaction transaction = startTransaction(httpRequest, sentryTraceHeader);
try {
Expand All @@ -86,7 +91,6 @@ protected void doFilterInternal(
if (transactionName != null) {
transaction.setName(transactionName);
transaction.setOperation(TRANSACTION_OP);
transaction.setRequest(requestResolver.resolveSentryRequest(httpRequest));
transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus()));
transaction.finish();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

/**
* Makes annotated method execution or a method execution within an annotated class to get wrapped
* into {@link io.sentry.SentryTransaction}.
* into {@link io.sentry.protocol.SentryTransaction}.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.TYPE})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@

import com.jakewharton.nopen.annotation.Open;
import io.sentry.IHub;
import io.sentry.ISpan;
import io.sentry.ITransaction;
import io.sentry.util.Objects;
import java.lang.reflect.Method;
import java.util.concurrent.atomic.AtomicBoolean;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.jetbrains.annotations.ApiStatus;
Expand Down Expand Up @@ -78,16 +76,6 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
}

private boolean isTransactionActive() {
AtomicBoolean isTransactionActiveRef = new AtomicBoolean(false);

hub.configureScope(
scope -> {
ISpan span = scope.getSpan();

if (span != null) {
isTransactionActiveRef.set(true);
}
});
return isTransactionActiveRef.get();
return hub.getSpan() != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import com.nhaarman.mockitokotlin2.whenever
import io.sentry.IHub
import io.sentry.Scope
import io.sentry.SentryOptions
import io.sentry.SentryTransaction
import io.sentry.SpanContext
import io.sentry.SentryTracer
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import java.lang.RuntimeException
Expand Down Expand Up @@ -42,13 +41,13 @@ class SentrySpanAdviceTest {

@BeforeTest
fun setup() {
whenever(hub.startTransaction(any<TransactionContext>(), any())).thenAnswer { SentryTransaction(it.arguments[0] as String, SpanContext(it.arguments[1] as String), hub) }
whenever(hub.startTransaction(any<TransactionContext>(), any())).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) }
}

@Test
fun `when class is annotated with @SentrySpan, every method call attaches span to existing transaction`() {
val scope = Scope(SentryOptions())
val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
scope.setTransaction(tx)

whenever(hub.span).thenReturn(tx)
Expand All @@ -62,7 +61,7 @@ class SentrySpanAdviceTest {
@Test
fun `when class is annotated with @SentrySpan with operation set, every method call attaches span to existing transaction`() {
val scope = Scope(SentryOptions())
val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
scope.setTransaction(tx)

whenever(hub.span).thenReturn(tx)
Expand All @@ -76,7 +75,7 @@ class SentrySpanAdviceTest {
@Test
fun `when method is annotated with @SentrySpan with properties set, attaches span to existing transaction`() {
val scope = Scope(SentryOptions())
val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
scope.setTransaction(tx)

whenever(hub.span).thenReturn(tx)
Expand All @@ -90,7 +89,7 @@ class SentrySpanAdviceTest {
@Test
fun `when method is annotated with @SentrySpan without properties set, attaches span to existing transaction and sets Span description as className dot methodName`() {
val scope = Scope(SentryOptions())
val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
scope.setTransaction(tx)

whenever(hub.span).thenReturn(tx)
Expand All @@ -104,7 +103,7 @@ class SentrySpanAdviceTest {
@Test
fun `when method is annotated with @SentrySpan and returns, attached span has status OK`() {
val scope = Scope(SentryOptions())
val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
scope.setTransaction(tx)

whenever(hub.span).thenReturn(tx)
Expand All @@ -115,7 +114,7 @@ class SentrySpanAdviceTest {
@Test
fun `when method is annotated with @SentrySpan and throws exception, attached span has throwable set and INTERNAL_ERROR status`() {
val scope = Scope(SentryOptions())
val tx = SentryTransaction("aTransaction", SpanContext("op"), hub)
val tx = SentryTracer(TransactionContext("aTransaction", "op"), hub)
scope.setTransaction(tx)

whenever(hub.span).thenReturn(tx)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import com.nhaarman.mockitokotlin2.verifyNoMoreInteractions
import com.nhaarman.mockitokotlin2.verifyZeroInteractions
import com.nhaarman.mockitokotlin2.whenever
import io.sentry.IHub
import io.sentry.Scope
import io.sentry.SentryOptions
import io.sentry.SentryTransaction
import io.sentry.SpanContext
import io.sentry.SentryTracer
import io.sentry.SpanId
import io.sentry.SpanStatus
import io.sentry.TransactionContext
Expand Down Expand Up @@ -47,17 +47,33 @@ class SentryTracingFilterTest {
request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}")
if (sentryTraceHeader != null) {
request.addHeader("sentry-trace", sentryTraceHeader)
whenever(hub.startTransaction(any<TransactionContext>(), any())).thenAnswer { SentryTransaction((it.arguments[0] as TransactionContext).name, it.arguments[0] as SpanContext, hub) }
whenever(hub.startTransaction(any<TransactionContext>(), any())).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) }
}
response.status = 200
whenever(hub.startTransaction(any<String>(), any(), any())).thenAnswer { SentryTransaction(it.arguments[0] as String, SpanContext(it.arguments[1] as String), hub) }
whenever(hub.startTransaction(any(), any(), any())).thenAnswer { SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
whenever(hub.isEnabled).thenReturn(isEnabled)
return SentryTracingFilter(hub, sentryRequestResolver, transactionNameProvider)
}
}

private val fixture = Fixture()

@Test
fun `sets the request on the scope`() {
val filter = fixture.getSut()

filter.doFilter(fixture.request, fixture.response, fixture.chain)

verify(fixture.chain).doFilter(fixture.request, fixture.response)
verify(fixture.hub).configureScope(check {
val scope = mock<Scope>()
it.run(scope)
verify(scope).setRequest(check { request ->
assertThat(request.url).isEqualTo("http://localhost/product/12")
})
})
}

@Test
fun `creates transaction around the request`() {
val filter = fixture.getSut()
Expand All @@ -73,8 +89,7 @@ class SentryTracingFilterTest {
assertThat(it.transaction).isEqualTo("POST /product/{id}")
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
assertThat(it.contexts.trace!!.operation).isEqualTo("http.server")
assertThat(it.request).isNotNull()
}, eq(null))
})
}

@Test
Expand All @@ -85,7 +100,7 @@ class SentryTracingFilterTest {

verify(fixture.hub).captureTransaction(check {
assertThat(it.contexts.trace!!.parentSpanId).isNull()
}, eq(null))
})
}

@Test
Expand All @@ -97,7 +112,7 @@ class SentryTracingFilterTest {

verify(fixture.hub).captureTransaction(check {
assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId)
}, eq(null))
})
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package io.sentry.spring.tracing

import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.check
import com.nhaarman.mockitokotlin2.eq
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.times
import com.nhaarman.mockitokotlin2.verify
Expand All @@ -11,7 +10,7 @@ import io.sentry.IHub
import io.sentry.Scope
import io.sentry.ScopeCallback
import io.sentry.SentryOptions
import io.sentry.SpanContext
import io.sentry.TransactionContext
import kotlin.test.BeforeTest
import kotlin.test.Test
import org.assertj.core.api.Assertions.assertThat
Expand Down Expand Up @@ -42,7 +41,7 @@ class SentryTransactionAdviceTest {

@BeforeTest
fun setup() {
whenever(hub.startTransaction(any<String>(), any())).thenAnswer { io.sentry.SentryTransaction(it.arguments[0] as String, SpanContext(it.arguments[1] as String), hub) }
whenever(hub.startTransaction(any<String>(), any())).thenAnswer { io.sentry.SentryTracer(TransactionContext(it.arguments[0] as String, it.arguments[1] as String), hub) }
}

@Test
Expand All @@ -51,7 +50,7 @@ class SentryTransactionAdviceTest {
verify(hub).captureTransaction(check {
assertThat(it.transaction).isEqualTo("customName")
assertThat(it.contexts.trace!!.operation).isEqualTo("bean")
}, eq(null))
})
}

@Test
Expand All @@ -60,13 +59,13 @@ class SentryTransactionAdviceTest {
verify(hub).captureTransaction(check {
assertThat(it.transaction).isEqualTo("SampleService.methodWithoutTransactionNameSet")
assertThat(it.contexts.trace!!.operation).isEqualTo("op")
}, eq(null))
})
}

@Test
fun `when transaction is already active, does not start new transaction`() {
val scope = Scope(SentryOptions())
scope.setTransaction(io.sentry.SentryTransaction("aTransaction", SpanContext("op"), hub))
scope.setTransaction(io.sentry.SentryTracer(TransactionContext("aTransaction", "op"), hub))

whenever(hub.configureScope(any())).thenAnswer {
(it.arguments[0] as ScopeCallback).run(scope)
Expand All @@ -82,7 +81,7 @@ class SentryTransactionAdviceTest {
verify(hub).captureTransaction(check {
assertThat(it.transaction).isEqualTo("ClassAnnotatedSampleService.hello")
assertThat(it.contexts.trace!!.operation).isEqualTo("op")
}, eq(null))
})
}

@Test
Expand All @@ -91,7 +90,7 @@ class SentryTransactionAdviceTest {
verify(hub).captureTransaction(check {
assertThat(it.transaction).isEqualTo("ClassAnnotatedWithOperationSampleService.hello")
assertThat(it.contexts.trace!!.operation).isEqualTo("my-op")
}, eq(null))
})
}

@Configuration
Expand Down