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

Added instrumentation for netfx SqlClient. #761

Merged
5 changes: 3 additions & 2 deletions src/OpenTelemetry.Api/Trace/SpanAttributeConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ public static class SpanAttributeConstants
public const string HttpRouteKey = "http.route";
public const string HttpFlavorKey = "http.flavor";

public const string DatabaseTypeKey = "db.type";
public const string DatabaseInstanceKey = "db.instance";
public const string DatabaseSystemKey = "db.system";
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Created #782 for this.

public const string DatabaseNameKey = "db.name";
public const string DatabaseStatementKey = "db.statement";
public const string DatabaseStatementTypeKey = "db.statement_type";
Copy link
Member

Choose a reason for hiding this comment

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

Is this part of spec?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I now recollect we already discussed this. Will need to follow up on spec. Note to myself!

#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
}
}
22 changes: 11 additions & 11 deletions src/OpenTelemetry.Api/Trace/SpanExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -207,34 +207,34 @@ public static TelemetrySpan PutHttpFlavorAttribute(this TelemetrySpan span, stri
}

/// <summary>
/// Helper method that populates database type
/// to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-database.md.
/// Helper method that populates database system
/// to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md.
/// </summary>
/// <param name="span">Span to fill out.</param>
/// <param name="type">Database type.</param>
/// <param name="system">Database system.</param>
/// <returns>Span with populated properties.</returns>
public static TelemetrySpan PutDatabaseTypeAttribute(this TelemetrySpan span, string type)
public static TelemetrySpan PutDatabaseSystemAttribute(this TelemetrySpan span, string system)
{
span.SetAttribute(SpanAttributeConstants.DatabaseTypeKey, type);
span.SetAttribute(SpanAttributeConstants.DatabaseSystemKey, system);
return span;
}

/// <summary>
/// Helper method that populates database instance
/// to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-database.md.
/// Helper method that populates database name
/// to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md.
/// </summary>
/// <param name="span">Span to fill out.</param>
/// <param name="instance">Database instance.</param>
/// <param name="name">Database name.</param>
/// <returns>Span with populated properties.</returns>
public static TelemetrySpan PutDatabaseInstanceAttribute(this TelemetrySpan span, string instance)
public static TelemetrySpan PutDatabaseNameAttribute(this TelemetrySpan span, string name)
{
span.SetAttribute(SpanAttributeConstants.DatabaseInstanceKey, instance);
span.SetAttribute(SpanAttributeConstants.DatabaseNameKey, name);
return span;
}

/// <summary>
/// Helper method that populates database statement
/// to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-database.md.
/// to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md.
/// </summary>
/// <param name="span">Span to fill out.</param>
/// <param name="statement">Database statement.</param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public static JaegerSpan ToJaegerSpan(this Activity activity)
PooledList<JaegerTag>.Add(ref jaegerTags.Tags, new JaegerTag("library.name", JaegerTagType.STRING, vStr: activitySource.Name));
if (!string.IsNullOrEmpty(activitySource.Version))
{
PooledList<JaegerTag>.Add(ref jaegerTags.Tags, new JaegerTag("library.version", JaegerTagType.STRING, vStr: activitySource.Name));
PooledList<JaegerTag>.Add(ref jaegerTags.Tags, new JaegerTag("library.version", JaegerTagType.STRING, vStr: activitySource.Version));
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
using System.Data;
using System.Diagnostics;
using OpenTelemetry.Trace;
using OpenTelemetry.Trace.Samplers;

namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
Expand All @@ -32,7 +31,7 @@ internal class SqlClientDiagnosticListener : ListenerHandler
internal const string SqlDataWriteCommandError = "System.Data.SqlClient.WriteCommandError";
internal const string SqlMicrosoftWriteCommandError = "Microsoft.Data.SqlClient.WriteCommandError";

private const string DatabaseStatementTypeSpanAttributeKey = "db.statementType";
internal const string MicrosoftSqlServerDatabaseSystemName = "mssql";

private readonly PropertyFetcher commandFetcher = new PropertyFetcher("Command");
private readonly PropertyFetcher connectionFetcher = new PropertyFetcher("Connection");
Expand Down Expand Up @@ -85,17 +84,16 @@ public override void OnCustom(string name, Activity activity, object payload)
var commandText = this.commandTextFetcher.Fetch(command);

activity.AddTag(SpanAttributeConstants.ComponentKey, "sql");
activity.AddTag(SpanAttributeConstants.DatabaseTypeKey, "sql");
activity.AddTag(SpanAttributeConstants.DatabaseSystemKey, MicrosoftSqlServerDatabaseSystemName);
activity.AddTag(SpanAttributeConstants.PeerServiceKey, (string)dataSource);
activity.AddTag(SpanAttributeConstants.DatabaseInstanceKey, (string)database);
activity.AddTag(SpanAttributeConstants.DatabaseNameKey, (string)database);

if (this.commandTypeFetcher.Fetch(command) is CommandType commandType)
{
activity.AddTag(DatabaseStatementTypeSpanAttributeKey, commandType.ToString());
Copy link
Member Author

Choose a reason for hiding this comment

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

This change was made to avoid the .ToString perf hit. Unrelated to the main changes, sorry.

Copy link
Member

Choose a reason for hiding this comment

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

:D I read this comment late, and was still trying to figure out this part !


switch (commandType)
{
case CommandType.StoredProcedure:
activity.AddTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.StoredProcedure));
if (this.options.CaptureStoredProcedureCommandName)
{
activity.AddTag(SpanAttributeConstants.DatabaseStatementKey, (string)commandText);
Expand All @@ -104,12 +102,17 @@ public override void OnCustom(string name, Activity activity, object payload)
break;

case CommandType.Text:
activity.AddTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.Text));
if (this.options.CaptureTextCommandContent)
{
activity.AddTag(SpanAttributeConstants.DatabaseStatementKey, (string)commandText);
}

break;

case CommandType.TableDirect:
activity.AddTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.TableDirect));
break;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
// <copyright file="SqlEventSourceListener.netfx.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>
#if NETFRAMEWORK
using System;
using System.Data;
using System.Diagnostics;
using System.Diagnostics.Tracing;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
/// <summary>
/// .NET Framework SqlClient doesn't emit DiagnosticSource events.
/// We hook into its EventSource if it is available:
/// See: <a href="https://github.com/microsoft/referencesource/blob/3b1eaf5203992df69de44c783a3eda37d3d4cd10/System.Data/System/Data/Common/SqlEventSource.cs#L29">reference source</a>.
/// </summary>
internal class SqlEventSourceListener : EventListener
{
internal const string ActivitySourceName = "System.Data.SqlClient";
internal const string ActivityName = ActivitySourceName + ".Execute";

private const string AdoNetEventSourceName = "Microsoft-AdoNet-SystemData";
private const int BeginExecuteEventId = 1;
private const int EndExecuteEventId = 2;

private static readonly Version Version = typeof(SqlEventSourceListener).Assembly.GetName().Version;
private static readonly ActivitySource SqlClientActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());

private readonly SqlClientInstrumentationOptions options;
private EventSource eventSource;

public SqlEventSourceListener(SqlClientInstrumentationOptions options = null)
{
this.options = options ?? new SqlClientInstrumentationOptions();
}

public override void Dispose()
{
if (this.eventSource != null)
{
this.DisableEvents(this.eventSource);
}

base.Dispose();
}

protected override void OnEventSourceCreated(EventSource eventSource)
{
if (eventSource?.Name == AdoNetEventSourceName)
{
this.eventSource = eventSource;
this.EnableEvents(eventSource, EventLevel.Informational, (EventKeywords)1);
}

base.OnEventSourceCreated(eventSource);
}

protected override void OnEventWritten(EventWrittenEventArgs eventData)
{
if (eventData?.Payload == null)
{
InstrumentationEventSource.Log.NullPayload(nameof(SqlEventSourceListener) + nameof(this.OnEventWritten));
return;
}

try
{
if (eventData.EventId == BeginExecuteEventId)
{
this.OnBeginExecute(eventData);
}
else if (eventData.EventId == EndExecuteEventId)
{
this.OnEndExecute(eventData);
}
}
catch (Exception exc)
{
InstrumentationEventSource.Log.UnknownErrorProcessingEvent(nameof(SqlEventSourceListener), nameof(this.OnEventWritten), exc);
}
}

private void OnBeginExecute(EventWrittenEventArgs eventData)
{
/*
Expected payload:
[0] -> ObjectId
[1] -> DataSource
[2] -> Database
[3] -> CommandText ([3] = CommandType == CommandType.StoredProcedure ? CommandText : string.Empty)
*/

if (eventData.Payload.Count < 4)
{
return;
}

var activity = SqlClientActivitySource.StartActivity(ActivityName, ActivityKind.Client);
if (activity == null)
{
return;
}

string databaseName = (string)eventData.Payload[2];
Copy link
Member

Choose a reason for hiding this comment

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

wondering why setting displayname is not inside if (activity.IsAllDataRequested) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I modeled after the HttpWebRequestActivitySource:

private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, Activity activity)
{
activity.DisplayName = HttpTagHelper.GetOperationNameForHttpMethod(request.Method);
InstrumentRequest(request, activity);
activity.SetCustomProperty("HttpWebRequest.Request", request);
if (activity.IsAllDataRequested)
{
activity.AddTag(SpanAttributeConstants.ComponentKey, "http");
activity.AddTag(SpanAttributeConstants.HttpMethodKey, request.Method);
activity.AddTag(SpanAttributeConstants.HttpHostKey, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
activity.AddTag(SpanAttributeConstants.HttpUrlKey, request.RequestUri.OriginalString);
activity.AddTag(SpanAttributeConstants.HttpFlavorKey, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
}
}

Line 96 in that snippet.

I did it there originally because the DisplayName felt useful to have always? Open to change it.

Copy link
Member

Choose a reason for hiding this comment

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

Got. I couldn't find any guidelines on this, so at this points, its up to individual library authors to do it. (in case adapters "we" are the authors).


activity.DisplayName = databaseName;

if (activity.IsAllDataRequested)
{
activity.AddTag(SpanAttributeConstants.ComponentKey, "sql");
Copy link
Member

Choose a reason for hiding this comment

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

note to myself.
component is removed from spec and we need to remove it from this repo.


activity.AddTag(SpanAttributeConstants.DatabaseSystemKey, SqlClientDiagnosticListener.MicrosoftSqlServerDatabaseSystemName);
activity.AddTag(SpanAttributeConstants.PeerServiceKey, (string)eventData.Payload[1]);
activity.AddTag(SpanAttributeConstants.DatabaseNameKey, databaseName);

string commandText = (string)eventData.Payload[3];
if (string.IsNullOrEmpty(commandText))
{
activity.AddTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.Text));
}
else
{
activity.AddTag(SpanAttributeConstants.DatabaseStatementTypeKey, nameof(CommandType.StoredProcedure));
if (this.options.CaptureStoredProcedureCommandName)
{
activity.AddTag(SpanAttributeConstants.DatabaseStatementKey, commandText);
}
}
}
}

private void OnEndExecute(EventWrittenEventArgs eventData)
{
/*
Expected payload:
[0] -> ObjectId
[1] -> CompositeState bitmask (0b001 -> successFlag, 0b010 -> isSqlExceptionFlag , 0b100 -> synchronousFlag)
[2] -> SqlExceptionNumber
*/

if (eventData.Payload.Count < 3)
{
return;
}

var activity = Activity.Current;
if (activity?.Source != SqlClientActivitySource)
{
return;
}

try
{
if (activity.IsAllDataRequested)
{
int compositeState = (int)eventData.Payload[1];
if ((compositeState & 0b001) == 0b001)
{
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(StatusCanonicalCode.Ok));
}
else
{
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(StatusCanonicalCode.Unknown));
if ((compositeState & 0b010) == 0b010)
{
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, $"SqlExceptionNumber {eventData.Payload[2]} thrown.");
}
else
{
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, $"Unknown Sql failure.");
}
}
}
}
finally
{
activity.Stop();
}
}
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ public static OpenTelemetryBuilder AddSqlClientDependencyInstrumentation(
configureSqlClientInstrumentationOptions?.Invoke(sqlOptions);

builder.AddInstrumentation((activitySource) => new SqlClientInstrumentation(activitySource, sqlOptions));
#if NETFRAMEWORK
builder.AddActivitySource(SqlEventSourceListener.ActivitySourceName);
#endif

return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public class SqlClientInstrumentation : IDisposable
internal const string SqlClientDiagnosticListenerName = "SqlClientDiagnosticListener";

private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;
#if NETFRAMEWORK
private readonly SqlEventSourceListener sqlEventSourceListener;
#endif

/// <summary>
/// Initializes a new instance of the <see cref="SqlClientInstrumentation"/> class.
Expand All @@ -49,12 +52,19 @@ public SqlClientInstrumentation(ActivitySourceAdapter activitySource, SqlClientI
listener => listener.Name == SqlClientDiagnosticListenerName,
null);
this.diagnosticSourceSubscriber.Subscribe();

#if NETFRAMEWORK
this.sqlEventSourceListener = new SqlEventSourceListener(options);
#endif
}

/// <inheritdoc/>
public void Dispose()
{
this.diagnosticSourceSubscriber?.Dispose();
#if NETFRAMEWORK
this.sqlEventSourceListener?.Dispose();
#endif
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ public void SqlClientCallsAreCollectedSuccessfully(
Assert.Null(span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.StatusDescriptionKey).Value);

Assert.Equal("sql", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.ComponentKey).Value);
Assert.Equal("sql", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseTypeKey).Value);
Assert.Equal("master", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseInstanceKey).Value);
Assert.Equal(SqlClientDiagnosticListener.MicrosoftSqlServerDatabaseSystemName, span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseSystemKey).Value);
Assert.Equal("master", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseNameKey).Value);

switch (commandType)
{
Expand Down Expand Up @@ -205,8 +205,8 @@ public void SqlClientErrorsAreCollectedSuccessfully(string beforeCommand, string
Assert.Equal("Unknown", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.StatusCodeKey).Value);
Assert.Equal("Boom!", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.StatusDescriptionKey).Value);
Assert.Equal("sql", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.ComponentKey).Value);
Assert.Equal("sql", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseTypeKey).Value);
Assert.Equal("master", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseInstanceKey).Value);
Assert.Equal(SqlClientDiagnosticListener.MicrosoftSqlServerDatabaseSystemName, span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseSystemKey).Value);
Assert.Equal("master", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseNameKey).Value);
Assert.Equal("SP_GetOrders", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.DatabaseStatementKey).Value);
Assert.Equal("(localdb)\\MSSQLLocalDB", span.Tags.FirstOrDefault(i => i.Key == SpanAttributeConstants.PeerServiceKey).Value);
}
Expand Down