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

Jump to bottom

Fix inheriting sampling decision from parent. #1100

Merged
merged 3 commits into from
Dec 11, 2020
Merged

Fix inheriting sampling decision from parent. #1100

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
920d40e
Fix inheriting sampling decision from parent.
maciejwalkowiak Dec 9, 2020
b63929c
Changelog.
maciejwalkowiak Dec 9, 2020
66d4608
Merge branch 'main' into gh-1099
maciejwalkowiak Dec 11, 2020
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# vNext

* Fix inheriting sampling decision from parent (#1100)

# 4.0.0-alpha.2

* Feat: Add basic support for attachments (#1082)
Expand Down
23 changes: 12 additions & 11 deletions sentry/src/main/java/io/sentry/Hub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,20 @@ public void flush(long timeoutMillis) {
@Override
public @Nullable SentryTransaction startTransaction(
final @NotNull TransactionContext transactionContexts) {
return this.startTransaction(transactionContexts, null);
}

@Override
public @Nullable SentryTransaction startTransaction(
final @NotNull TransactionContext transactionContexts,
final @Nullable CustomSamplingContext customSamplingContext) {
Objects.requireNonNull(transactionContexts, "transactionContexts is required");

final SamplingContext samplingContext =
new SamplingContext(transactionContexts, customSamplingContext);
boolean samplingDecision = tracingSampler.sample(samplingContext);
transactionContexts.setSampled(samplingDecision);

SentryTransaction transaction = null;
if (!isEnabled()) {
options
Expand All @@ -706,17 +718,6 @@ public void flush(long timeoutMillis) {
return transaction;
}

@Override
public @Nullable SentryTransaction startTransaction(
final @NotNull TransactionContext transactionContexts,
final @Nullable CustomSamplingContext customSamplingContext) {
final SamplingContext samplingContext =
new SamplingContext(transactionContexts, customSamplingContext);
boolean samplingDecision = tracingSampler.sample(samplingContext);
transactionContexts.setSampled(samplingDecision);
return this.startTransaction(transactionContexts);
}

@Override
public @Nullable SentryTraceHeader traceHeaders() {
SentryTraceHeader traceHeader = null;
Expand Down
11 changes: 11 additions & 0 deletions sentry/src/test/java/io/sentry/HubTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,17 @@ class HubTest {
}
}

@Test
fun `when startTransaction with parent sampled and no traces sampler provided, transaction inherits sampling decision`() {
val hub = generateHub()
val transactionContext = TransactionContext("name")
transactionContext.parentSampled = true
val transaction = hub.startTransaction(transactionContext)
assertNotNull(transaction)
assertNotNull(transaction.isSampled)
assertTrue(transaction.isSampled!!)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need !! here? Kotlin should do it automatically for you as you did assertNotNull(transaction.isSampled) before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we wouldn't if its immutable. But unfortunately it is mutable.

}

@Test
fun `Hub should close the sentry executor processor on close call`() {
val executor = mock<ISentryExecutorService>()
Expand Down