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

Fix: Ignore DiagnosticSource Integration if no Sampling available. #1238

Merged
merged 15 commits into from
Oct 12, 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixes

- Ignore DiagnosticSource Integration if no Sampling available ([#1238](https://github.com/getsentry/sentry-dotnet/pull/1238))

### Features

- Add additional primitive values as tags on SentryLogger ([#1246](https://github.com/getsentry/sentry-dotnet/pull/1246))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,16 @@ internal class SentryDiagnosticListenerIntegration : IInternalSdkIntegration

public void Register(IHub hub, SentryOptions options)
{
_subscriber = new SentryDiagnosticSubscriber(hub, options);
_diagnosticListener = DiagnosticListener.AllListeners.Subscribe(_subscriber);
if (options.TracesSampleRate == 0)
{
options.DiagnosticLogger?.Log(SentryLevel.Info, "DiagnosticSource Integration is now disabled due to TracesSampleRate being set to zero.");
Copy link
Member

Choose a reason for hiding this comment

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

Prefer LogInfo instead of Log(Level.Info

options.DisableDiagnosticSourceIntegration();
Copy link
Member

Choose a reason for hiding this comment

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

THe integration instance was already called, why mutate options after the fact to remove it?
It's probably best to remove this line

}
else
{
_subscriber = new SentryDiagnosticSubscriber(hub, options);
_diagnosticListener = DiagnosticListener.AllListeners.Subscribe(_subscriber);
}
}

public void Unregister(IHub hub)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,46 @@ public SentryEFCoreListener(IHub hub, SentryOptions options)

private ISpan? AddSpan(SentryEFSpanType type, string operation, string? description)
{
if (_hub.GetSpan()?.StartChild(operation, description) is { } span &&
GetSpanBucket(type) is { } asyncLocalSpan)
ISpan? span = null;
_hub.ConfigureScope(scope =>
{
asyncLocalSpan.Value = new WeakReference<ISpan>(span);
return span;
}
return null;
if (scope.Transaction?.IsSampled != true)
{
return;
}

if (scope.GetSpan()?.StartChild(operation, description) is not { } startedChild)
{
return;
}

if (GetSpanBucket(type) is not { } asyncLocalSpan)
{
return;
}

asyncLocalSpan.Value = new WeakReference<ISpan>(startedChild);
span = startedChild;
});
return span;
}

private ISpan? TakeSpan(SentryEFSpanType type)
{
if (GetSpanBucket(type)?.Value is { } reference && reference.TryGetTarget(out var span))
ISpan? span = null;
_hub.ConfigureScope(scope =>
{
return span;
}
_options.LogWarning("Trying to close a span that was already garbage collected. {0}", type);
return null;
if (scope.Transaction?.IsSampled == true)
{
if (GetSpanBucket(type)?.Value is { } reference &&
reference.TryGetTarget(out var startedSpan))
{
span = startedSpan;
}
_options.LogWarning("Trying to close a span that was already garbage collected. {0}", type);
}
});
return span;
}

private AsyncLocal<WeakReference<ISpan>>? GetSpanBucket(SentryEFSpanType type)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,63 +77,86 @@ private void SetOperationId(ISpan span, Guid? operationId)
return null;
}

private void AddSpan(SentrySqlSpanType type, string operation, string? description, Guid operationId, Guid? connectionId = null)
private void AddSpan(SentrySqlSpanType type, string operation, KeyValuePair<string, object?> value)
{
_hub.ConfigureScope(scope =>
{
if (scope.Transaction is { } transaction)
{
if (type == SentrySqlSpanType.Connection &&
transaction?.StartChild(operation, description) is { } connectionSpan)
transaction?.StartChild(operation) is { } connectionSpan)
{
SetOperationId(connectionSpan, operationId);
SetOperationId(connectionSpan, value.GetProperty<Guid>(OperationKey));
}
else if (type == SentrySqlSpanType.Execution && connectionId != null)
else if (type == SentrySqlSpanType.Execution && value.GetProperty<Guid>(ConnectionKey) is { } connectionId)
{
var span = TryStartChild(
TryGetConnectionSpan(scope, connectionId.Value) ?? transaction,
TryGetConnectionSpan(scope, connectionId) ?? transaction,
operation,
description);
null);
if (span is not null)
{
SetOperationId(span, operationId);
SetOperationId(span, value.GetProperty<Guid>(OperationKey));
SetConnectionId(span, connectionId);
}
}
}
});
}

private ISpan? GetSpan(SentrySqlSpanType type, Guid? operationId = null, Guid? connectionId = null)
private ISpan? GetSpan(SentrySqlSpanType type, KeyValuePair<string, object?> value)
{
ISpan? span = null;
_hub.ConfigureScope(scope =>
{
if (type == SentrySqlSpanType.Execution &&
operationId is { } queryId &&
TryGetQuerySpan(scope, queryId) is { } querySpan)
if (scope.Transaction == null)
{
span = querySpan;
return;
}

if (span.ParentSpanId == scope.Transaction?.SpanId &&
TryGetConnectionId(span) is { } spanConnectionId &&
spanConnectionId is Guid spanConnectionGuid &&
span is SpanTracer executionTracer &&
TryGetConnectionSpan(scope, spanConnectionGuid) is { } spanConnectionRef)
if (type == SentrySqlSpanType.Execution)
{
var operationId = value.GetProperty<Guid>(OperationKey);
if (TryGetQuerySpan(scope, operationId) is { } querySpan)
{
// Connection Span exist but wasn't set as the parent of the current Span.
executionTracer.ParentSpanId = spanConnectionRef.SpanId;
span = querySpan;

if (span.ParentSpanId == scope.Transaction?.SpanId &&
TryGetConnectionId(span) is { } spanConnectionId &&
spanConnectionId is Guid spanConnectionGuid &&
span is SpanTracer executionTracer &&
TryGetConnectionSpan(scope, spanConnectionGuid) is { } spanConnectionRef)
{
// Connection Span exist but wasn't set as the parent of the current Span.
executionTracer.ParentSpanId = spanConnectionRef.SpanId;
}
}
else
{
_options.DiagnosticLogger?.LogWarning("Trying to get a span of type {0} with operation id {1}, but it was not found.",
type,
operationId);
}
}
else if (type == SentrySqlSpanType.Connection &&
connectionId is { } id &&
else if ((value.Key == SqlMicrosoftWriteConnectionCloseAfterCommand ||
value.Key == SqlDataWriteConnectionCloseAfterCommand) &&
value.GetProperty<Guid>(ConnectionKey) is { } id &&
TryGetConnectionSpan(scope, id) is { } connectionSpan)
{
span = connectionSpan;
}
else if ((value.Key is SqlMicrosoftWriteTransactionCommitAfter ||
value.Key is SqlDataWriteTransactionCommitAfter) &&
value.GetSubProperty<Guid>("Connection", "ClientConnectionId") is { } commitId &&
TryGetConnectionSpan(scope, commitId) is { } commitSpan)
{
span = commitSpan;
}
else
{
_options.LogWarning("Trying to get a span of type {0} with operation id {1}, but it was not found.", type, operationId);
_options.LogWarning("Trying to get a span of type {0} with operation id {1}, but it was not found.",
type,
value.GetProperty<Guid>(OperationKey));
}
});
return span;
Expand Down Expand Up @@ -174,16 +197,16 @@ public void OnNext(KeyValuePair<string, object?> value)
// Query.
if (value.Key == SqlMicrosoftBeforeExecuteCommand || value.Key == SqlDataBeforeExecuteCommand)
{
AddSpan(SentrySqlSpanType.Execution, "db.query", null, value.GetProperty<Guid>(OperationKey), value.GetProperty<Guid>(ConnectionKey));
AddSpan(SentrySqlSpanType.Execution, "db.query", value);
}
else if ((value.Key == SqlMicrosoftAfterExecuteCommand || value.Key == SqlDataAfterExecuteCommand) &&
GetSpan(SentrySqlSpanType.Execution, value.GetProperty<Guid>(OperationKey)) is { } commandSpan)
GetSpan(SentrySqlSpanType.Execution, value) is { } commandSpan)
{
commandSpan.Description = value.GetSubProperty<string>("Command", "CommandText");
commandSpan.Finish(SpanStatus.Ok);
}
else if ((value.Key == SqlMicrosoftWriteCommandError || value.Key == SqlDataWriteCommandError) &&
GetSpan(SentrySqlSpanType.Execution, value.GetProperty<Guid>(OperationKey)) is { } errorSpan)
GetSpan(SentrySqlSpanType.Execution, value) is { } errorSpan)
{
errorSpan.Description = value.GetSubProperty<string>("Command", "CommandText");
errorSpan.Finish(SpanStatus.InternalError);
Expand All @@ -192,21 +215,21 @@ public void OnNext(KeyValuePair<string, object?> value)
// Connection.
else if (value.Key == SqlMicrosoftWriteConnectionOpenBeforeCommand || value.Key == SqlDataWriteConnectionOpenBeforeCommand)
{
AddSpan(SentrySqlSpanType.Connection, "db.connection", null, value.GetProperty<Guid>(OperationKey));
AddSpan(SentrySqlSpanType.Connection, "db.connection", value);
}
else if (value.Key == SqlMicrosoftWriteConnectionOpenAfterCommand || value.Key == SqlDataWriteConnectionOpenAfterCommand)
{
UpdateConnectionSpan(value.GetProperty<Guid>(OperationKey), value.GetProperty<Guid>(ConnectionKey));
}
else if ((value.Key == SqlMicrosoftWriteConnectionCloseAfterCommand ||
value.Key == SqlDataWriteConnectionCloseAfterCommand) &&
GetSpan(SentrySqlSpanType.Connection, null, value.GetProperty<Guid>(ConnectionKey)) is { } connectionSpan)
GetSpan(SentrySqlSpanType.Connection, value) is { } connectionSpan)
{
TrySetConnectionStatistics(connectionSpan, value);
connectionSpan.Finish(SpanStatus.Ok);
}
else if ((value.Key is SqlMicrosoftWriteTransactionCommitAfter || value.Key is SqlDataWriteTransactionCommitAfter) &&
GetSpan(SentrySqlSpanType.Connection, null, value.GetSubProperty<Guid>("Connection", "ClientConnectionId")) is { } connectionSpan2)
GetSpan(SentrySqlSpanType.Connection, value) is { } connectionSpan2)
{
// If some query makes changes to the Database data, CloseAfterCommand event will not be invoked,
// instead, TransactionCommitAfter is invoked.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ private class Fixture
internal SentryScopeManager ScopeManager { get; }
public Fixture()
{
var options = new SentryOptions();
var options = new SentryOptions
{
TracesSampleRate = 1.0
};
ScopeManager = new SentryScopeManager(
new AsyncLocalScopeStackContainer(),
options,
Expand All @@ -35,6 +38,8 @@ public Fixture()
Hub.GetSpan().ReturnsForAnyArgs(_ => GetSpan());
Hub.StartTransaction(Arg.Any<ITransactionContext>(), Arg.Any<IReadOnlyDictionary<string, object>>())
.ReturnsForAnyArgs(callinfo => StartTransaction(Hub, callinfo.Arg<ITransactionContext>()));
Hub.When(hub => hub.ConfigureScope(Arg.Any<Action<Scope>>()))
.Do(callback => callback.Arg<Action<Scope>>().Invoke(ScopeManager.GetCurrent().Key));

DiagnosticListener.AllListeners.Subscribe(new SentryDiagnosticSubscriber(Hub, options));

Expand Down
Loading