Skip to content

[redisreceiver] Fix metric types and units according to Redis semantics #40852

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

Closed
wants to merge 3 commits into from

Conversation

shmsr
Copy link

@shmsr shmsr commented Jun 21, 2025

Description

  • Changed metric types from Sum to Gauge for point-in-time measurements:
    • redis.clients.blocked - current count of clients pending on blocking calls
    • redis.clients.connected - current count of client connections
    • redis.rdb.changes_since_last_save - current count of changes since last dump
    • redis.slaves.connected - current count of connected replicas
    • redis.role - current role indicator
  • Fixed units for replication offset metrics from "By" (bytes) to "1" (dimensionless):
    • redis.replication.backlog_first_byte_offset
    • redis.replication.offset
    • redis.replication.replica_offset
  • Added legacy Redis role terminology to role attribute enum: master, slave, sentinel

Link to tracking issue

Fixes #40814

Testing

Documentation

@shmsr shmsr requested review from dmitryax and a team as code owners June 21, 2025 11:31
@github-actions github-actions bot added the receiver/redis Redis related issues label Jun 21, 2025
Comment on lines +53 to +55
- master
- slave
- sentinel
Copy link
Author

Choose a reason for hiding this comment

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

I added these based on the documentation: https://redis.io/docs/latest/commands/role/

Also, I took a INFO dump by connecting redis-cli to redis-server, I got this output:

# Replication
role:master

And master was not there in the enum. Also, added it to the last to maintain the enum order.

@github-actions github-actions bot requested a review from hughesjj June 24, 2025 17:24
@shmsr
Copy link
Author

shmsr commented Jun 27, 2025

@dmitryax / @hughesjj Did you get a chance to look at this?

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@atoulme
Copy link
Contributor

atoulme commented Jul 16, 2025

@shmsr I am pinging the codeowners on slack - please review the conflict.

@dmitryax
Copy link
Member

I don't see why the metrics have to be gauges. They are additive meaning if they are re-agregated, the values should be summed up. They seem to me proper UpDownCounters. See https://github.com/open-telemetry/opentelemetry-specification/blob/8aa1d27d9cf8c224299dcf0b58bfd5fcaacf88eb/specification/metrics/supplementary-guidelines.md#guidelines-for-instrumentation-library-authors for more details.

See #23454 for the second item. Looks like you're reverting it. We should avoid 1 units.

Please submit a separate PRs for each item where still applicable

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 31, 2025
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Aug 15, 2025
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.

[redisreceiver] Fix incorrect metric types and units in Redis receiver metadata
3 participants