Skip to content

Commit

Permalink
Add metrics to track account cache hits and misses (#1018)
Browse files Browse the repository at this point in the history
* Add metrics to track settings cache population if it is enabled

* Add metrics to track account cache hits and misses
  • Loading branch information
schernysh authored and nickluck8 committed Aug 10, 2021
1 parent 888bd28 commit 73b305f
Show file tree
Hide file tree
Showing 10 changed files with 372 additions and 99 deletions.
3 changes: 3 additions & 0 deletions docs/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ where `[DATASOURCE]` is a data source name, `DEFAULT_DS` by defaul.
- `timeout_notification.ok` - number of times bidders were successfully notified about timeouts
- `timeout_notification.failed` - number of unsuccessful attempts to notify bidders about timeouts
- `currency-rates.stale` - a flag indicating if currency rates obtained from external source are fresh (`0`) or stale (`1`)
- `settings.cache.(stored-request|amp-stored-request).refresh.(initialize|update).db_query_time` - timer tracking how long was settings cache population
- `settings.cache.(stored-request|amp-stored-request).refresh.(initialize|update).err` - number of errors during settings cache population
- `settings.cache.account.(hit|miss)` - number of times account was found or was missing in cache

## Auction per-adapter metrics
- `adapter.<bidder-name>.no_cookie_requests` - number of requests made to `<bidder-name>` that did not contain UID
Expand Down
11 changes: 10 additions & 1 deletion src/main/java/org/prebid/server/metric/MetricName.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,16 @@ public enum MetricName {
rejected,

//currency rates
stale;
stale,

// settings cache
stored_request("stored-request"),
amp_stored_request("amp-stored-request"),
account,
initialize,
update,
hit,
miss;

private final String name;

Expand Down
20 changes: 20 additions & 0 deletions src/main/java/org/prebid/server/metric/Metrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class Metrics extends UpdatableMetrics {
private final Function<String, AdapterMetrics> adapterMetricsCreator;
private final Function<Integer, BidderCardinalityMetrics> bidderCardinalityMetricsCreator;
private final Function<MetricName, CircuitBreakerMetrics> circuitBreakerMetricsCreator;
private final Function<MetricName, SettingsCacheMetrics> settingsCacheMetricsCreator;
// not thread-safe maps are intentionally used here because it's harmless in this particular case - eventually
// this all boils down to metrics lookup by underlying metric registry and that operation is guaranteed to be
// thread-safe
Expand All @@ -46,6 +47,7 @@ public class Metrics extends UpdatableMetrics {
private final CacheMetrics cacheMetrics;
private final TimeoutNotificationMetrics timeoutNotificationMetrics;
private final CurrencyRatesMetrics currencyRatesMetrics;
private final Map<MetricName, SettingsCacheMetrics> settingsCacheMetrics;

public Metrics(MetricRegistry metricRegistry, CounterType counterType, AccountMetricsVerbosity
accountMetricsVerbosity, BidderCatalog bidderCatalog) {
Expand All @@ -60,6 +62,7 @@ public Metrics(MetricRegistry metricRegistry, CounterType counterType, AccountMe
bidderCardinalityMetricsCreator = cardinality -> new BidderCardinalityMetrics(
metricRegistry, counterType, cardinality);
circuitBreakerMetricsCreator = type -> new CircuitBreakerMetrics(metricRegistry, counterType, type);
settingsCacheMetricsCreator = type -> new SettingsCacheMetrics(metricRegistry, counterType, type);
requestMetrics = new EnumMap<>(MetricName.class);
accountMetrics = new HashMap<>();
adapterMetrics = new HashMap<>();
Expand All @@ -71,6 +74,7 @@ public Metrics(MetricRegistry metricRegistry, CounterType counterType, AccountMe
cacheMetrics = new CacheMetrics(metricRegistry, counterType);
timeoutNotificationMetrics = new TimeoutNotificationMetrics(metricRegistry, counterType);
currencyRatesMetrics = new CurrencyRatesMetrics(metricRegistry, counterType);
settingsCacheMetrics = new HashMap<>();
}

RequestStatusMetrics forRequestType(MetricName requestType) {
Expand Down Expand Up @@ -113,6 +117,10 @@ CurrencyRatesMetrics currencyRates() {
return currencyRatesMetrics;
}

SettingsCacheMetrics forSettingsCacheType(MetricName type) {
return settingsCacheMetrics.computeIfAbsent(type, settingsCacheMetricsCreator);
}

public void updateSafariRequestsMetric(boolean isSafari) {
if (isSafari) {
incCounter(MetricName.safari_requests);
Expand Down Expand Up @@ -464,6 +472,18 @@ public void createCurrencyRatesGauge(BooleanSupplier stateSupplier) {
currencyRates().createGauge(MetricName.stale, () -> stateSupplier.getAsBoolean() ? 1 : 0);
}

public void updateSettingsCacheRefreshTime(MetricName cacheType, MetricName refreshType, long timeElapsed) {
forSettingsCacheType(cacheType).forRefreshType(refreshType).updateTimer(MetricName.db_query_time, timeElapsed);
}

public void updateSettingsCacheRefreshErrorMetric(MetricName cacheType, MetricName refreshType) {
forSettingsCacheType(cacheType).forRefreshType(refreshType).incCounter(MetricName.err);
}

public void updateSettingsCacheEventMetric(MetricName cacheType, MetricName event) {
forSettingsCacheType(cacheType).incCounter(event);
}

private String resolveMetricsBidderName(String bidder) {
return bidderCatalog.isValidName(bidder) ? bidder : METRICS_UNKNOWN_BIDDER;
}
Expand Down
58 changes: 58 additions & 0 deletions src/main/java/org/prebid/server/metric/SettingsCacheMetrics.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package org.prebid.server.metric;

import com.codahale.metrics.MetricRegistry;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;

/**
* Settings cache metrics support.
*/
class SettingsCacheMetrics extends UpdatableMetrics {

private final Function<MetricName, RefreshSettingsCacheMetrics> refreshSettingsCacheMetricsCreator;
private final Map<MetricName, RefreshSettingsCacheMetrics> refreshSettingsCacheMetrics;

SettingsCacheMetrics(MetricRegistry metricRegistry, CounterType counterType, MetricName type) {
super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
nameCreator(createPrefix(Objects.requireNonNull(type))));

refreshSettingsCacheMetricsCreator = refreshType ->
new RefreshSettingsCacheMetrics(metricRegistry, counterType, createPrefix(type), refreshType);
refreshSettingsCacheMetrics = new HashMap<>();
}

RefreshSettingsCacheMetrics forRefreshType(MetricName refreshType) {
return refreshSettingsCacheMetrics.computeIfAbsent(refreshType, refreshSettingsCacheMetricsCreator);
}

private static String createPrefix(MetricName type) {
return String.format("settings.cache.%s", type.toString());
}

private static Function<MetricName, String> nameCreator(String prefix) {
return metricName -> String.format("%s.%s", prefix, metricName.toString());
}

static class RefreshSettingsCacheMetrics extends UpdatableMetrics {

RefreshSettingsCacheMetrics(MetricRegistry metricRegistry,
CounterType counterType,
String prefix,
MetricName type) {

super(Objects.requireNonNull(metricRegistry), Objects.requireNonNull(counterType),
nameCreator(createPrefix(Objects.requireNonNull(prefix), Objects.requireNonNull(type))));
}

private static String createPrefix(String prefix, MetricName type) {
return String.format("%s.refresh.%s", prefix, type.toString());
}

private static Function<MetricName, String> nameCreator(String prefix) {
return metricName -> String.format("%s.%s", prefix, metricName.toString());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.exception.PreBidException;
import org.prebid.server.execution.Timeout;
import org.prebid.server.metric.MetricName;
import org.prebid.server.metric.Metrics;
import org.prebid.server.settings.helper.StoredDataFetcher;
import org.prebid.server.settings.helper.StoredItemResolver;
import org.prebid.server.settings.model.Account;
Expand All @@ -20,6 +22,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Consumer;

/**
* Adds caching functionality for {@link ApplicationSettings} implementation.
Expand All @@ -36,9 +39,16 @@ public class CachingApplicationSettings implements ApplicationSettings {
private final SettingsCache cache;
private final SettingsCache ampCache;
private final SettingsCache videoCache;
private final Metrics metrics;

public CachingApplicationSettings(ApplicationSettings delegate,
SettingsCache cache,
SettingsCache ampCache,
SettingsCache videoCache,
Metrics metrics,
int ttl,
int size) {

public CachingApplicationSettings(ApplicationSettings delegate, SettingsCache cache, SettingsCache ampCache,
SettingsCache videoCache, int ttl, int size) {
if (ttl <= 0 || size <= 0) {
throw new IllegalArgumentException("ttl and size must be positive");
}
Expand All @@ -49,46 +59,67 @@ public CachingApplicationSettings(ApplicationSettings delegate, SettingsCache ca
this.cache = Objects.requireNonNull(cache);
this.ampCache = Objects.requireNonNull(ampCache);
this.videoCache = Objects.requireNonNull(videoCache);
this.metrics = Objects.requireNonNull(metrics);
}

/**
* Retrieves account from cache or delegates it to original fetcher.
*/
@Override
public Future<Account> getAccountById(String accountId, Timeout timeout) {
return getFromCacheOrDelegate(accountCache, accountToErrorCache, accountId, timeout, delegate::getAccountById);
return getFromCacheOrDelegate(
accountCache,
accountToErrorCache,
accountId,
timeout,
delegate::getAccountById,
event -> metrics.updateSettingsCacheEventMetric(MetricName.account, event));
}

/**
* Retrieves adUnit config from cache or delegates it to original fetcher.
*/
@Override
public Future<String> getAdUnitConfigById(String adUnitConfigId, Timeout timeout) {
return getFromCacheOrDelegate(adUnitConfigCache, accountToErrorCache, adUnitConfigId, timeout,
delegate::getAdUnitConfigById);
return getFromCacheOrDelegate(
adUnitConfigCache,
accountToErrorCache,
adUnitConfigId,
timeout,
delegate::getAdUnitConfigById,
CachingApplicationSettings::noOp);
}

/**
* Retrieves stored data from cache or delegates it to original fetcher.
*/
@Override
public Future<StoredDataResult> getStoredData(String accountId, Set<String> requestIds, Set<String> impIds,
public Future<StoredDataResult> getStoredData(String accountId,
Set<String> requestIds,
Set<String> impIds,
Timeout timeout) {

return getFromCacheOrDelegate(cache, accountId, requestIds, impIds, timeout, delegate::getStoredData);
}

/**
* Retrieves amp stored data from cache or delegates it to original fetcher.
*/
@Override
public Future<StoredDataResult> getAmpStoredData(String accountId, Set<String> requestIds, Set<String> impIds,
public Future<StoredDataResult> getAmpStoredData(String accountId,
Set<String> requestIds,
Set<String> impIds,
Timeout timeout) {

return getFromCacheOrDelegate(ampCache, accountId, requestIds, impIds, timeout, delegate::getAmpStoredData);
}

@Override
public Future<StoredDataResult> getVideoStoredData(String accountId, Set<String> requestIds, Set<String> impIds,
public Future<StoredDataResult> getVideoStoredData(String accountId,
Set<String> requestIds,
Set<String> impIds,
Timeout timeout) {

return getFromCacheOrDelegate(videoCache, accountId, requestIds, impIds, timeout, delegate::getVideoStoredData);
}

Expand All @@ -100,15 +131,22 @@ public Future<StoredResponseDataResult> getStoredResponses(Set<String> responseI
return delegate.getStoredResponses(responseIds, timeout);
}

private static <T> Future<T> getFromCacheOrDelegate(Map<String, T> cache, Map<String, String> accountToErrorCache,
String key, Timeout timeout,
BiFunction<String, Timeout, Future<T>> retriever) {
private static <T> Future<T> getFromCacheOrDelegate(Map<String, T> cache,
Map<String, String> accountToErrorCache,
String key,
Timeout timeout,
BiFunction<String, Timeout, Future<T>> retriever,
Consumer<MetricName> metricUpdater) {

final T cachedValue = cache.get(key);
if (cachedValue != null) {
metricUpdater.accept(MetricName.hit);

return Future.succeededFuture(cachedValue);
}

metricUpdater.accept(MetricName.miss);

final String preBidExceptionMessage = accountToErrorCache.get(key);
if (preBidExceptionMessage != null) {
return Future.failedFuture(new PreBidException(preBidExceptionMessage));
Expand All @@ -129,7 +167,11 @@ private static <T> Future<T> getFromCacheOrDelegate(Map<String, T> cache, Map<St
* with all found stored items and error from origin source id call was made.
*/
private static Future<StoredDataResult> getFromCacheOrDelegate(
SettingsCache cache, String accountId, Set<String> requestIds, Set<String> impIds, Timeout timeout,
SettingsCache cache,
String accountId,
Set<String> requestIds,
Set<String> impIds,
Timeout timeout,
StoredDataFetcher<String, Set<String>, Set<String>, Timeout, Future<StoredDataResult>> retriever) {

// empty string account ID doesn't make sense
Expand Down Expand Up @@ -170,19 +212,24 @@ private static Future<StoredDataResult> getFromCacheOrDelegate(
});
}

private static <T> Future<T> cacheAndReturnFailedFuture(Throwable throwable, String key,
private static <T> Future<T> cacheAndReturnFailedFuture(Throwable throwable,
String key,
Map<String, String> cache) {

if (throwable instanceof PreBidException) {
cache.put(key, throwable.getMessage());
}

return Future.failedFuture(throwable);
}

private static Map<String, String> getFromCacheOrAddMissedIds(String accountId,
Set<String> ids,
Map<String, Set<StoredItem>> cache,
Set<String> missedIds) {

final Map<String, String> idToStoredItem = new HashMap<>(ids.size());

for (String id : ids) {
try {
final StoredItem resolvedStoredItem = StoredItemResolver.resolve(null, accountId, id, cache.get(id));
Expand All @@ -191,11 +238,15 @@ private static Map<String, String> getFromCacheOrAddMissedIds(String accountId,
missedIds.add(id);
}
}

return idToStoredItem;
}

public void invalidateAccountCache(String accountId) {
accountCache.remove(accountId);
logger.debug("Account with id {0} was invalidated", accountId);
}

private static <ANY> void noOp(ANY any) {
}
}
Loading

0 comments on commit 73b305f

Please sign in to comment.