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

[receiver/sqlserver] Enable more perf counter metrics #33420

Merged
merged 4 commits into from
Jun 19, 2024

Conversation

crobert-1
Copy link
Member

Description:

These metrics already exist, and are all enabled by default, but were previously not being recorded when directly connecting to the SQL Server instance. This means these metrics will now be enabled by default across all systems and configurations.

Metrics:

sqlserver.batch.request.rate
sqlserver.batch.sql_compilation.rate
sqlserver.batch.sql_recompilation.rate
sqlserver.page.buffer_cache.hit_ratio
sqlserver.user.connection.count

Testing:
Updated tests to account for given metrics

Documentation
Updated documentation to show these metrics are now available on more systems than just Windows.

@crobert-1 crobert-1 requested a review from djaglowski as a code owner June 6, 2024 23:11
@crobert-1 crobert-1 requested a review from a team June 6, 2024 23:11
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement
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'm on the fence as to whether this should be considered an enhancement or breaking change. These metrics are all shown as being enabled by default, but the reality is they aren't received unless running on Windows.

This change does increase the number of metrics being received when directly connecting to the SQL server instance in reality, so let me know if we should mark this as breaking instead.

Copy link
Member

Choose a reason for hiding this comment

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

Seems reasonable to call it an enhancement

@crobert-1 crobert-1 added the Run Windows Enable running windows test on a PR label Jun 6, 2024
@crobert-1 crobert-1 force-pushed the sqlserver_pc_metrics branch from c25f8e7 to a35c7a1 Compare June 7, 2024 14:45
@crobert-1
Copy link
Member Author

Both CI/CD failures are unrelated to this change. I've added frequencies on relevant issues.

# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement
Copy link
Member

Choose a reason for hiding this comment

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

Seems reasonable to call it an enhancement

@djaglowski djaglowski merged commit 9bffe94 into open-telemetry:main Jun 19, 2024
178 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/sqlserver Run Windows Enable running windows test on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants