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

[sqlclient] Enable collecting server.* attributes by default #2249

Merged
merged 4 commits into from
Oct 25, 2024

Conversation

alanwest
Copy link
Member

Fixes #2235

Confirmed with @lmolkova and @trask in the DB conventions SIG that the server.address/server.port attributes need to be collected by default.

@alanwest alanwest requested a review from a team as a code owner October 25, 2024 19:54
@github-actions github-actions bot added the comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient label Oct 25, 2024
Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.36%. Comparing base (71655ce) to head (40305e9).
Report is 549 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2249       +/-   ##
===========================================
+ Coverage   73.91%   91.36%   +17.44%     
===========================================
  Files         267        8      -259     
  Lines        9615      301     -9314     
===========================================
- Hits         7107      275     -6832     
+ Misses       2508       26     -2482     
Flag Coverage Δ
unittests-Instrumentation.SqlClient 91.36% <100.00%> (?)

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

Files with missing lines Coverage Δ
....SqlClient/SqlClientTraceInstrumentationOptions.cs 100.00% <100.00%> (ø)

... and 271 files with indirect coverage changes

Copy link
Member Author

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

I guess I was a bit surprised there were no tests I needed to update. I do have some work in flight for refactoring and expanding the test coverage, so I don't want to address that in this PR.

@CodeBlanch CodeBlanch changed the title Enable collecting server.* attributes by default [sqlclient] Enable collecting server.* attributes by default Oct 25, 2024
Copy link
Member

@CodeBlanch CodeBlanch left a comment

Choose a reason for hiding this comment

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

LGTM

@alanwest alanwest merged commit cf14a5a into open-telemetry:main Oct 25, 2024
56 checks passed
@alanwest alanwest deleted the alanwest/server-attributes branch October 25, 2024 22:32
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] Consider capturing server.address and server.port by default
3 participants