-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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/redis] Command latency metrics. #26569
[receiver/redis] Command latency metrics. #26569
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
redis.cmd.latency: | ||
enabled: false | ||
description: Command execution latency | ||
unit: us |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed this during the review. The OTel spec recommends sticking to basic units, e.g. seconds instead of milliseconds if possible https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#instrument-units. E.g. HTTP latency metrics is in seconds as well https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpserverrequestduration
I think we should convert the values to seconds in a follow-up PR
**Description:** Added command latency metric to the redis receiver. **Link to tracking Issue:** open-telemetry#6942 **Testing:** Added tests for parsing `latencystats` section of redis info. **Documentation:** Metric documentation in metadata.yaml
Description: Added command latency metric to the redis receiver.
Link to tracking Issue: #6942
Testing: Added tests for parsing
latencystats
section of redis info.Documentation: Metric documentation in metadata.yaml