Added Sql connection-level attributes (from spec) as an opt-in feature. #782
Conversation
Codecov Report
@@ Coverage Diff @@
## master #782 +/- ##
==========================================
+ Coverage 71.09% 71.16% +0.07%
==========================================
Files 223 223
Lines 7009 7061 +52
Branches 1180 1191 +11
==========================================
+ Hits 4983 5025 +42
- Misses 1710 1722 +12
+ Partials 316 314 -2
Continue to review full report at Codecov.
|
| { | ||
| [SpanAttributeConstants.PeerServiceKey] = 0, // peer.service primary. | ||
| ["net.peer.name"] = 1, // peer.service first alternative. | ||
| [SpanAttributeConstants.NetPeerName] = 1, // peer.service first alternative. |
There was a problem hiding this comment.
one first and three second alternatives =)
There was a problem hiding this comment.
Hah ok yes, the comments suck. It's more like tiers. I was trying to have the official named attributes called out in the spec be taken first, in case multiple matches are found on a span. Spec PR (open-telemetry/opentelemetry-specification#534) never gained much traction.
| } | ||
| internal const string MicrosoftSqlServerDatabaseInstanceName = "db.mssql.instance_name"; | ||
|
|
||
| private static readonly Regex DataSourceRegex = new Regex("^(.*?)(?:[\\\\,]|$)\\s*(.*?)(?:,|$)\\s*(.*)$", RegexOptions.Compiled); |
There was a problem hiding this comment.
it would be handy to have an example of a string in a comment
| /// </remarks> | ||
| public bool EnableConnectionLevelAttributes { get; set; } = false; | ||
|
|
||
| internal static SqlConnectionDetails ParseDataSource(string dataSource) |
There was a problem hiding this comment.
minor: I'd prefer to move this logic to common helper class and keep options class simple.
3 participants
Changes
You can now set
options.EnableConnectionLevelAttributes = truewhen configuring Sql Instrumentation to send connection-level attributes. I went with the option because it does come with a slight perf hit, looking up cached values and then setting tags.Checklist
For significant contributions please make sure you have completed the following items: