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.

Sign up for GitHub

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

Copy options tags to transactions. #1198

Merged
merged 5 commits into from
Jan 22, 2021
Merged

Copy options tags to transactions. #1198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
870cd09
Copy options tags to transactions.
maciejwalkowiak Jan 21, 2021
b118255
Changelog.
maciejwalkowiak Jan 21, 2021
052ba30
Merge branch 'main' into gh-1163
maciejwalkowiak Jan 22, 2021
ca8e771
Merge remote-tracking branch 'origin/main' into gh-1163
maciejwalkowiak Jan 22, 2021
860aeae
Merge branch 'gh-1163' of https://github.com/getsentry/sentry-java in…
maciejwalkowiak Jan 22, 2021
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* Enhancement: Enable Kotlin map-like access on CustomSamplingContext (#1192)
* Enhancement: Auto register custom ITransportFactory in Spring integration (#1194)
* Enhancement: Improve Kotlin property access in Performance API (#1193)
* Enhancement: Copy options tags to transactions (#1198)
* Enhancement: Add convenient method for accessing event's throwable (1202)

# 4.0.0-alpha.3
Expand Down
9 changes: 9 additions & 0 deletions sentry/src/main/java/io/sentry/SentryClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec
if (transaction.getEnvironment() == null) {
transaction.setEnvironment(options.getEnvironment());
}
if (transaction.getTags() == null) {
transaction.setTags(new HashMap<>(options.getTags()));
} else {
for (Map.Entry<String, String> item : options.getTags().entrySet()) {
if (!transaction.getTags().containsKey(item.getKey())) {
transaction.setTag(item.getKey(), item.getValue());
}
}
}
Comment on lines +403 to +411
Copy link
Contributor

Choose a reason for hiding this comment

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

what's about the tags set on the scope? should we also copy them?

cc @bruno-garcia

Copy link
Member

Choose a reason for hiding this comment

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

oof, yeah we're missing that. All tags should be added

return transaction;
}

Expand Down
21 changes: 21 additions & 0 deletions sentry/src/test/java/io/sentry/SentryClientTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,27 @@ class SentryClientTest {
assertEquals("transactionEnvironment", transaction.environment)
}

@Test
fun `when transaction does not have tags, and tags are set on options, options values are applied to transactions`() {
fixture.sentryOptions.setTag("tag1", "value1")
val sut = fixture.getSut()
val transaction = SentryTransaction("name")
sut.captureTransaction(transaction)
assertEquals(mapOf("tag1" to "value1"), transaction.tags)
}

@Test
fun `when transaction has tags, and tags are set on options, options tags are added to transactions`() {
fixture.sentryOptions.setTag("tag1", "value1")
fixture.sentryOptions.setTag("tag2", "value2")
val sut = fixture.getSut()
val transaction = SentryTransaction("name")
transaction.setTag("tag3", "value3")
transaction.setTag("tag2", "transaction-tag")
sut.captureTransaction(transaction)
assertEquals(mapOf("tag1" to "value1", "tag2" to "transaction-tag", "tag3" to "value3"), transaction.tags)
}

private fun createScope(): Scope {
return Scope(SentryOptions()).apply {
addBreadcrumb(Breadcrumb().apply {
Expand Down