Skip to content

[chore][receiver/sqlserver] update error handling and time unit and refactory #39042

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

Merged
merged 15 commits into from
Apr 1, 2025

Conversation

cuichenli
Copy link
Contributor

@cuichenli cuichenli commented Mar 29, 2025

Description

  • update time unit of lock_timeout, this one was not updated in the previous pr
  • update error handling in sample query, the previous version ignored all the errors
  • update top query logic on put attributes to similar to sample query

Link to tracking issue

Fixes

Testing

Documentation

@cuichenli cuichenli requested review from crobert-1 and a team as code owners March 29, 2025 03:24
@github-actions github-actions bot added receiver/sqlserver Run Windows Enable running windows test on a PR labels Mar 29, 2025
@cuichenli cuichenli marked this pull request as draft March 29, 2025 03:27
@cuichenli cuichenli marked this pull request as ready for review March 29, 2025 03:52
Copy link
Member

@XSAM XSAM left a comment

Choose a reason for hiding this comment

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

I think PR still needs further polish.

@cuichenli
Copy link
Contributor Author

updated @XSAM

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

Since the unit of a value is being changed, this would need a changelog if it doesn't get merged in time for v0.123.0, which is due to be created soon.

@crobert-1
Copy link
Member

Since the unit of a value is being changed, this would need a changelog if it doesn't get merged in time for v0.123.0, which is due to be created soon.

v0.123.0 has been released, so this needs a changelog for the unit change, right?

@cuichenli
Copy link
Contributor Author

a change log was added

@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label Apr 1, 2025
@songy23 songy23 merged commit b462a9e into open-telemetry:main Apr 1, 2025
241 of 244 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 1, 2025
dmathieu pushed a commit to dmathieu/opentelemetry-collector-contrib that referenced this pull request Apr 8, 2025
…efactory (open-telemetry#39042)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
- update time unit of `lock_timeout`, this one was not updated in the
previous pr
- update error handling in sample query, the previous version ignored
all the errors
- update top query logic on put attributes to similar to sample query 

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->
Fiery-Fenix pushed a commit to Fiery-Fenix/opentelemetry-collector-contrib that referenced this pull request Apr 24, 2025
…efactory (open-telemetry#39042)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
- update time unit of `lock_timeout`, this one was not updated in the
previous pr
- update error handling in sample query, the previous version ignored
all the errors
- update top query logic on put attributes to similar to sample query 

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers receiver/sqlserver Run Windows Enable running windows test on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants