Skip to content

Add OpenTelemetry Native Metrics Support#3637

Merged
ndyakov merged 37 commits into
masterfrom
add-metrics
Feb 13, 2026
Merged

Add OpenTelemetry Native Metrics Support#3637
ndyakov merged 37 commits into
masterfrom
add-metrics

Conversation

@ofekshenawa

@ofekshenawa ofekshenawa commented Dec 3, 2025

Copy link
Copy Markdown
Collaborator

This PR implements Observability (Metrics) for go-redis, adding comprehensive OpenTelemetry metrics support following the OpenTelemetry Database Client Semantic Conventions.

The implementation uses a Bridge Pattern to keep the core library dependency-free while providing optional, metrics instrumentation through the new extra/redisotel-native package.

What's Included:
Metric groups covering aspects of Redis operations:

  • Command metrics: Operation duration with retry tracking
  • Connection basic: Connection count and creation time
  • Resiliency: Errors, handoffs, timeout relaxation
  • Connection advanced: Wait time and use time
  • Pubsub metrics: Published and received messages
  • Stream metrics: Processing duration and maintenance notifications

@ofekshenawa ofekshenawa marked this pull request as ready for review December 3, 2025 15:06

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

submitting partial review

Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/otel/metrics.go Outdated
Comment thread maintnotifications/push_notification_handler.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread extra/redisotel-native/config.go
Comment thread extra/redisotel-native/config.go
Comment thread extra/redisotel-native/redisotel.go Outdated
Comment thread otel.go Outdated
Comment thread maintnotifications/push_notification_handler.go

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

left two comments that can be considered for all get/set operations. other than that the thread safety looks ok if we are willing to accept that there is a time window for which getters may have returned an old value. can we make sure if we remove the telemetry, this won't break the app? In the case when the old value is nil I assume we can accept a single event not being tracked.

Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go Outdated
Comment thread internal/pool/pool.go
Comment thread extra/redisotel-native/config.go
vladvildanov
vladvildanov previously approved these changes Jan 23, 2026
Comment thread extra/redisotel-native/redisotel.go Outdated
ndyakov
ndyakov previously approved these changes Feb 4, 2026

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall looks good, can merge as is, left couple of improvements that we can address

Comment thread extra/redisotel-native/metrics.go
Comment thread extra/redisotel-native/metrics.go Outdated
Comment thread extra/redisotel-native/metrics.go Outdated
Comment thread extra/redisotel-native/redisotel.go Outdated
Comment thread extra/redisotel-native/redisotel.go Outdated
Comment thread maintnotifications/handoff_worker.go Outdated
Comment thread maintnotifications/handoff_worker.go Outdated
Comment thread maintnotifications/push_notification_handler.go Outdated
Comment thread maintnotifications/push_notification_handler.go Outdated
Comment thread options.go Outdated
@ndyakov

ndyakov commented Feb 13, 2026

Copy link
Copy Markdown
Member

@ofekshenawa let's resolve the comments here and work on merging this

@ndyakov ndyakov self-requested a review February 13, 2026 15:36

@ndyakov ndyakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, we can resolve:
#3637 (comment)
in a separate PR in the future.

@ndyakov ndyakov merged commit ee6e9db into master Feb 13, 2026
41 checks passed
@ndyakov ndyakov deleted the add-metrics branch February 13, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants