-
-
Notifications
You must be signed in to change notification settings - Fork 225
QOL: Allow finishing spans and transactions with using
#4627
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
base: version6
Are you sure you want to change the base?
Changes from all commits
0adcefc
0ac4ade
0b0fdff
4e5f03d
69ce439
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,10 +6,11 @@ namespace Sentry; | |
/// <summary> | ||
/// Transaction span tracer. | ||
/// </summary> | ||
public class SpanTracer : IBaseTracer, ISpan | ||
public sealed class SpanTracer : IBaseTracer, ISpan | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Is it worth a consideration to make So that users cannot run into var span = new SpanTracer(null!, transaction, default, default, null!); //CA2000
span.Finish(); Or is it a potential user code scenario to try to cast? if (SentrySdk.StartSpan("operation", "description") is SpanTracer span) But this could have quite a large impact. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: CHANGELOG Is it worth a note in the CHANGELOG that both
are no longer inheritable? |
||
{ | ||
private readonly IHub _hub; | ||
private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); | ||
private string? _origin; | ||
|
||
private readonly Instrumenter _instrumenter = Instrumenter.Sentry; | ||
|
||
|
@@ -190,5 +191,13 @@ internal set | |
_origin = value; | ||
} | ||
} | ||
private string? _origin; | ||
|
||
/// <summary> | ||
/// Automatically finishes the span at the end of a <c>using</c> block. This is a convenience method only. Disposing | ||
/// is not required (and analyser warnings are suppressed). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion:
|
||
/// </summary> | ||
public void Dispose() | ||
{ | ||
Finish(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,13 +7,14 @@ namespace Sentry; | |
/// <summary> | ||
/// Transaction tracer. | ||
/// </summary> | ||
public class TransactionTracer : IBaseTracer, ITransactionTracer | ||
public sealed class TransactionTracer : IBaseTracer, ITransactionTracer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Is it worth a consideration to make So that users cannot run into var transaction = new TransactionTracer(null!, null!); //CA2000
transaction.Finish(); Or is it a potential user code scenario to try to cast? if (SentrySdk.StartTransaction("name", "operation") is TransactionTracer transaction) But this could have quite a large impact. |
||
{ | ||
private readonly IHub _hub; | ||
private readonly SentryOptions? _options; | ||
private readonly Timer? _idleTimer; | ||
private long _cancelIdleTimeout; | ||
private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); | ||
private int _hasFinished = 0; | ||
|
||
private readonly Instrumenter _instrumenter = Instrumenter.Sentry; | ||
|
||
|
@@ -361,6 +362,12 @@ public void Clear() | |
/// <inheritdoc /> | ||
public void Finish() | ||
{ | ||
// TODO: Replace with InterlockedBoolean once this has been merged into version6 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO (as a "resolvable" comment) |
||
if (Interlocked.Exchange(ref _hasFinished, 1) == 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Shall we create a dedicated test for double-Disposal / double-Finishing? Or is it actually sufficiently covered already by |
||
{ | ||
return; | ||
} | ||
|
||
_options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); | ||
if (Interlocked.Exchange(ref _cancelIdleTimeout, 0) == 1) | ||
{ | ||
|
@@ -431,4 +438,13 @@ private void ReleaseSpans() | |
#endif | ||
_activeSpanTracker.Clear(); | ||
} | ||
|
||
/// <summary> | ||
/// Automatically finishes the span at the end of a <c>using</c> block. This is a convenience method only. Disposing | ||
/// is not required (and analyser warnings are suppressed). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion:
|
||
/// </summary> | ||
public void Dispose() | ||
{ | ||
Finish(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion:
IDispose
->IDisposable