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

Add metrics to track account cache hits and misses #1018

Merged
merged 3 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions docs/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ where `[DATASOURCE]` is a data source name, `DEFAULT_DS` by defaul.
- `circuit-breaker.geo.opened` - state of the geo location circuit breaker: `1` means opened (geo location resource is unavailable), `0` - closed
- `timeout_notification.ok` - number of times bidders were successfully notified about timeouts
- `timeout_notification.failed` - number of unsuccessful attempts to notify bidders about timeouts
- `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 @@ -101,7 +101,16 @@ public enum MetricName {
creative_size,

//account.*.requests.
rejected;
rejected,

// 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 @@ -45,6 +46,7 @@ public class Metrics extends UpdatableMetrics {
private final Map<MetricName, CircuitBreakerMetrics> circuitBreakerMetrics;
private final CacheMetrics cacheMetrics;
private final TimeoutNotificationMetrics timeoutNotificationMetrics;
private final Map<MetricName, SettingsCacheMetrics> settingsCacheMetrics;

public Metrics(MetricRegistry metricRegistry, CounterType counterType, AccountMetricsVerbosity
accountMetricsVerbosity, BidderCatalog bidderCatalog) {
Expand All @@ -59,6 +61,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 @@ -69,6 +72,7 @@ public Metrics(MetricRegistry metricRegistry, CounterType counterType, AccountMe
circuitBreakerMetrics = new HashMap<>();
cacheMetrics = new CacheMetrics(metricRegistry, counterType);
timeoutNotificationMetrics = new TimeoutNotificationMetrics(metricRegistry, counterType);
settingsCacheMetrics = new HashMap<>();
}

RequestStatusMetrics forRequestType(MetricName requestType) {
Expand Down Expand Up @@ -107,6 +111,10 @@ CacheMetrics cache() {
return cacheMetrics;
}

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

public void updateSafariRequestsMetric(boolean isSafari) {
if (isSafari) {
incCounter(MetricName.safari_requests);
Expand Down Expand Up @@ -454,6 +462,18 @@ public void updateTimeoutNotificationMetric(boolean success) {
}
}

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