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

Redesign command latency metrics publishing #1409

Closed
mp911de opened this issue Sep 8, 2020 · 0 comments
Closed

Redesign command latency metrics publishing #1409

mp911de opened this issue Sep 8, 2020 · 0 comments
Labels
type: breaking Breaking change type: task A general task
Milestone

Comments

@mp911de
Copy link
Collaborator

mp911de commented Sep 8, 2020

Right now, Lettuce defines a MetricCollector and a MetricEventPublisher which assumes a certain distribution and collection model for command latency metrics. When considering a Micrometer integration (#795), this model isn't applicable as Micrometer is the facility that encapsulates both aspects.

We should change CommandLatencyCollector to no longer assume metric collection on the interface level (MetricCollector.retrieveMetrics()) but make metric event publication an implementation detail of DefaultCommandLatencyCollector. EventPublisherOptions commandLatencyPublisherOptions (from ClientResources) would become also a part of DefaultCommandLatencyCollector.

@mp911de mp911de added type: task A general task target: Lettuce 6 type: breaking Breaking change labels Sep 8, 2020
@mp911de mp911de added this to the 6.0.0 milestone Sep 8, 2020
mp911de added a commit that referenced this issue Sep 10, 2020
Command latency metric collection uses now CommandLatencyRecorder which is a simplified variant of CommandLatencyCollector exposing isEnabled() and recordCommandLatency(…) methods. Latency metrics are now less tied to how latencies are published/surfaced. The publishing mechanism checks whether the configured CommandLatencyRecorder collects and exposes metrics so the publisher can obtain the collected metrics. Using a latency recorder that don't expose metrics through CommandLatencyRecorder will leave latency metrics event publication disabled.

This is a breaking changes as several methods were renamed.
@mp911de mp911de closed this as completed Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking Breaking change type: task A general task
Projects
None yet
Development

No branches or pull requests

1 participant