Skip to content

Commit

Permalink
Fix entry count metric for lookup caches (#4558)
Browse files Browse the repository at this point in the history
* Fix entry count metric for lookup caches

The way the `LookupCache#entryCount()` method was defined, caused implementers to write incorrect implementations.

For example `GuavaLookupCache#entryCount()` returned a constant `Gauge<Long>` on the first call (in the constructor
of `LookupCache`) which was never updated.

Additionally, the `CacheTableEntry` tried to access the entry count metric through an invalid object path which
resulted in the metric always being displayed as "NaN" in the web interface.

Fixes #4540

* Make entry count Gauge compatible with Graylog 2.4.x

Refs #4499

Refs #4541
(cherry picked from commit 0ddbeac)
  • Loading branch information
joschi authored and bernd committed Feb 2, 2018
1 parent 0455335 commit 1f4bcbd
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/
package org.graylog2.lookup.caches;

import com.codahale.metrics.Gauge;
import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.Timer;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
Expand Down Expand Up @@ -82,11 +81,11 @@ public GuavaLookupCache(@Assisted("id") String id,
}

@Override
public Gauge<Long> entryCount() {
public long entryCount() {
if (cache != null) {
return cache::size;
return cache.size();
} else {
return () -> 0L;
return 0L;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,13 @@ protected LookupCache(String id,
this.hitCount = metricRegistry.meter(MetricRegistry.name("org.graylog2.lookup.caches", id, "hits"));
this.missCount = metricRegistry.meter(MetricRegistry.name("org.graylog2.lookup.caches", id, "misses"));
this.lookupTimer = metricRegistry.timer(MetricRegistry.name("org.graylog2.lookup.caches", id, "lookupTime"));
MetricUtils.safelyRegister(metricRegistry, MetricRegistry.name("org.graylog2.lookup.caches", id, "entries"), entryCount());
final Gauge<Long> entriesGauge = new Gauge<Long>() {
@Override
public Long getValue() {
return entryCount();
}
};
MetricUtils.safelyRegister(metricRegistry, MetricRegistry.name("org.graylog2.lookup.caches", id, "entries"), entriesGauge);
}

public void incrTotalCount() {
Expand All @@ -79,9 +85,13 @@ public Timer.Context lookupTimer() {
return lookupTimer.time();
}

public Gauge<Long> entryCount() {
// Returns -1 if the cache does not support counting entries
return () -> -1L;
/**
* Get the number of elements in this lookup cache.
*
* @return the number of elements in this lookup cache or {@code -1L} if the cache does not support counting entries
*/
public long entryCount() {
return -1L;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const LUTTableEntry = React.createClass({
_onEntriesMetrics(metrics) {
let total = 0;

Object.keys(metrics).map(nodeId => metrics[nodeId].count.metric.value.value).forEach((v) => { total += v; });
Object.keys(metrics).map(nodeId => metrics[nodeId].count.metric.value).forEach((v) => { total += v; });

if (total < 0) {
return 'n/a';
Expand Down

0 comments on commit 1f4bcbd

Please sign in to comment.