Skip to content

[SqlClient] Add tags required for sampling before activity start#3675

Merged
alanwest merged 2 commits intoopen-telemetry:mainfrom
alanwest:alanwest/sql-client-start-tags
Jan 9, 2026
Merged

[SqlClient] Add tags required for sampling before activity start#3675
alanwest merged 2 commits intoopen-telemetry:mainfrom
alanwest:alanwest/sql-client-start-tags

Conversation

@alanwest
Copy link
Copy Markdown
Member

@alanwest alanwest commented Jan 8, 2026

Fixes #2224

@martincostello Oops I think there was a miscommunication. #2224 has not been resolved yet. This PR conforms to the spec by making sure all the attributes that are required for sampling are present when the activity is started.

I did not touch the EFCore instrumentation in this PR. I ended up duplicating some code in DatabaseSemanticConventionHelper that could probably be refactored a bit once this is ported to EFCore.

@alanwest alanwest requested a review from a team as a code owner January 8, 2026 23:24
@github-actions github-actions bot added the comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient label Jan 8, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 8, 2026

Codecov Report

❌ Patch coverage is 51.42857% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.45%. Comparing base (5771ebc) to head (d307a63).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Shared/DatabaseSemanticConventionHelper.cs 0.00% 17 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3675      +/-   ##
==========================================
- Coverage   71.68%   71.45%   -0.23%     
==========================================
  Files         455      455              
  Lines       17640    17654      +14     
==========================================
- Hits        12645    12615      -30     
- Misses       4995     5039      +44     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 87.19% <0.00%> (-1.87%) ⬇️
unittests-Exporter.Geneva 53.76% <ø> (-0.33%) ⬇️
unittests-Exporter.InfluxDB 95.14% <ø> (ø)
unittests-Exporter.Instana 74.86% <ø> (ø)
unittests-Exporter.OneCollector 94.62% <ø> (ø)
unittests-Extensions 90.65% <ø> (ø)
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 83.42% <ø> (ø)
unittests-Instrumentation.AspNet 78.14% <ø> (ø)
unittests-Instrumentation.AspNetCore 71.69% <ø> (ø)
unittests-Instrumentation.Cassandra 23.52% <ø> (ø)
unittests-Instrumentation.ConfluentKafka 14.10% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.12% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 80.80% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.42% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 79.61% <ø> (ø)
unittests-Instrumentation.Hangfire 86.05% <ø> (ø)
unittests-Instrumentation.Http 74.22% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 34.54% <ø> (ø)
unittests-Instrumentation.SqlClient 87.03% <100.00%> (-0.10%) ⬇️
unittests-Instrumentation.StackExchangeRedis 71.80% <ø> (ø)
unittests-Instrumentation.Wcf 79.44% <ø> (-0.14%) ⬇️
unittests-OpAmp.Client 80.16% <ø> (-0.57%) ⬇️
unittests-PersistentStorage 74.91% <ø> (-1.68%) ⬇️
unittests-Resources.AWS 74.42% <ø> (ø)
unittests-Resources.Azure 85.31% <ø> (ø)
unittests-Resources.Container 67.34% <ø> (ø)
unittests-Resources.Gcp 71.42% <ø> (ø)
unittests-Resources.Host 71.85% <ø> (ø)
unittests-Resources.OperatingSystem 76.98% <ø> (ø)
unittests-Resources.Process 100.00% <ø> (ø)
unittests-Resources.ProcessRuntime 79.59% <ø> (ø)
unittests-Sampler.AWS 94.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ient/Implementation/SqlClientDiagnosticListener.cs 88.07% <100.00%> (-0.16%) ⬇️
...ent/Implementation/SqlEventSourceListener.netfx.cs 79.61% <100.00%> (-0.20%) ⬇️
src/Shared/DatabaseSemanticConventionHelper.cs 25.86% <0.00%> (-10.73%) ⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

&& (string)kvp.Value == testCase.Expected.DbCollectionName);
}

if (!string.IsNullOrEmpty(testCase.Expected.DbStoredProcedureName))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Technically, the spec does not require db.stored_procedure.name be available prior to sampling, but it was simpler to just include it with the rest of the attributes.

@alanwest alanwest changed the title [SqlClient] Add tags required for sampling before activty start [SqlClient] Add tags required for sampling before activity start Jan 8, 2026
@martincostello
Copy link
Copy Markdown
Member

I did not touch the EFCore instrumentation in this PR. I ended up duplicating some code in DatabaseSemanticConventionHelper that could probably be refactored a bit once this is ported to EFCore.

I'll look to do a clean-up post-merge.

@alanwest alanwest added this pull request to the merge queue Jan 9, 2026
Merged via the queue into open-telemetry:main with commit 224c66b Jan 9, 2026
320 of 321 checks passed
@alanwest alanwest deleted the alanwest/sql-client-start-tags branch January 9, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[sql] Database attributes required at span creation time

3 participants