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

feat: Add addToTransactions to Attachment #1191

Merged
merged 2 commits into from
Jan 21, 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
@@ -1,5 +1,6 @@
# vNext

* Feat: Add addToTransactions to Attachment (#1191)
* Enhancement: Support SENTRY_TRACES_SAMPLE_RATE conf. via env variables (#1171)
* Enhancement: Pass request to CustomSamplingContext in Spring integration (#1172)
* Ref: Set SpanContext on SentryTransaction to avoid potential NPE (#1173)
Expand Down
2 changes: 2 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ public final class io/sentry/Attachment {
public fun <init> (Ljava/lang/String;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Z)V
public fun <init> ([BLjava/lang/String;)V
public fun <init> ([BLjava/lang/String;Ljava/lang/String;)V
public fun <init> ([BLjava/lang/String;Ljava/lang/String;Z)V
public fun getBytes ()[B
public fun getContentType ()Ljava/lang/String;
public fun getFilename ()Ljava/lang/String;
Expand Down
64 changes: 60 additions & 4 deletions sentry/src/main/java/io/sentry/Attachment.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public final class Attachment {
private @Nullable String pathname;
private final @NotNull String filename;
private final @NotNull String contentType;
private final boolean addToTransactions;

/**
* We could use Files.probeContentType(path) to determine the content type of the filename. This
Expand All @@ -24,7 +25,8 @@ public final class Attachment {
private static final String DEFAULT_CONTENT_TYPE = "application/octet-stream";

/**
* Initializes an Attachment with bytes and a filename.
* Initializes an Attachment with bytes and a filename. Sets addToTransaction to <code>false
* </code>.
*
* @param bytes The bytes of file.
* @param filename The name of the attachment to display in Sentry.
Expand All @@ -34,7 +36,8 @@ public Attachment(final @NotNull byte[] bytes, final @NotNull String filename) {
}

/**
* Initializes an Attachment with bytes, filename and content type.
* Initializes an Attachment with bytes, a filename, and a content type. Sets addToTransaction to
* <code>false</code>.
*
* @param bytes The bytes of file.
* @param filename The name of the attachment to display in Sentry.
Expand All @@ -44,13 +47,32 @@ public Attachment(
final @NotNull byte[] bytes,
final @NotNull String filename,
final @NotNull String contentType) {
this(bytes, filename, contentType, false);
}

/**
* Initializes an Attachment with bytes, a filename, a content type, and addToTransactions.
*
* @param bytes The bytes of file.
* @param filename The name of the attachment to display in Sentry.
* @param contentType The content type of the attachment.
* @param addToTransactions <code>true</code> if the SDK should add this attachment to every
* {@link ITransaction} or set to <code>false</code> if it shouldn't.
*/
public Attachment(
final @NotNull byte[] bytes,
final @NotNull String filename,
final @NotNull String contentType,
final boolean addToTransactions) {
this.bytes = bytes;
this.filename = filename;
this.contentType = contentType;
this.addToTransactions = addToTransactions;
}

/**
* Initializes an Attachment with a path. The filename of the file located at the path is used.
* Sets addToTransaction to <code>false</code>.
*
* <p>The file located at the pathname is read lazily when the SDK captures an event or
* transaction not when the attachment is initialized. The pathname string is converted into an
Expand All @@ -63,7 +85,8 @@ public Attachment(final @NotNull String pathname) {
}

/**
* Initializes an Attachment with a path and a filename.
* Initializes an Attachment with a path and a filename. Sets addToTransaction to <code>false
* </code>.
*
* <p>The file located at the pathname is read lazily when the SDK captures an event or
* transaction not when the attachment is initialized. The pathname string is converted into an
Expand All @@ -77,7 +100,8 @@ public Attachment(final @NotNull String pathname, final @NotNull String filename
}

/**
* Initializes an Attachment with a path, a filename and a content type.
* Initializes an Attachment with a path, a filename, and a content type. Sets addToTransaction to
* <code>false</code>.
*
* <p>The file located at the pathname is read lazily when the SDK captures an event or
* transaction not when the attachment is initialized. The pathname string is converted into an
Expand All @@ -91,9 +115,31 @@ public Attachment(
final @NotNull String pathname,
final @NotNull String filename,
final @NotNull String contentType) {
this(pathname, filename, contentType, false);
}

/**
* Initializes an Attachment with a path, a filename, a content type, and addToTransactions.
*
* <p>The file located at the pathname is read lazily when the SDK captures an event or
* transaction not when the attachment is initialized. The pathname string is converted into an
* abstract pathname before reading the file.
*
* @param pathname The pathname string of the file to upload as an attachment.
* @param filename The name of the attachment to display in Sentry.
* @param contentType The content type of the attachment.
* @param addToTransactions <code>true</code> if the SDK should add this attachment to every
* {@link ITransaction} or set to <code>false</code> if it shouldn't.
*/
public Attachment(
final @NotNull String pathname,
final @NotNull String filename,
final @NotNull String contentType,
final boolean addToTransactions) {
this.pathname = pathname;
this.filename = filename;
this.contentType = contentType;
this.addToTransactions = addToTransactions;
}

/**
Expand Down Expand Up @@ -131,4 +177,14 @@ public Attachment(
public @NotNull String getContentType() {
return contentType;
}

/**
* Returns <code>true</code> if the SDK should add this attachment to every {@link ITransaction}
* and <code>false</code> if it shouldn't. Default is <code>false</code>.
*
* @return <code>true</code> if attachment should be added to every {@link ITransaction}.
*/
boolean isAddToTransactions() {
return addToTransactions;
}
}
17 changes: 16 additions & 1 deletion sentry/src/main/java/io/sentry/SentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
processTransaction((SentryTransaction) transaction);
try {
final SentryEnvelope envelope =
buildEnvelope(sentryTransaction, getAttachmentsFromScope(scope));
buildEnvelope(sentryTransaction, filterForTransaction(getAttachmentsFromScope(scope)));
if (envelope != null) {
transport.send(envelope, hint);
} else {
Expand All @@ -377,6 +377,21 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
return sentryId;
}

private @Nullable List<Attachment> filterForTransaction(@Nullable List<Attachment> attachments) {
if (attachments == null) {
return null;
}

List<Attachment> attachmentsToSend = new ArrayList<>();
for (Attachment attachment : attachments) {
if (attachment.isAddToTransactions()) {
attachmentsToSend.add(attachment);
}
}

return attachmentsToSend;
}

private @NotNull SentryTransaction processTransaction(
final @NotNull SentryTransaction transaction) {
if (transaction.getRelease() == null) {
Expand Down
20 changes: 20 additions & 0 deletions sentry/src/test/java/io/sentry/AttachmentTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package io.sentry

import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNull
import kotlin.test.assertTrue

class AttachmentTest {

Expand Down Expand Up @@ -71,4 +73,22 @@ class AttachmentTest {
val byteAttachment = Attachment(fixture.bytes, fixture.filename, fixture.contentType)
assertEquals(fixture.contentType, byteAttachment.contentType)
}

@Test
fun `default of addToTransactions is false`() {
val fileAttachment = Attachment(fixture.filename)
assertFalse(fileAttachment.isAddToTransactions)

val byteAttachment = Attachment(fixture.bytes, fixture.filename)
assertFalse(byteAttachment.isAddToTransactions)
}

@Test
fun `set addToTransactions`() {
val fileAttachment = Attachment(fixture.pathname, fixture.filename, fixture.contentType, true)
assertTrue(fileAttachment.isAddToTransactions)

val byteAttachment = Attachment(fixture.bytes, fixture.filename, fixture.contentType, true)
assertTrue(byteAttachment.isAddToTransactions)
}
}
15 changes: 13 additions & 2 deletions sentry/src/test/java/io/sentry/SentryClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class SentryClientTest {
whenever(factory.create(any(), any())).thenReturn(transport)
}

var attachment = Attachment("hello".toByteArray(), "hello.txt")
var attachment = Attachment("hello".toByteArray(), "hello.txt", "text/plain", true)

fun getSut() = SentryClient(sentryOptions)
}
Expand Down Expand Up @@ -763,6 +763,17 @@ class SentryClientTest {
verifyAttachmentsInEnvelope(transaction.eventId)
}

@Test
fun `when captureTransaction with attachments not added to transaction`() {
val transaction = SentryTransaction("a-transaction")

val scope = createScopeWithAttachments()
scope.addAttachment(Attachment("hello".toByteArray(), "application/octet-stream"))
fixture.getSut().captureTransaction(transaction, scope, null)

verifyAttachmentsInEnvelope(transaction.eventId)
}

@Test
fun `when scope's active span is a transaction, transaction context is applied to an event`() {
val event = SentryEvent()
Expand Down Expand Up @@ -834,7 +845,7 @@ class SentryClientTest {
addAttachment(fixture.attachment)

val bytesTooBig = ByteArray((fixture.maxAttachmentSize + 1).toInt()) { 0 }
addAttachment(Attachment(bytesTooBig, "will_get_dropped.txt"))
addAttachment(Attachment(bytesTooBig, "will_get_dropped.txt", "application/octet-stream", true))
}
}

Expand Down