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

Add pool.latency_us metric and correct unit for pool.latency_ms #10335

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

ian28223
Copy link
Contributor

@ian28223 ian28223 commented Oct 5, 2021

What does this PR do?

  • Correct unit for pool.latency_ms to report in milliseconds as per documentation.
  • Introduce pool.latency_us which is actually in microsecond

Latency_ms is a ProxySQL typo which reported values in microseconds instead of milliseconds. This has then been renamed to Latency_us since ProxySQL 1.3.3

Motivation

AGENT-6978

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@codecov
Copy link

codecov bot commented Oct 5, 2021

Codecov Report

Merging #10335 (ad7c61f) into master (feff48d) will not change coverage.
The diff coverage is n/a.

Flag Coverage Δ
proxysql 98.97% <ø> (ø)

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

@ian28223 ian28223 force-pushed the ian.bucad/proxysql_latency_microsecond branch from d84304a to 535e8cc Compare October 5, 2021 14:03
@ian28223 ian28223 force-pushed the ian.bucad/proxysql_latency_microsecond branch from 535e8cc to ad7c61f Compare October 6, 2021 03:31
Copy link
Contributor

@coignetp coignetp left a comment

Choose a reason for hiding this comment

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

Why introducing a new latency_us ? It's duplicated from latency_ms

@ian28223
Copy link
Contributor Author

Why introducing a new latency_us ? It's duplicated from latency_ms

Reason is that Latency_us is the actual metric since v1.3.3 (2017 release)

And I didn't want to remove latency_ms for backwards compatibility (in case some user already have this in their monitors) so I had it actually report in milliseconds as the name (ms) suggests.

@yzhan289 yzhan289 changed the title correct unit for pool.latency_ms Add pool.latency_us and correct unit for pool.latency_ms Oct 28, 2021
@yzhan289 yzhan289 merged commit 3d059a4 into master Oct 28, 2021
@yzhan289 yzhan289 deleted the ian.bucad/proxysql_latency_microsecond branch October 28, 2021 13:50
@sarah-witt sarah-witt changed the title Add pool.latency_us and correct unit for pool.latency_ms Add pool.latency_us metric and correct unit for pool.latency_ms Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants