Skip to content

Commit

Permalink
[SqlClient] Include instance in db.namespace and activity name, sta…
Browse files Browse the repository at this point in the history
…rt activity with required tags (#2277)
  • Loading branch information
alanwest authored Oct 30, 2024
1 parent 0351a0b commit 72d1030
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 107 deletions.
8 changes: 7 additions & 1 deletion src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
specification for more information regarding the new database
semantic conventions for
[spans](https://github.com/open-telemetry/semantic-conventions/blob/v1.28.0/docs/database/database-spans.md).
([#2229](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2229))
([#2229](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2229),
[#2277](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2277))
* **Breaking change**: The `peer.service` and `server.socket.address` attributes
are no longer emitted. Users should rely on the `server.address` attribute
for the same information. Note that `server.address` is only included when
Expand All @@ -37,6 +38,11 @@
([#2233](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2233))
* The `EnableConnectionLevelAttributes` option is now enabled by default.
([#2249](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2249))
* The following attributes are now provided when starting an activity for a database
call: `db.system`, `db.name` (old conventions), `db.namespace` (new conventions),
`server.address`, and `server.port`. These attributes are now available for sampling
decisions.
([#2277](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2277))

## 1.9.0-beta.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,41 +20,72 @@ internal sealed class SqlActivitySourceHelper
public static readonly AssemblyName AssemblyName = Assembly.GetName();
public static readonly string ActivitySourceName = AssemblyName.Name!;
public static readonly ActivitySource ActivitySource = new(ActivitySourceName, Assembly.GetPackageVersion());
public static readonly string ActivityName = ActivitySourceName + ".Execute";

public static readonly IEnumerable<KeyValuePair<string, object?>> CreationTags = new[]
public static TagList GetTagListFromConnectionInfo(string? dataSource, string? databaseName, SqlClientTraceInstrumentationOptions options, out string activityName)
{
new KeyValuePair<string, object?>(SemanticConventions.AttributeDbSystem, MicrosoftSqlServerDatabaseSystemName),
};
activityName = MicrosoftSqlServerDatabaseSystemName;

public static void AddConnectionLevelDetailsToActivity(string dataSource, Activity activity, SqlClientTraceInstrumentationOptions options)
{
// TODO: The attributes added here are required. We need to consider
// collecting these attributes by default.
if (options.EnableConnectionLevelAttributes)
var tags = new TagList
{
var connectionDetails = SqlConnectionDetails.ParseFromDataSource((string)dataSource);
{ SemanticConventions.AttributeDbSystem, MicrosoftSqlServerDatabaseSystemName },
};

// TODO: In the new conventions, instance name should now be captured
// as a part of db.namespace, when available.
if (options.EmitOldAttributes && !string.IsNullOrEmpty(connectionDetails.InstanceName))
if (options.EnableConnectionLevelAttributes && dataSource != null)
{
var connectionDetails = SqlConnectionDetails.ParseFromDataSource(dataSource);

if (options.EmitOldAttributes && !string.IsNullOrEmpty(databaseName))
{
tags.Add(SemanticConventions.AttributeDbName, databaseName);
activityName = databaseName!;
}

if (options.EmitNewAttributes && !string.IsNullOrEmpty(databaseName))
{
var dbNamespace = !string.IsNullOrEmpty(connectionDetails.InstanceName)
? $"{connectionDetails.InstanceName}.{databaseName}" // TODO: Refactor SqlConnectionDetails to include database to avoid string allocation here.
: databaseName!;
tags.Add(SemanticConventions.AttributeDbNamespace, dbNamespace);
activityName = dbNamespace;
}

var serverAddress = connectionDetails.ServerHostName ?? connectionDetails.ServerIpAddress;
if (!string.IsNullOrEmpty(serverAddress))
{
activity.SetTag(SemanticConventions.AttributeDbMsSqlInstanceName, connectionDetails.InstanceName);
tags.Add(SemanticConventions.AttributeServerAddress, serverAddress);
if (connectionDetails.Port.HasValue)
{
tags.Add(SemanticConventions.AttributeServerPort, connectionDetails.Port);
}

if (activityName == MicrosoftSqlServerDatabaseSystemName)
{
activityName = connectionDetails.Port.HasValue
? $"{serverAddress}:{connectionDetails.Port}" // TODO: Another opportunity to refactor SqlConnectionDetails
: serverAddress!;
}
}

if (!string.IsNullOrEmpty(connectionDetails.ServerHostName))
if (options.EmitOldAttributes && !string.IsNullOrEmpty(connectionDetails.InstanceName))
{
activity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerHostName);
tags.Add(SemanticConventions.AttributeDbMsSqlInstanceName, connectionDetails.InstanceName);
}
else
}
else if (!string.IsNullOrEmpty(databaseName))
{
if (options.EmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeServerAddress, connectionDetails.ServerIpAddress);
tags.Add(SemanticConventions.AttributeDbNamespace, databaseName);
}

if (connectionDetails.Port.HasValue)
if (options.EmitOldAttributes)
{
activity.SetTag(SemanticConventions.AttributeServerPort, connectionDetails.Port);
tags.Add(SemanticConventions.AttributeDbName, databaseName);
}

activityName = databaseName!;
}

return tags;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ internal sealed class SqlClientDiagnosticListener : ListenerHandler

private readonly PropertyFetcher<object> commandFetcher = new("Command");
private readonly PropertyFetcher<object> connectionFetcher = new("Connection");
private readonly PropertyFetcher<object> dataSourceFetcher = new("DataSource");
private readonly PropertyFetcher<object> databaseFetcher = new("Database");
private readonly PropertyFetcher<string> dataSourceFetcher = new("DataSource");
private readonly PropertyFetcher<string> databaseFetcher = new("Database");
private readonly PropertyFetcher<CommandType> commandTypeFetcher = new("CommandType");
private readonly PropertyFetcher<object> commandTextFetcher = new("CommandText");
private readonly PropertyFetcher<Exception> exceptionFetcher = new("Exception");
Expand All @@ -50,27 +50,30 @@ public override void OnEventWritten(string name, object? payload)
case SqlDataBeforeExecuteCommand:
case SqlMicrosoftBeforeExecuteCommand:
{
// SqlClient does not create an Activity. So the activity coming in here will be null or the root span.
_ = this.commandFetcher.TryFetch(payload, out var command);
if (command == null)
{
SqlClientInstrumentationEventSource.Log.NullPayload(nameof(SqlClientDiagnosticListener), name);
return;
}

_ = this.connectionFetcher.TryFetch(command, out var connection);
_ = this.databaseFetcher.TryFetch(connection, out var databaseName);
_ = this.dataSourceFetcher.TryFetch(connection, out var dataSource);

var startTags = SqlActivitySourceHelper.GetTagListFromConnectionInfo(dataSource, databaseName, this.options, out var activityName);
activity = SqlActivitySourceHelper.ActivitySource.StartActivity(
SqlActivitySourceHelper.ActivityName,
activityName,
ActivityKind.Client,
default(ActivityContext),
SqlActivitySourceHelper.CreationTags);
startTags);

if (activity == null)
{
// There is no listener or it decided not to sample the current request.
return;
}

_ = this.commandFetcher.TryFetch(payload, out var command);
if (command == null)
{
SqlClientInstrumentationEventSource.Log.NullPayload(nameof(SqlClientDiagnosticListener), name);
activity.Stop();
return;
}

if (activity.IsAllDataRequested)
{
try
Expand All @@ -91,34 +94,8 @@ public override void OnEventWritten(string name, object? payload)
return;
}

_ = this.connectionFetcher.TryFetch(command, out var connection);
_ = this.databaseFetcher.TryFetch(connection, out var database);

// TODO: Need to understand what scenario (if any) database will be null here
// so that we set DisplayName and db.name/db.namespace correctly.
if (database != null)
{
activity.DisplayName = (string)database;

if (this.options.EmitOldAttributes)
{
activity.SetTag(SemanticConventions.AttributeDbName, database);
}

if (this.options.EmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeDbNamespace, database);
}
}

_ = this.dataSourceFetcher.TryFetch(connection, out var dataSource);
_ = this.commandTextFetcher.TryFetch(command, out var commandText);

if (dataSource != null)
{
SqlActivitySourceHelper.AddConnectionLevelDetailsToActivity((string)dataSource, activity, this.options);
}

if (this.commandTypeFetcher.TryFetch(command, out CommandType commandType))
{
switch (commandType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,36 +112,23 @@ private void OnBeginExecute(EventWrittenEventArgs eventData)
return;
}

string dataSource = (string)eventData.Payload[1];
string databaseName = (string)eventData.Payload[2];
var startTags = SqlActivitySourceHelper.GetTagListFromConnectionInfo(dataSource, databaseName, this.options, out var activityName);
var activity = SqlActivitySourceHelper.ActivitySource.StartActivity(
SqlActivitySourceHelper.ActivityName,
activityName,
ActivityKind.Client,
default(ActivityContext),
SqlActivitySourceHelper.CreationTags);
startTags);

if (activity == null)
{
// There is no listener or it decided not to sample the current request.
return;
}

string? databaseName = (string)eventData.Payload[2];

activity.DisplayName = databaseName;

if (activity.IsAllDataRequested)
{
if (this.options.EmitOldAttributes)
{
activity.SetTag(SemanticConventions.AttributeDbName, databaseName);
}

if (this.options.EmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeDbNamespace, databaseName);
}

SqlActivitySourceHelper.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity, this.options);

string commandText = (string)eventData.Payload[3];
if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatementForText)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Collections;

namespace OpenTelemetry.Instrumentation.SqlClient.Tests;

public class SqlClientTestCase : IEnumerable<object[]>
{
public string ConnectionString { get; set; } = string.Empty;

public string ExpectedActivityName { get; set; } = string.Empty;

public string? ExpectedServerAddress { get; set; }

public int? ExpectedPort { get; set; }

public string? ExpectedDbNamespace { get; set; }

public string? ExpectedInstanceName { get; set; }

private static SqlClientTestCase[] TestCases =>
[
new SqlClientTestCase
{
ConnectionString = @"Data Source=SomeServer",
ExpectedActivityName = "SomeServer",
ExpectedServerAddress = "SomeServer",
ExpectedPort = null,
ExpectedDbNamespace = null,
ExpectedInstanceName = null,
},
new SqlClientTestCase
{
ConnectionString = @"Data Source=SomeServer,1434",
ExpectedActivityName = "SomeServer:1434",
ExpectedServerAddress = "SomeServer",
ExpectedPort = 1434,
ExpectedDbNamespace = null,
ExpectedInstanceName = null,
},
];

public IEnumerator<object[]> GetEnumerator()
{
foreach (var testCase in TestCases)
{
yield return new object[] { testCase };
}
}

IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();
}
Loading

0 comments on commit 72d1030

Please sign in to comment.