Skip to content

Commit a1bfa95

Browse files
adinaueradinauer
authored
Hubs / Scopes Merge 16 - Move client and throwable to span map to scope (#3318)
* replace hub with scopes * Add Scopes * Introduce `IScopes` interface. * Replace `IHub` with `IScopes` in core * Replace `IHub` with `IScopes` in android core * Replace `IHub` with `IScopes` in android integrations * Replace `IHub` with `IScopes` in apollo integrations * Replace `IHub` with `IScopes` in okhttp integration * Replace `IHub` with `IScopes` in graphql integration * Replace `IHub` with `IScopes` in logging integrations * Replace `IHub` with `IScopes` in more integrations * Replace `IHub` with `IScopes` in OTel integration * Replace `IHub` with `IScopes` in Spring 5 / Spring Boot 2 integrations * Replace `IHub` with `IScopes` in Spring 6 / Spring Boot 3 integrations * Replace `IHub` with `IScopes` in samples * gitscopes -> github * Replace ThreadLocal with ScopesStorage * Move client and throwable to span map to scope
1 parent edb4be2 commit a1bfa95

File tree

5 files changed

+105
-59
lines changed

5 files changed

+105
-59
lines changed

‎sentry/api/sentry.api‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,8 @@ public abstract interface class io/sentry/IScope {
660660
public abstract fun addBreadcrumb (Lio/sentry/Breadcrumb;)V
661661
public abstract fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V
662662
public abstract fun addEventProcessor (Lio/sentry/EventProcessor;)V
663+
public abstract fun assignTraceContext (Lio/sentry/SentryEvent;)V
664+
public abstract fun bindClient (Lio/sentry/ISentryClient;)V
663665
public abstract fun clear ()V
664666
public abstract fun clearAttachments ()V
665667
public abstract fun clearBreadcrumbs ()V
@@ -703,6 +705,7 @@ public abstract interface class io/sentry/IScope {
703705
public abstract fun setPropagationContext (Lio/sentry/PropagationContext;)V
704706
public abstract fun setRequest (Lio/sentry/protocol/Request;)V
705707
public abstract fun setScreen (Ljava/lang/String;)V
708+
public abstract fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)V
706709
public abstract fun setTag (Ljava/lang/String;Ljava/lang/String;)V
707710
public abstract fun setTransaction (Lio/sentry/ITransaction;)V
708711
public abstract fun setTransaction (Ljava/lang/String;)V
@@ -1298,6 +1301,8 @@ public final class io/sentry/NoOpScope : io/sentry/IScope {
12981301
public fun addBreadcrumb (Lio/sentry/Breadcrumb;)V
12991302
public fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V
13001303
public fun addEventProcessor (Lio/sentry/EventProcessor;)V
1304+
public fun assignTraceContext (Lio/sentry/SentryEvent;)V
1305+
public fun bindClient (Lio/sentry/ISentryClient;)V
13011306
public fun clear ()V
13021307
public fun clearAttachments ()V
13031308
public fun clearBreadcrumbs ()V
@@ -1343,6 +1348,7 @@ public final class io/sentry/NoOpScope : io/sentry/IScope {
13431348
public fun setPropagationContext (Lio/sentry/PropagationContext;)V
13441349
public fun setRequest (Lio/sentry/protocol/Request;)V
13451350
public fun setScreen (Ljava/lang/String;)V
1351+
public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)V
13461352
public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
13471353
public fun setTransaction (Lio/sentry/ITransaction;)V
13481354
public fun setTransaction (Ljava/lang/String;)V
@@ -1731,6 +1737,8 @@ public final class io/sentry/Scope : io/sentry/IScope {
17311737
public fun addBreadcrumb (Lio/sentry/Breadcrumb;)V
17321738
public fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V
17331739
public fun addEventProcessor (Lio/sentry/EventProcessor;)V
1740+
public fun assignTraceContext (Lio/sentry/SentryEvent;)V
1741+
public fun bindClient (Lio/sentry/ISentryClient;)V
17341742
public fun clear ()V
17351743
public fun clearAttachments ()V
17361744
public fun clearBreadcrumbs ()V
@@ -1775,6 +1783,7 @@ public final class io/sentry/Scope : io/sentry/IScope {
17751783
public fun setPropagationContext (Lio/sentry/PropagationContext;)V
17761784
public fun setRequest (Lio/sentry/protocol/Request;)V
17771785
public fun setScreen (Ljava/lang/String;)V
1786+
public fun setSpanContext (Ljava/lang/Throwable;Lio/sentry/ISpan;Ljava/lang/String;)V
17781787
public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
17791788
public fun setTransaction (Lio/sentry/ITransaction;)V
17801789
public fun setTransaction (Ljava/lang/String;)V

‎sentry/src/main/java/io/sentry/IScope.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,17 @@ public interface IScope {
377377
@NotNull
378378
SentryId getLastEventId();
379379

380-
void setClient(final @NotNull ISentryClient client);
380+
void bindClient(final @NotNull ISentryClient client);
381381

382382
@NotNull
383383
ISentryClient getClient();
384+
385+
@ApiStatus.Internal
386+
void assignTraceContext(final @NotNull SentryEvent event);
387+
388+
@ApiStatus.Internal
389+
void setSpanContext(
390+
final @NotNull Throwable throwable,
391+
final @NotNull ISpan span,
392+
final @NotNull String transactionName);
384393
}

‎sentry/src/main/java/io/sentry/NoOpScope.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,17 @@ public void setLastEventId(@NotNull SentryId lastEventId) {}
256256
}
257257

258258
@Override
259-
public void setClient(@NotNull ISentryClient client) {}
259+
public void bindClient(@NotNull ISentryClient client) {}
260260

261261
@Override
262262
public @NotNull ISentryClient getClient() {
263263
return NoOpSentryClient.getInstance();
264264
}
265+
266+
@Override
267+
public void assignTraceContext(@NotNull SentryEvent event) {}
268+
269+
@Override
270+
public void setSpanContext(
271+
@NotNull Throwable throwable, @NotNull ISpan span, @NotNull String transactionName) {}
265272
}

‎sentry/src/main/java/io/sentry/Scope.java‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,18 @@
77
import io.sentry.protocol.TransactionNameSource;
88
import io.sentry.protocol.User;
99
import io.sentry.util.CollectionUtils;
10+
import io.sentry.util.ExceptionUtils;
1011
import io.sentry.util.Objects;
12+
import io.sentry.util.Pair;
13+
import java.lang.ref.WeakReference;
1114
import java.util.ArrayList;
1215
import java.util.Collection;
16+
import java.util.Collections;
1317
import java.util.HashMap;
1418
import java.util.List;
1519
import java.util.Map;
1620
import java.util.Queue;
21+
import java.util.WeakHashMap;
1722
import java.util.concurrent.ConcurrentHashMap;
1823
import java.util.concurrent.CopyOnWriteArrayList;
1924
import org.jetbrains.annotations.ApiStatus;
@@ -85,6 +90,11 @@ public final class Scope implements IScope {
8590

8691
private @NotNull ISentryClient client = NoOpSentryClient.getInstance();
8792

93+
// TODO intended only for global scope
94+
// TODO test for memory leak
95+
private final @NotNull Map<Throwable, Pair<WeakReference<ISpan>, String>> throwableToSpan =
96+
Collections.synchronizedMap(new WeakHashMap<>());
97+
8898
/**
8999
* Scope's ctor
90100
*
@@ -103,6 +113,7 @@ private Scope(final @NotNull Scope scope) {
103113
this.session = scope.session;
104114
this.options = scope.options;
105115
this.level = scope.level;
116+
this.client = scope.client;
106117
// TODO should we do this? didn't do it for Hub
107118
this.lastEventId = scope.getLastEventId();
108119

@@ -964,7 +975,7 @@ public void setLastEventId(@NotNull SentryId lastEventId) {
964975
}
965976

966977
@Override
967-
public void setClient(@NotNull ISentryClient client) {
978+
public void bindClient(@NotNull ISentryClient client) {
968979
this.client = client;
969980
}
970981

@@ -973,6 +984,45 @@ public void setClient(@NotNull ISentryClient client) {
973984
return client;
974985
}
975986

987+
@Override
988+
@ApiStatus.Internal
989+
public void assignTraceContext(final @NotNull SentryEvent event) {
990+
if (options.isTracingEnabled() && event.getThrowable() != null) {
991+
final Pair<WeakReference<ISpan>, String> pair =
992+
throwableToSpan.get(ExceptionUtils.findRootCause(event.getThrowable()));
993+
if (pair != null) {
994+
final WeakReference<ISpan> spanWeakRef = pair.getFirst();
995+
if (event.getContexts().getTrace() == null && spanWeakRef != null) {
996+
final ISpan span = spanWeakRef.get();
997+
if (span != null) {
998+
event.getContexts().setTrace(span.getSpanContext());
999+
}
1000+
}
1001+
final String transactionName = pair.getSecond();
1002+
if (event.getTransaction() == null && transactionName != null) {
1003+
event.setTransaction(transactionName);
1004+
}
1005+
}
1006+
}
1007+
}
1008+
1009+
@Override
1010+
@ApiStatus.Internal
1011+
public void setSpanContext(
1012+
final @NotNull Throwable throwable,
1013+
final @NotNull ISpan span,
1014+
final @NotNull String transactionName) {
1015+
Objects.requireNonNull(throwable, "throwable is required");
1016+
Objects.requireNonNull(span, "span is required");
1017+
Objects.requireNonNull(transactionName, "transactionName is required");
1018+
// to match any cause, span context is always attached to the root cause of the exception
1019+
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
1020+
// the most inner span should be assigned to a throwable
1021+
if (!throwableToSpan.containsKey(rootCause)) {
1022+
throwableToSpan.put(rootCause, new Pair<>(new WeakReference<>(span), transactionName));
1023+
}
1024+
}
1025+
9761026
/** The IWithTransaction callback */
9771027
@ApiStatus.Internal
9781028
public interface IWithTransaction {

‎sentry/src/main/java/io/sentry/Scopes.java‎

Lines changed: 27 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,15 @@
99
import io.sentry.protocol.SentryTransaction;
1010
import io.sentry.protocol.User;
1111
import io.sentry.transport.RateLimiter;
12-
import io.sentry.util.ExceptionUtils;
1312
import io.sentry.util.HintUtils;
1413
import io.sentry.util.Objects;
15-
import io.sentry.util.Pair;
1614
import io.sentry.util.TracingUtils;
1715
import java.io.Closeable;
1816
import java.io.IOException;
19-
import java.lang.ref.WeakReference;
2017
import java.util.Collections;
2118
import java.util.HashMap;
2219
import java.util.List;
2320
import java.util.Map;
24-
import java.util.WeakHashMap;
2521
import org.jetbrains.annotations.ApiStatus;
2622
import org.jetbrains.annotations.NotNull;
2723
import org.jetbrains.annotations.Nullable;
@@ -40,10 +36,6 @@ public final class Scopes implements IScopes, MetricsApi.IMetricsInterface {
4036
private final @NotNull SentryOptions options;
4137
private volatile boolean isEnabled;
4238
private final @NotNull TracesSampler tracesSampler;
43-
44-
// TODO should this go on global scope?
45-
private final @NotNull Map<Throwable, Pair<WeakReference<ISpan>, String>> throwableToSpan =
46-
Collections.synchronizedMap(new WeakHashMap<>());
4739
private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector;
4840
private final @NotNull MetricsApi metricsApi;
4941

@@ -120,7 +112,10 @@ public boolean isAncestorOf(final @Nullable Scopes otherScopes) {
120112

121113
// TODO add to IScopes interface
122114
public @NotNull Scopes forkedCurrentScope(final @NotNull String creator) {
123-
return new Scopes(scope.clone(), isolationScope, this, options, creator);
115+
IScope clone = scope.clone();
116+
// TODO should use isolation scope
117+
// return new Scopes(clone, isolationScope, this, options, creator);
118+
return new Scopes(clone, clone, this, options, creator);
124119
}
125120

126121
// // TODO in Sentry.init?
@@ -180,23 +175,7 @@ public boolean isEnabled() {
180175
}
181176

182177
private void assignTraceContext(final @NotNull SentryEvent event) {
183-
if (options.isTracingEnabled() && event.getThrowable() != null) {
184-
final Pair<WeakReference<ISpan>, String> pair =
185-
throwableToSpan.get(ExceptionUtils.findRootCause(event.getThrowable()));
186-
if (pair != null) {
187-
final WeakReference<ISpan> spanWeakRef = pair.getFirst();
188-
if (event.getContexts().getTrace() == null && spanWeakRef != null) {
189-
final ISpan span = spanWeakRef.get();
190-
if (span != null) {
191-
event.getContexts().setTrace(span.getSpanContext());
192-
}
193-
}
194-
final String transactionName = pair.getSecond();
195-
if (event.getTransaction() == null && transactionName != null) {
196-
event.setTransaction(transactionName);
197-
}
198-
}
199-
}
178+
Sentry.getGlobalScope().assignTraceContext(event);
200179
}
201180

202181
private IScope buildLocalScope(
@@ -691,10 +670,10 @@ public void bindClient(final @NotNull ISentryClient client) {
691670
} else {
692671
if (client != null) {
693672
options.getLogger().log(SentryLevel.DEBUG, "New client bound to scope.");
694-
getDefaultWriteScope().setClient(client);
673+
getDefaultWriteScope().bindClient(client);
695674
} else {
696675
options.getLogger().log(SentryLevel.DEBUG, "NoOp client bound to scope.");
697-
getDefaultWriteScope().setClient(NoOpSentryClient.getInstance());
676+
getDefaultWriteScope().bindClient(NoOpSentryClient.getInstance());
698677
}
699678
}
700679
}
@@ -871,34 +850,26 @@ public void setSpanContext(
871850
final @NotNull Throwable throwable,
872851
final @NotNull ISpan span,
873852
final @NotNull String transactionName) {
874-
Objects.requireNonNull(throwable, "throwable is required");
875-
Objects.requireNonNull(span, "span is required");
876-
Objects.requireNonNull(transactionName, "transactionName is required");
877-
// to match any cause, span context is always attached to the root cause of the exception
878-
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
879-
// the most inner span should be assigned to a throwable
880-
if (!throwableToSpan.containsKey(rootCause)) {
881-
throwableToSpan.put(rootCause, new Pair<>(new WeakReference<>(span), transactionName));
882-
}
883-
}
884-
885-
// TODO this seems unused
886-
@Nullable
887-
SpanContext getSpanContext(final @NotNull Throwable throwable) {
888-
Objects.requireNonNull(throwable, "throwable is required");
889-
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
890-
final Pair<WeakReference<ISpan>, String> pair = this.throwableToSpan.get(rootCause);
891-
if (pair != null) {
892-
final WeakReference<ISpan> spanWeakRef = pair.getFirst();
893-
if (spanWeakRef != null) {
894-
final ISpan span = spanWeakRef.get();
895-
if (span != null) {
896-
return span.getSpanContext();
897-
}
898-
}
899-
}
900-
return null;
901-
}
853+
Sentry.getGlobalScope().setSpanContext(throwable, span, transactionName);
854+
}
855+
856+
// // TODO this seems unused
857+
// @Nullable
858+
// SpanContext getSpanContext(final @NotNull Throwable throwable) {
859+
// Objects.requireNonNull(throwable, "throwable is required");
860+
// final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
861+
// final Pair<WeakReference<ISpan>, String> pair = this.throwableToSpan.get(rootCause);
862+
// if (pair != null) {
863+
// final WeakReference<ISpan> spanWeakRef = pair.getFirst();
864+
// if (spanWeakRef != null) {
865+
// final ISpan span = spanWeakRef.get();
866+
// if (span != null) {
867+
// return span.getSpanContext();
868+
// }
869+
// }
870+
// }
871+
// return null;
872+
// }
902873

903874
@Override
904875
public @Nullable ISpan getSpan() {

0 commit comments

Comments
 (0)