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

Improve ITransaction and ISpan null-safety compatibility #1161

Merged
merged 4 commits into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* Fix: Set release and environment on Transactions (#1152)
* Fix: Do not set transaction on the scope automatically
* Enhancement: Automatically assign span context to captured events (#1156)
* Enhancement: Improve ITransaction and ISpan null-safety compatibility (#1161)

# 4.0.0-alpha.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,15 @@ class SentryAppenderTest {

await.untilAsserted {
verify(fixture.transport).send(checkEvent { event ->
assertEquals(BuildConfig.SENTRY_LOG4J2_SDK_NAME, event.sdk.name)
assertEquals(BuildConfig.VERSION_NAME, event.sdk.version)
assertNotNull(event.sdk.packages)
assertTrue(event.sdk.packages!!.any { pkg ->
"maven:sentry-log4j2" == pkg.name &&
BuildConfig.VERSION_NAME == pkg.version
})
assertNotNull(event.sdk) {
assertEquals(BuildConfig.SENTRY_LOG4J2_SDK_NAME, it.name)
assertEquals(BuildConfig.VERSION_NAME, it.version)
assertNotNull(it.packages)
assertTrue(it.packages!!.any { pkg ->
"maven:sentry-log4j2" == pkg.name &&
BuildConfig.VERSION_NAME == pkg.version
})
}
}, anyOrNull())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,15 @@ class SentryAppenderTest {

await.untilAsserted {
verify(fixture.transport).send(checkEvent { event ->
assertEquals(BuildConfig.SENTRY_LOGBACK_SDK_NAME, event.sdk.name)
assertEquals(BuildConfig.VERSION_NAME, event.sdk.version)
assertNotNull(event.sdk.packages)
assertTrue(event.sdk.packages!!.any { pkg ->
"maven:sentry-logback" == pkg.name &&
BuildConfig.VERSION_NAME == pkg.version
})
assertNotNull(event.sdk) {
assertEquals(BuildConfig.SENTRY_LOGBACK_SDK_NAME, it.name)
assertEquals(BuildConfig.VERSION_NAME, it.version)
assertNotNull(it.packages)
assertTrue(it.packages!!.any { pkg ->
"maven:sentry-logback" == pkg.name &&
BuildConfig.VERSION_NAME == pkg.version
})
}
}, anyOrNull())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import java.net.URI
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
import org.springframework.mock.web.MockServletContext
Expand All @@ -25,13 +26,15 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertEquals("GET", event.request.method)
assertNotNull(event.request)
val eventRequest = event.request!!
assertEquals("GET", eventRequest.method)
assertEquals(mapOf(
"some-header" to "some-header value",
"Accept" to "application/json"
), event.request.headers)
assertEquals("http://example.com", event.request.url)
assertEquals("param1=xyz", event.request.queryString)
), eventRequest.headers)
assertEquals("http://example.com", eventRequest.url)
assertEquals("param1=xyz", eventRequest.queryString)
}

@Test
Expand All @@ -46,9 +49,11 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertEquals(mapOf(
"another-header" to "another value,another value2"
), event.request.headers)
assertNotNull(event.request) {
assertEquals(mapOf(
"another-header" to "another value,another value2"
), it.headers)
}
}

@Test
Expand All @@ -64,7 +69,9 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertNull(event.request.cookies)
assertNotNull(event.request) {
assertNull(it.cookies)
}
}

@Test
Expand All @@ -84,10 +91,12 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertFalse(event.request.headers.containsKey("X-FORWARDED-FOR"))
assertFalse(event.request.headers.containsKey("Authorization"))
assertFalse(event.request.headers.containsKey("authorization"))
assertFalse(event.request.headers.containsKey("Cookies"))
assertTrue(event.request.headers.containsKey("some-header"))
assertNotNull(event.request) {
assertFalse(it.headers.containsKey("X-FORWARDED-FOR"))
assertFalse(it.headers.containsKey("Authorization"))
assertFalse(it.headers.containsKey("authorization"))
assertFalse(it.headers.containsKey("Cookies"))
assertTrue(it.headers.containsKey("some-header"))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,11 @@ class SentryAutoConfigurationTest {
val transport = it.getBean(ITransport::class.java)
await.untilAsserted {
verify(transport).send(checkEvent { event ->
assertThat(event.sdk.version).isEqualTo(BuildConfig.VERSION_NAME)
assertThat(event.sdk.name).isEqualTo(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME)
assertThat(event.sdk.packages).anyMatch { pkg ->
assertThat(event.sdk).isNotNull()
val sdk = event.sdk!!
assertThat(sdk.version).isEqualTo(BuildConfig.VERSION_NAME)
assertThat(sdk.name).isEqualTo(BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME)
assertThat(sdk.packages).anyMatch { pkg ->
pkg.name == "maven:sentry-spring-boot-starter" && pkg.version == BuildConfig.VERSION_NAME
}
}, anyOrNull())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class SentrySpringIntegrationTest {
await.untilAsserted {
verify(transport).send(checkEvent { event ->
assertThat(event.request).isNotNull()
assertThat(event.request.url).isEqualTo("http://localhost:$port/hello")
assertThat(event.request!!.url).isEqualTo("http://localhost:$port/hello")
assertThat(event.user).isNotNull()
assertThat(event.user.username).isEqualTo("user")
assertThat(event.user.ipAddress).isEqualTo("169.128.0.1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertEquals("GET", event.request.method)
assertEquals(mapOf(
"some-header" to "some-header value",
"Accept" to "application/json"
), event.request.headers)
assertEquals("http://example.com", event.request.url)
assertEquals("param1=xyz", event.request.queryString)
assertNotNull(event.request) {
assertEquals("GET", it.method)
assertEquals(mapOf(
"some-header" to "some-header value",
"Accept" to "application/json"
), it.headers)
assertEquals("http://example.com", it.url)
assertEquals("param1=xyz", it.queryString)
}
}

@Test
Expand All @@ -49,9 +51,11 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertEquals(mapOf(
"another-header" to "another value,another value2"
), event.request.headers)
assertNotNull(event.request) {
assertEquals(mapOf(
"another-header" to "another value,another value2"
), it.headers)
}
}

@Test
Expand All @@ -68,7 +72,9 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertEquals("name=value,name2=value2", event.request.cookies)
assertNotNull(event.request) {
assertEquals("name=value,name2=value2", it.cookies)
}
}

@Test
Expand All @@ -84,7 +90,9 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertNull(event.request.cookies)
assertNotNull(event.request) {
assertNull(it.cookies)
}
}

@Test
Expand All @@ -104,11 +112,13 @@ class SentryRequestHttpServletRequestProcessorTest {

eventProcessor.process(event, null)

assertFalse(event.request.headers.containsKey("X-FORWARDED-FOR"))
assertFalse(event.request.headers.containsKey("Authorization"))
assertFalse(event.request.headers.containsKey("authorization"))
assertFalse(event.request.headers.containsKey("Cookie"))
assertTrue(event.request.headers.containsKey("some-header"))
assertNotNull(event.request) {
assertFalse(it.headers.containsKey("X-FORWARDED-FOR"))
assertFalse(it.headers.containsKey("Authorization"))
assertFalse(it.headers.containsKey("authorization"))
assertFalse(it.headers.containsKey("Cookie"))
assertTrue(it.headers.containsKey("some-header"))
}
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class SentrySpringIntegrationTest {
await.untilAsserted {
verify(transport).send(checkEvent { event ->
assertThat(event.request).isNotNull()
assertThat(event.request.url).isEqualTo("http://localhost:$port/hello")
assertThat(event.request!!.url).isEqualTo("http://localhost:$port/hello")
assertThat(event.user).isNotNull()
assertThat(event.user.username).isEqualTo("user")
assertThat(event.user.ipAddress).isEqualTo("169.128.0.1")
Expand Down
3 changes: 0 additions & 3 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan {
public abstract fun getSpans ()Ljava/util/List;
public abstract fun getTransaction ()Ljava/lang/String;
public abstract fun isSampled ()Ljava/lang/Boolean;
public abstract fun setContexts (Lio/sentry/protocol/Contexts;)V
public abstract fun setName (Ljava/lang/String;)V
public abstract fun setRequest (Lio/sentry/protocol/Request;)V
public abstract fun startChild (Lio/sentry/SpanId;)Lio/sentry/Span;
Expand Down Expand Up @@ -354,7 +353,6 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
public fun getThrowable ()Ljava/lang/Throwable;
public fun getTransaction ()Ljava/lang/String;
public fun isSampled ()Ljava/lang/Boolean;
public fun setContexts (Lio/sentry/protocol/Contexts;)V
public fun setDescription (Ljava/lang/String;)V
public fun setName (Ljava/lang/String;)V
public fun setOperation (Ljava/lang/String;)V
Expand Down Expand Up @@ -526,7 +524,6 @@ public abstract class io/sentry/SentryBaseEvent {
public fun getTag (Ljava/lang/String;)Ljava/lang/String;
public fun getThrowable ()Ljava/lang/Throwable;
public fun removeTag (Ljava/lang/String;)V
public fun setContexts (Lio/sentry/protocol/Contexts;)V
public fun setEnvironment (Ljava/lang/String;)V
public fun setEventId (Lio/sentry/protocol/SentryId;)V
public fun setRelease (Ljava/lang/String;)V
Expand Down
6 changes: 1 addition & 5 deletions sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import io.sentry.Stack.StackItem;
import io.sentry.hints.SessionEndHint;
import io.sentry.hints.SessionStartHint;
import io.sentry.protocol.Contexts;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.User;
import io.sentry.util.Objects;
Expand Down Expand Up @@ -174,10 +173,7 @@ private void assignTraceContext(final @NotNull SentryEvent event) {
if (event.getThrowable() != null) {
final SpanContext spanContext = throwableToSpanContext.get(event.getThrowable());
if (spanContext != null) {
if (event.getContexts() == null) {
event.setContexts(new Contexts());
event.getContexts().setTrace(spanContext);
} else if (event.getContexts().getTrace() == null) {
if (event.getContexts().getTrace() == null) {
event.getContexts().setTrace(spanContext);
}
}
Expand Down
5 changes: 4 additions & 1 deletion sentry/src/main/java/io/sentry/ISpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public interface ISpan {
*
* @return a new transaction span
*/
@NotNull
Span startChild();

/**
Expand All @@ -19,13 +20,15 @@ public interface ISpan {
* @param description - new span description name
* @return a new transaction span
*/
Span startChild(String operation, String description);
@NotNull
Span startChild(@NotNull String operation, @NotNull String description);

/**
* Returns a string that could be sent as a sentry-trace header.
*
* @return SentryTraceHeader.
*/
@NotNull
SentryTraceHeader toSentryTrace();

/** Sets span timestamp marking this span as finished. */
Expand Down
7 changes: 4 additions & 3 deletions sentry/src/main/java/io/sentry/ITransaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public interface ITransaction extends ISpan {
/**
* Starts a child Span.
*
* @param parentSpanId - parent span id
* @return a new transaction span
*/
@NotNull
Expand All @@ -45,19 +46,19 @@ Span startChild(
*
* @param request the request
*/
void setRequest(Request request);
void setRequest(@Nullable Request request);

/**
* Returns the request information from the transaction
*
* @return the request or {@code null} if not set
*/
@Nullable
Request getRequest();

@NotNull
Contexts getContexts();

void setContexts(Contexts contexts);

/**
* Returns the transaction's description.
*
Expand Down
Loading