Skip to content

Add replicaset name to metrics #70

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ CHANGES:
* Bump go-tarantool from v2.3.0 to v2.3.1.
* Get rid of nameToReplicasetMutex, use atomic instead.
* Add configurable pause before retrying r.Route in Router.Call method.
* Metrics provider now requires replicaset name.

FEATURES:
* Add replicaset name support for prometheus provider.

## v2.0.5

Expand Down
6 changes: 3 additions & 3 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (r *Router) Call(ctx context.Context, bucketID uint64, mode CallMode,

for {
if spent := time.Since(requestStartTime); spent > timeout {
r.metrics().RequestDuration(spent, fnc, false, false)
r.metrics().RequestDuration(spent, fnc, "", false, false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We discussed privately that every request to any replicaset should generate a metric. So, I suggest these fixes to the current PR: #72


r.log().Debugf(ctx, "Return result on timeout; spent %s of timeout %s", spent, timeout)
if err == nil {
Expand Down Expand Up @@ -407,7 +407,7 @@ func (r *Router) Call(ctx context.Context, bucketID uint64, mode CallMode,
}
}

r.metrics().RequestDuration(time.Since(requestStartTime), fnc, true, false)
r.metrics().RequestDuration(time.Since(requestStartTime), fnc, rs.info.Name, true, false)

return storageCallResponse.CallResp, nil
}
Expand Down Expand Up @@ -681,7 +681,7 @@ func RouterMapCallRW[T any](r *Router, ctx context.Context,
nameToResult[rsFuture.name] = storageMapResponse.value
}

r.metrics().RequestDuration(time.Since(timeStart), fnc, true, true)
r.metrics().RequestDuration(time.Since(timeStart), fnc, "all", true, true)

return nameToResult, nil
}
Expand Down
8 changes: 4 additions & 4 deletions providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,16 @@ func (s StdoutLoggerf) Errorf(_ context.Context, format string, v ...any) {
type MetricsProvider interface {
CronDiscoveryEvent(ok bool, duration time.Duration, reason string)
RetryOnCall(reason string)
RequestDuration(duration time.Duration, procedure string, ok, mapReduce bool)
RequestDuration(duration time.Duration, procedure, rsName string, ok, mapReduce bool)
}

// EmptyMetrics is default empty metrics provider
// you can embed this type and realize just some metrics
type EmptyMetrics struct{}

func (e *EmptyMetrics) CronDiscoveryEvent(_ bool, _ time.Duration, _ string) {}
func (e *EmptyMetrics) RetryOnCall(_ string) {}
func (e *EmptyMetrics) RequestDuration(_ time.Duration, _ string, _, _ bool) {}
func (e *EmptyMetrics) CronDiscoveryEvent(_ bool, _ time.Duration, _ string) {}
func (e *EmptyMetrics) RetryOnCall(_ string) {}
func (e *EmptyMetrics) RequestDuration(_ time.Duration, _, _ string, _, _ bool) {}

// TopologyProvider is external module that can lookup current topology of cluster
// it might be etcd/config/consul or smth else
Expand Down
5 changes: 3 additions & 2 deletions providers/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,12 @@ func (pp *Provider) RetryOnCall(reason string) {
}

// RequestDuration records the duration of a request with labels for success and map-reduce usage.
func (pp *Provider) RequestDuration(duration time.Duration, procedure string, ok, mapReduce bool) {
func (pp *Provider) RequestDuration(duration time.Duration, procedure, rsName string, ok, mapReduce bool) {
pp.requestDuration.With(prometheus.Labels{
"ok": strconv.FormatBool(ok),
"map_reduce": strconv.FormatBool(mapReduce),
"procedure": procedure,
"replicaset": rsName,
}).Observe(float64(duration.Milliseconds()))
}

Expand Down Expand Up @@ -98,6 +99,6 @@ func NewPrometheusProvider() *Provider {
requestDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{
Name: "request_duration",
Namespace: "vshard",
}, []string{"procedure", "ok", "map_reduce"}), // Histogram for request durations
}, []string{"procedure", "ok", "map_reduce", "replicaset"}), // Histogram for request durations
}
}
2 changes: 1 addition & 1 deletion providers/prometheus/prometheus_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ExampleNewPrometheusProvider() {

provider.CronDiscoveryEvent(true, 150*time.Millisecond, "success")
provider.RetryOnCall("timeout")
provider.RequestDuration(200*time.Millisecond, "test", true, false)
provider.RequestDuration(200*time.Millisecond, "test", "test-rs", true, false)

resp, err := http.Get(server.URL + "/metrics")
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion providers/prometheus/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestPrometheusMetricsServer(t *testing.T) {

provider.CronDiscoveryEvent(true, 150*time.Millisecond, "success")
provider.RetryOnCall("timeout")
provider.RequestDuration(200*time.Millisecond, "test", true, false)
provider.RequestDuration(200*time.Millisecond, "test", "test-rs", true, false)

resp, err := http.Get(server.URL + "/metrics")
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestEmptyMetrics_RetryOnCall(t *testing.T) {

func TestEmptyMetrics_RequestDuration(t *testing.T) {
require.NotPanics(t, func() {
emptyMetrics.RequestDuration(time.Second, "test", false, false)
emptyMetrics.RequestDuration(time.Second, "test", "test-rs", false, false)
})
}

Expand Down
Loading