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.

Sign up for GitHub

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
merged 12 commits into from
Jul 7, 2020
Merged

Added instrumentation for netfx SqlClient. #761

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
06978ba
Added instrumentation for netfx SqlClient.
CodeBlanch Jul 2, 2020
40dd3b1
Code review.
CodeBlanch Jul 2, 2020
fb25ff8
Send "db.statement_type" instead of "db.statementType".
CodeBlanch Jul 2, 2020
401f740
Added integration tests for SqlEventSource.
CodeBlanch Jul 3, 2020
dade046
Merge branch 'master' into netfx-sqlclient-instrumentation
CodeBlanch Jul 3, 2020
baa5f1b
Adding .github folder to solution.
CodeBlanch Jul 3, 2020
14c399d
Added a couple missing files into the solution.
CodeBlanch Jul 3, 2020
c3a499c
Added FakeSqlEventSource tests.
CodeBlanch Jul 3, 2020
83200ea
Merging latest from master.
CodeBlanch Jul 7, 2020
7e62d1b
Fixed up GrpcClientDiagnosticListener for EventSource changes.
CodeBlanch Jul 7, 2020
b9908b8
Removed extra ActivityListener from unit tests.
CodeBlanch Jul 7, 2020
e9ac05e
Merge branch 'master' into netfx-sqlclient-instrumentation
cijothomas Jul 7, 2020
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
36 changes: 36 additions & 0 deletions OpenTelemetry.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,38 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "OpenTelemetry.Instrumentati
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests", "test\OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests\OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests.csproj", "{7C4026CA-6434-4762-8B77-D657EAEE1325}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".github", ".github", "{F1D0972B-38CF-49C2-9F4B-4C5DE02FB71D}"
ProjectSection(SolutionItems) = preProject
.github\CODEOWNERS = .github\CODEOWNERS
.github\PULL_REQUEST_TEMPLATE.md = .github\PULL_REQUEST_TEMPLATE.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ISSUE_TEMPLATE", "ISSUE_TEMPLATE", "{A533C800-3DC3-4D04-90A7-0CE7A1E6BDB3}"
ProjectSection(SolutionItems) = preProject
.github\ISSUE_TEMPLATE\bug_report.md = .github\ISSUE_TEMPLATE\bug_report.md
.github\ISSUE_TEMPLATE\feature_request.md = .github\ISSUE_TEMPLATE\feature_request.md
.github\ISSUE_TEMPLATE\question.md = .github\ISSUE_TEMPLATE\question.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "workflows", "workflows", "{E69578EB-B456-4062-A645-877CD964528B}"
ProjectSection(SolutionItems) = preProject
.github\workflows\dotnet-core-linux.yml = .github\workflows\dotnet-core-linux.yml
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
.github\workflows\dotnet-core-win.yml = .github\workflows\dotnet-core-win.yml
.github\workflows\dotnet-core.yml = .github\workflows\dotnet-core.yml
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{C1542297-8763-4DF4-957C-489ED771C21D}"
ProjectSection(SolutionItems) = preProject
src\Directory.Build.props = src\Directory.Build.props
src\Directory.Build.targets = src\Directory.Build.targets
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{D2E73927-5966-445C-94E9-EFE6F269C8D5}"
ProjectSection(SolutionItems) = preProject
test\Directory.Build.props = test\Directory.Build.props
test\Directory.Build.targets = test\Directory.Build.targets
EndProjectSection
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -238,6 +270,10 @@ Global
{47318988-CA8B-4C81-B55D-2FA11D295A49} = {E359BB2B-9AEC-497D-B321-7DF2450C3B8E}
{25C06046-C7D0-46B4-AAAC-90C50C43DE7A} = {E359BB2B-9AEC-497D-B321-7DF2450C3B8E}
{9A4E3A68-904B-4835-A3C8-F664B73098DB} = {E359BB2B-9AEC-497D-B321-7DF2450C3B8E}
{A533C800-3DC3-4D04-90A7-0CE7A1E6BDB3} = {F1D0972B-38CF-49C2-9F4B-4C5DE02FB71D}
{E69578EB-B456-4062-A645-877CD964528B} = {F1D0972B-38CF-49C2-9F4B-4C5DE02FB71D}
{C1542297-8763-4DF4-957C-489ED771C21D} = {7CB2F02E-03FA-4FFF-89A5-C51F107623FD}
{D2E73927-5966-445C-94E9-EFE6F269C8D5} = {7CB2F02E-03FA-4FFF-89A5-C51F107623FD}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {55639B5C-0770-4A22-AB56-859604650521}
Expand Down
5 changes: 3 additions & 2 deletions src/OpenTelemetry.Api/Trace/SpanAttributeConstants.cs
View file
Open in desktop
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.

can you add this if feasible?
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md#connection-level-attributes-for-specific-technologies

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
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs
View file
Open in desktop
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
8 changes: 2 additions & 6 deletions src/OpenTelemetry.Instrumentation.AspNet.Win/Implementation/HttpInListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ public HttpInListener(string name, AspNetInstrumentationOptions options, Activit

public override void OnStartActivity(Activity activity, object payload)
{
const string EventNameSuffix = ".OnStartActivity";

var context = HttpContext.Current;
if (context == null)
{
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener) + EventNameSuffix);
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity));
return;
}

Expand Down Expand Up @@ -114,8 +112,6 @@ public override void OnStartActivity(Activity activity, object payload)

public override void OnStopActivity(Activity activity, object payload)
{
const string EventNameSuffix = ".OnStopActivity";

Activity activityToEnrich = activity;

if (!(this.options.TextFormat is TraceContextFormatActivity))
Expand All @@ -139,7 +135,7 @@ public override void OnStopActivity(Activity activity, object payload)
var context = HttpContext.Current;
if (context == null)
{
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener) + EventNameSuffix);
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStopActivity));
return;
}

Expand Down
7 changes: 2 additions & 5 deletions src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,11 @@ public HttpInListener(string name, AspNetCoreInstrumentationOptions options, Act

public override void OnStartActivity(Activity activity, object payload)
{
const string EventNameSuffix = ".OnStartActivity";
var context = this.startContextFetcher.Fetch(payload) as HttpContext;

if (context == null)
{
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener) + EventNameSuffix);
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStartActivity));
return;
}

Expand Down Expand Up @@ -120,13 +119,11 @@ public override void OnStartActivity(Activity activity, object payload)

public override void OnStopActivity(Activity activity, object payload)
{
const string EventNameSuffix = ".OnStopActivity";

if (activity.IsAllDataRequested)
{
if (!(this.stopContextFetcher.Fetch(payload) is HttpContext context))
{
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener) + EventNameSuffix);
InstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnStopActivity));
return;
}

Expand Down
7 changes: 2 additions & 5 deletions ...penTelemetry.Instrumentation.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,9 @@ public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options, A

public override void OnStartActivity(Activity activity, object payload)
{
const string EventNameSuffix = ".OnStartActivity";
if (!(this.startRequestFetcher.Fetch(payload) is HttpRequestMessage request))
{
InstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener) + EventNameSuffix);
InstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener), nameof(this.OnStartActivity));
return;
}

Expand Down Expand Up @@ -141,13 +140,11 @@ public override void OnStopActivity(Activity activity, object payload)

public override void OnException(Activity activity, object payload)
{
const string EventNameSuffix = ".OnException";

if (activity.IsAllDataRequested)
{
if (!(this.stopExceptionFetcher.Fetch(payload) is Exception exc))
{
InstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener) + EventNameSuffix);
InstrumentationEventSource.Log.NullPayload(nameof(HttpHandlerDiagnosticListener), nameof(this.OnException));
return;
}

Expand Down
19 changes: 11 additions & 8 deletions src/OpenTelemetry.Instrumentation.Dependencies/Implementation/SqlClientDiagnosticListener.cs
View file
Open in desktop
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 @@ -66,7 +65,7 @@ public override void OnCustom(string name, Activity activity, object payload)

if (command == null)
{
InstrumentationEventSource.Log.NullPayload($"{nameof(SqlClientDiagnosticListener)}-{name}");
InstrumentationEventSource.Log.NullPayload(nameof(SqlClientDiagnosticListener), name);
return;
}

Expand All @@ -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 All @@ -136,7 +139,7 @@ public override void OnCustom(string name, Activity activity, object payload)
}
else
{
InstrumentationEventSource.Log.NullPayload($"{nameof(SqlClientDiagnosticListener)}-{name}");
InstrumentationEventSource.Log.NullPayload(nameof(SqlClientDiagnosticListener), name);
}
}

Expand Down
Loading