From 363a50f4a73a26eff6909e93cfab215985b00479 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Thu, 13 Feb 2025 21:13:53 -0500 Subject: [PATCH 1/7] Support adding a trace information to cache UUID on demand --- .../prebid/server/cache/CoreCacheService.java | 66 ++++++- .../spring/config/ServiceConfiguration.java | 4 + .../server/cache/CoreCacheServiceTest.java | 174 +++++++++++++++++- 3 files changed, 240 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/prebid/server/cache/CoreCacheService.java b/src/main/java/org/prebid/server/cache/CoreCacheService.java index e86ac4f5d9b..5647a45fd0f 100644 --- a/src/main/java/org/prebid/server/cache/CoreCacheService.java +++ b/src/main/java/org/prebid/server/cache/CoreCacheService.java @@ -63,6 +63,8 @@ public class CoreCacheService { private static final Logger logger = LoggerFactory.getLogger(CoreCacheService.class); private static final String BID_WURL_ATTRIBUTE = "wurl"; + private static final String TRACE_INFO_SEPARATOR = "-"; + private static final int MAX_DATACENTER_REGION_LENGTH = 4; private final HttpClient httpClient; private final URL endpointUrl; @@ -78,6 +80,9 @@ public class CoreCacheService { private final MultiMap cacheHeaders; private final Map> debugHeaders; + private final boolean appendTraceInfoToCacheId; + private final String datacenterRegion; + public CoreCacheService( HttpClient httpClient, URL endpointUrl, @@ -85,6 +90,8 @@ public CoreCacheService( long expectedCacheTimeMs, String apiKey, boolean isApiKeySecured, + boolean appendTraceInfoToCacheId, + String datacenterRegion, VastModifier vastModifier, EventsService eventsService, Metrics metrics, @@ -107,6 +114,9 @@ public CoreCacheService( ? HttpUtil.headers().add(HttpUtil.X_PBC_API_KEY_HEADER, Objects.requireNonNull(apiKey)) : HttpUtil.headers(); debugHeaders = HttpUtil.toDebugHeaders(cacheHeaders); + + this.appendTraceInfoToCacheId = appendTraceInfoToCacheId; + this.datacenterRegion = normalizeDatacenterRegion(datacenterRegion); } public String getEndpointHost() { @@ -211,6 +221,7 @@ private List updatePutObjects(List bidPutObjects, .bidid(null) .bidder(null) .timestamp(null) + .key(resolveCacheKey(accountId, putObject.getKey())) .value(vastModifier.modifyVastXml(isEventsEnabled, allowedBidders, putObject, @@ -268,7 +279,8 @@ private Future doCacheOpenrtb(List bids, final List cachedCreatives = Stream.concat( bids.stream().map(cacheBid -> createJsonPutObjectOpenrtb(cacheBid, accountId, eventsContext)), - videoBids.stream().map(videoBid -> createXmlPutObjectOpenrtb(videoBid, requestId, hbCacheId))) + videoBids.stream().map(videoBid -> + createXmlPutObjectOpenrtb(videoBid, requestId, hbCacheId, accountId))) .collect(Collectors.toCollection(ArrayList::new)); if (cachedCreatives.isEmpty()) { @@ -385,9 +397,12 @@ private CachedCreative createJsonPutObjectOpenrtb(CacheBid cacheBid, bidObjectNode.put(BID_WURL_ATTRIBUTE, eventUrl); } + final String resolvedCacheKey = resolveCacheKey(accountId); + final BidPutObject payload = BidPutObject.builder() .aid(eventsContext.getAuctionId()) .type("json") + .key(resolvedCacheKey) .value(bidObjectNode) .ttlseconds(cacheBid.getTtl()) .build(); @@ -395,16 +410,21 @@ private CachedCreative createJsonPutObjectOpenrtb(CacheBid cacheBid, return CachedCreative.of(payload, creativeSizeFromAdm(bid.getAdm())); } - private CachedCreative createXmlPutObjectOpenrtb(CacheBid cacheBid, String requestId, String hbCacheId) { + private CachedCreative createXmlPutObjectOpenrtb(CacheBid cacheBid, + String requestId, + String hbCacheId, + String accountId) { final BidInfo bidInfo = cacheBid.getBidInfo(); final Bid bid = bidInfo.getBid(); final String vastXml = bid.getAdm(); - final String customCacheKey = resolveCustomCacheKey(hbCacheId, bidInfo.getCategory()); + final String resolvedCacheKey = resolveCacheKey(accountId, hbCacheId); + final String customCacheKey = resolveCustomCacheKey(resolvedCacheKey, bidInfo.getCategory()); final BidPutObject payload = BidPutObject.builder() .aid(requestId) .type("xml") + .key(customCacheKey != null ? customCacheKey : resolvedCacheKey) .value(vastXml != null ? new TextNode(vastXml) : null) .ttlseconds(cacheBid.getTtl()) .key(customCacheKey) @@ -553,4 +573,44 @@ private BidCacheRequest toBidCacheRequest(List cachedCreatives) .map(CachedCreative::getPayload) .toList()); } + + private String resolveCacheKey(String accountId) { + return resolveCacheKey(accountId, null); + } + + // hbCacheId is accepted here to have a backwards-compatibility with video category mapping + private String resolveCacheKey(String accountId, String hbCacheId) { + if (!appendTraceInfoToCacheId) { + return hbCacheId; + } + + // no need to have additional separator if datacenter name won't be added + final boolean isDatacenterNamePopulated = StringUtils.isNotBlank(datacenterRegion); + final int separatorCount = isDatacenterNamePopulated ? 2 : 1; + final int accountIdLength = accountId.length(); + final int traceInfoLength = isDatacenterNamePopulated + ? accountIdLength + datacenterRegion.length() + separatorCount + : accountIdLength + separatorCount; + + final String cacheKey = hbCacheId != null ? hbCacheId : idGenerator.generateId(); + if (cacheKey != null && traceInfoLength < cacheKey.length()) { + final String substring = cacheKey.substring(0, cacheKey.length() - traceInfoLength); + return isDatacenterNamePopulated + ? accountId + TRACE_INFO_SEPARATOR + datacenterRegion + TRACE_INFO_SEPARATOR + substring + : accountId + TRACE_INFO_SEPARATOR + substring; + } else { + return hbCacheId; + } + } + + private String normalizeDatacenterRegion(String datacenterRegion) { + if (datacenterRegion == null) { + return null; + } + + final String trimmedDatacenterRegion = datacenterRegion.trim(); + return trimmedDatacenterRegion.length() > MAX_DATACENTER_REGION_LENGTH + ? trimmedDatacenterRegion.substring(0, MAX_DATACENTER_REGION_LENGTH) + : trimmedDatacenterRegion; + } } diff --git a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java index e0b00fb9623..15cf29a19f1 100644 --- a/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java +++ b/src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java @@ -164,6 +164,8 @@ CoreCacheService cacheService( @Value("${auction.cache.expected-request-time-ms}") long expectedCacheTimeMs, @Value("${pbc.api.key:#{null}}") String apiKey, @Value("${cache.api-key-secured:false}") boolean apiKeySecured, + @Value("${cache.append-trace-info-to-cache-id:false}") boolean appendTraceInfoToCacheId, + @Value("${datacenter-region:#{null}}") String datacenterRegion, VastModifier vastModifier, EventsService eventsService, HttpClient httpClient, @@ -178,6 +180,8 @@ CoreCacheService cacheService( expectedCacheTimeMs, apiKey, apiKeySecured, + appendTraceInfoToCacheId, + datacenterRegion, vastModifier, eventsService, metrics, diff --git a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java index 058f958fa48..cf9639027b7 100644 --- a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java +++ b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java @@ -111,6 +111,8 @@ public void setUp() throws MalformedURLException, JsonProcessingException { 100L, null, false, + false, + null, vastModifier, eventsService, metrics, @@ -385,6 +387,8 @@ public void cacheBidsOpenrtbShouldUseApiKeyWhenProvided() throws MalformedURLExc 100L, "ApiKey", true, + false, + null, vastModifier, eventsService, metrics, @@ -810,6 +814,8 @@ public void cachePutObjectsShouldUseApiKeyWhenProvided() throws MalformedURLExce 100L, "ApiKey", true, + false, + null, vastModifier, eventsService, metrics, @@ -827,7 +833,7 @@ public void cachePutObjectsShouldUseApiKeyWhenProvided() throws MalformedURLExce // when target.cachePutObjects( - asList(firstBidPutObject), + singletonList(firstBidPutObject), true, singleton("bidder1"), "account", @@ -839,6 +845,172 @@ public void cachePutObjectsShouldUseApiKeyWhenProvided() throws MalformedURLExce .isEqualTo("ApiKey"); } + @Test + public void cachePutObjectsShouldPrependTraceInfoWhenEnabled() throws IOException { + // given + target = new CoreCacheService( + httpClient, + new URL("http://cache-service/cache"), + "http://cache-service-host/cache?uuid=", + 100L, + "ApiKey", + false, + true, + null, + vastModifier, + eventsService, + metrics, + clock, + idGenerator, + jacksonMapper); + + given(idGenerator.generateId()).willReturn("high-entropy-cache-id"); + + final BidPutObject firstBidPutObject = BidPutObject.builder() + .type("json") + .bidid("bidId1") + .bidder("bidder1") + .timestamp(1L) + .value(new TextNode("vast")) + .build(); + final BidPutObject secondBidPutObject = BidPutObject.builder() + .type("xml") + .bidid("bidId2") + .bidder("bidder2") + .timestamp(1L) + .value(new TextNode("VAST")) + .build(); + final BidPutObject thirdBidPutObject = BidPutObject.builder() + .type("text") + .bidid("bidId3") + .bidder("bidder3") + .timestamp(1L) + .value(new TextNode("VAST")) + .build(); + + given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) + .willReturn(new TextNode("modifiedVast")) + .willReturn(new TextNode("VAST")) + .willReturn(new TextNode("updatedVast")); + + // when + target.cachePutObjects( + asList(firstBidPutObject, secondBidPutObject, thirdBidPutObject), + true, + singleton("bidder1"), + "account", + "pbjs", + timeout); + + // then + final BidPutObject modifiedFirstBidPutObject = firstBidPutObject.toBuilder() + .bidid(null) + .bidder(null) + .timestamp(null) + .key("account-high-entropy-") + .value(new TextNode("modifiedVast")) + .build(); + final BidPutObject modifiedSecondBidPutObject = secondBidPutObject.toBuilder() + .bidid(null) + .bidder(null) + .timestamp(null) + .key("account-high-entropy-") + .build(); + final BidPutObject modifiedThirdBidPutObject = thirdBidPutObject.toBuilder() + .bidid(null) + .bidder(null) + .timestamp(null) + .key("account-high-entropy-") + .value(new TextNode("updatedVast")) + .build(); + + assertThat(captureBidCacheRequest().getPuts()) + .containsExactly(modifiedFirstBidPutObject, modifiedSecondBidPutObject, modifiedThirdBidPutObject); + } + + @Test + public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() throws IOException { + // given + target = new CoreCacheService( + httpClient, + new URL("http://cache-service/cache"), + "http://cache-service-host/cache?uuid=", + 100L, + "ApiKey", + false, + true, + "apac", + vastModifier, + eventsService, + metrics, + clock, + idGenerator, + jacksonMapper); + + given(idGenerator.generateId()).willReturn("high-entropy-cache-id"); + + final BidPutObject firstBidPutObject = BidPutObject.builder() + .type("json") + .bidid("bidId1") + .bidder("bidder1") + .timestamp(1L) + .value(new TextNode("vast")) + .build(); + final BidPutObject secondBidPutObject = BidPutObject.builder() + .type("xml") + .bidid("bidId2") + .bidder("bidder2") + .timestamp(1L) + .value(new TextNode("VAST")) + .build(); + final BidPutObject thirdBidPutObject = BidPutObject.builder() + .type("text") + .bidid("bidId3") + .bidder("bidder3") + .timestamp(1L) + .value(new TextNode("VAST")) + .build(); + + given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) + .willReturn(new TextNode("modifiedVast")) + .willReturn(new TextNode("VAST")) + .willReturn(new TextNode("updatedVast")); + + // when + target.cachePutObjects( + asList(firstBidPutObject, secondBidPutObject, thirdBidPutObject), + true, + singleton("bidder1"), + "account", + "pbjs", + timeout); + + // then + final BidPutObject modifiedFirstBidPutObject = firstBidPutObject.toBuilder() + .bidid(null) + .bidder(null) + .timestamp(null) + .key("account-apac-high-ent") + .value(new TextNode("modifiedVast")) + .build(); + final BidPutObject modifiedSecondBidPutObject = secondBidPutObject.toBuilder() + .bidid(null) + .bidder(null) + .timestamp(null) + .key("account-apac-high-ent") + .build(); + final BidPutObject modifiedThirdBidPutObject = thirdBidPutObject.toBuilder() + .bidid(null) + .bidder(null) + .timestamp(null) + .key("account-apac-high-ent") + .value(new TextNode("updatedVast")) + .build(); + + assertThat(captureBidCacheRequest().getPuts()) + .containsExactly(modifiedFirstBidPutObject, modifiedSecondBidPutObject, modifiedThirdBidPutObject); + } + private AuctionContext givenAuctionContext(UnaryOperator accountCustomizer, UnaryOperator bidRequestCustomizer) { From 9a8c6b68146b96b01fe477a8e20cb3ffdfe85110 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Fri, 14 Feb 2025 14:45:04 -0500 Subject: [PATCH 2/7] Extend test a bit --- src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java index cf9639027b7..5c1f87d918c 100644 --- a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java +++ b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java @@ -939,7 +939,7 @@ public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() thr "ApiKey", false, true, - "apac", + "apacific", vastModifier, eventsService, metrics, From 6d4017bd84e87eed5fb11da1d8afc8b33fb903a3 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Sat, 15 Feb 2025 19:53:31 -0500 Subject: [PATCH 3/7] Emit TTL metrics per creative type per account --- .../prebid/server/cache/CoreCacheService.java | 18 +++- .../metric/CacheCreativeTtlMetrics.java | 19 ++++ .../prebid/server/metric/CacheMetrics.java | 7 ++ .../org/prebid/server/metric/Metrics.java | 5 ++ .../server/cache/CoreCacheServiceTest.java | 90 ++++++++++++++++++- .../org/prebid/server/metric/MetricsTest.java | 19 ++++ 6 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/prebid/server/metric/CacheCreativeTtlMetrics.java diff --git a/src/main/java/org/prebid/server/cache/CoreCacheService.java b/src/main/java/org/prebid/server/cache/CoreCacheService.java index 5647a45fd0f..a43f242b94c 100644 --- a/src/main/java/org/prebid/server/cache/CoreCacheService.java +++ b/src/main/java/org/prebid/server/cache/CoreCacheService.java @@ -148,7 +148,8 @@ public String cacheVideoDebugLog(CachedDebugLog cachedDebugLog, Integer videoCac return cacheKey; } - private CachedCreative makeDebugCacheCreative(CachedDebugLog videoCacheDebugLog, String hbCacheId, + private CachedCreative makeDebugCacheCreative(CachedDebugLog videoCacheDebugLog, + String hbCacheId, Integer videoCacheTtl) { final JsonNode value = mapper.mapper().valueToTree(videoCacheDebugLog.buildCacheBody()); videoCacheDebugLog.setCacheKey(hbCacheId); @@ -535,10 +536,21 @@ private static String resolveVideoBidUuid(String uuid, String hbCacheId) { } private void updateCreativeMetrics(String accountId, List cachedCreatives) { - for (final CachedCreative cachedCreative : cachedCreatives) { + for (CachedCreative cachedCreative : cachedCreatives) { + final MetricName creativeType = resolveCreativeTypeName(cachedCreative.getPayload()); + final Integer creativeTtl = cachedCreative.getPayload().getTtlseconds() != null + ? cachedCreative.getPayload().getTtlseconds() + : cachedCreative.getPayload().getExpiry(); + + if (creativeTtl != null) { + metrics.updateCacheCreativeTtl(accountId, + creativeTtl, + creativeType); + } + metrics.updateCacheCreativeSize(accountId, cachedCreative.getSize(), - resolveCreativeTypeName(cachedCreative.getPayload())); + creativeType); } } diff --git a/src/main/java/org/prebid/server/metric/CacheCreativeTtlMetrics.java b/src/main/java/org/prebid/server/metric/CacheCreativeTtlMetrics.java new file mode 100644 index 00000000000..f5325c382aa --- /dev/null +++ b/src/main/java/org/prebid/server/metric/CacheCreativeTtlMetrics.java @@ -0,0 +1,19 @@ +package org.prebid.server.metric; + +import com.codahale.metrics.MetricRegistry; + +import java.util.Objects; +import java.util.function.Function; + +public class CacheCreativeTtlMetrics extends UpdatableMetrics { + + CacheCreativeTtlMetrics(MetricRegistry metricRegistry, CounterType counterType, String prefix) { + super(Objects.requireNonNull(metricRegistry), + Objects.requireNonNull(counterType), + nameCreator(Objects.requireNonNull(prefix))); + } + + private static Function nameCreator(String prefix) { + return metricName -> "%s.creative_ttl.%s".formatted(prefix, metricName); + } +} diff --git a/src/main/java/org/prebid/server/metric/CacheMetrics.java b/src/main/java/org/prebid/server/metric/CacheMetrics.java index 23e1a2ea8d8..5116e10e08f 100644 --- a/src/main/java/org/prebid/server/metric/CacheMetrics.java +++ b/src/main/java/org/prebid/server/metric/CacheMetrics.java @@ -12,6 +12,7 @@ class CacheMetrics extends UpdatableMetrics { private final RequestMetrics requestsMetrics; private final CacheCreativeSizeMetrics cacheCreativeSizeMetrics; + private final CacheCreativeTtlMetrics cacheCreativeTtlMetrics; CacheMetrics(MetricRegistry metricRegistry, CounterType counterType) { super( @@ -21,6 +22,7 @@ class CacheMetrics extends UpdatableMetrics { requestsMetrics = new RequestMetrics(metricRegistry, counterType, createPrefix()); cacheCreativeSizeMetrics = new CacheCreativeSizeMetrics(metricRegistry, counterType, createPrefix()); + cacheCreativeTtlMetrics = new CacheCreativeTtlMetrics(metricRegistry, counterType, createPrefix()); } CacheMetrics(MetricRegistry metricRegistry, CounterType counterType, String prefix) { @@ -31,6 +33,7 @@ class CacheMetrics extends UpdatableMetrics { requestsMetrics = new RequestMetrics(metricRegistry, counterType, createPrefix(prefix)); cacheCreativeSizeMetrics = new CacheCreativeSizeMetrics(metricRegistry, counterType, createPrefix(prefix)); + cacheCreativeTtlMetrics = new CacheCreativeTtlMetrics(metricRegistry, counterType, createPrefix(prefix)); } private static String createPrefix(String prefix) { @@ -52,4 +55,8 @@ RequestMetrics requests() { CacheCreativeSizeMetrics creativeSize() { return cacheCreativeSizeMetrics; } + + CacheCreativeTtlMetrics creativeTtl() { + return cacheCreativeTtlMetrics; + } } diff --git a/src/main/java/org/prebid/server/metric/Metrics.java b/src/main/java/org/prebid/server/metric/Metrics.java index b4f3a5af9c3..2d0cc1aeaf2 100644 --- a/src/main/java/org/prebid/server/metric/Metrics.java +++ b/src/main/java/org/prebid/server/metric/Metrics.java @@ -694,6 +694,11 @@ public void updateAccountModuleDurationMetric(Account account, String moduleCode } } + public void updateCacheCreativeTtl(String accountId, Integer creativeTtl, MetricName creativeType) { + cache().creativeTtl().updateHistogram(creativeType, creativeTtl); + forAccount(accountId).cache().creativeTtl().updateHistogram(creativeType, creativeTtl); + } + private static class HookMetricMapper { private static final EnumMap STATUS_TO_METRIC = diff --git a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java index 5c1f87d918c..56e34c24aa6 100644 --- a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java +++ b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java @@ -73,6 +73,7 @@ import static org.mockito.BDDMockito.given; import static org.mockito.Mock.Strictness.LENIENT; import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -440,9 +441,10 @@ public void cacheBidsOpenrtbShouldPerformHttpRequestWithExpectedBody() throws IO ExtPrebid.of(ExtBidPrebid.builder().bidid("generatedId").build(), emptyMap())); final String receivedBid2Adm = "adm2"; - final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1"); + final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") + .toBuilder().ttl(1).build(); final BidInfo bidInfo2 = givenBidInfo(builder -> builder.id("bidId2").adm(receivedBid2Adm).ext(bidExt2), - BidType.video, "bidder2"); + BidType.video, "bidder2").toBuilder().ttl(2).build(); final EventsContext eventsContext = EventsContext.builder() .auctionId("auctionId") @@ -465,6 +467,10 @@ public void cacheBidsOpenrtbShouldPerformHttpRequestWithExpectedBody() throws IO verify(metrics).updateCacheCreativeSize(eq("accountId"), eq(4), eq(MetricName.json)); verify(metrics).updateCacheCreativeSize(eq("accountId"), eq(4), eq(MetricName.xml)); + verify(metrics).updateCacheCreativeTtl(eq("accountId"), eq(1), eq(MetricName.json)); + verify(metrics).updateCacheCreativeTtl(eq("accountId"), eq(2), eq(MetricName.json)); + + final Bid bid1 = bidInfo1.getBid(); final Bid bid2 = bidInfo2.getBid(); @@ -472,9 +478,9 @@ public void cacheBidsOpenrtbShouldPerformHttpRequestWithExpectedBody() throws IO assertThat(bidCacheRequest.getPuts()) .containsExactly( BidPutObject.builder().aid("auctionId").type("json") - .value(mapper.valueToTree(bid1)).build(), + .value(mapper.valueToTree(bid1)).ttlseconds(1).build(), BidPutObject.builder().aid("auctionId").type("json") - .value(mapper.valueToTree(bid2)).build(), + .value(mapper.valueToTree(bid2)).ttlseconds(2).build(), BidPutObject.builder().aid("auctionId").type("xml") .value(new TextNode(receivedBid2Adm)).build()); } @@ -744,6 +750,7 @@ public void cachePutObjectsShould() throws IOException { .bidder("bidder1") .timestamp(1L) .value(new TextNode("vast")) + .ttlseconds(1) .build(); final BidPutObject secondBidPutObject = BidPutObject.builder() .type("xml") @@ -751,6 +758,7 @@ public void cachePutObjectsShould() throws IOException { .bidder("bidder2") .timestamp(1L) .value(new TextNode("VAST")) + .ttlseconds(2) .build(); final BidPutObject thirdBidPutObject = BidPutObject.builder() .type("text") @@ -758,6 +766,7 @@ public void cachePutObjectsShould() throws IOException { .bidder("bidder3") .timestamp(1L) .value(new TextNode("VAST")) + .ttlseconds(3) .build(); given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) @@ -779,6 +788,10 @@ public void cachePutObjectsShould() throws IOException { verify(metrics).updateCacheCreativeSize(eq("account"), eq(4), eq(MetricName.xml)); verify(metrics).updateCacheCreativeSize(eq("account"), eq(11), eq(MetricName.unknown)); + verify(metrics).updateCacheCreativeTtl(eq("account"), eq(1), eq(MetricName.json)); + verify(metrics).updateCacheCreativeTtl(eq("account"), eq(2), eq(MetricName.xml)); + verify(metrics).updateCacheCreativeTtl(eq("account"), eq(3), eq(MetricName.unknown)); + verify(vastModifier).modifyVastXml(true, singleton("bidder1"), firstBidPutObject, "account", "pbjs"); verify(vastModifier).modifyVastXml(true, singleton("bidder1"), secondBidPutObject, "account", "pbjs"); @@ -1011,6 +1024,75 @@ public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() thr .containsExactly(modifiedFirstBidPutObject, modifiedSecondBidPutObject, modifiedThirdBidPutObject); } + @Test + public void cacheBidsOpenrtbShouldNotEmitEmptyTtlMetrics() { + // given + final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") + .toBuilder() + .ttl(null) + .build(); + final BidInfo bidInfo2 = givenBidInfo(builder -> builder.id("bidId2"), BidType.video, "bidder2") + .toBuilder() + .ttl(null) + .build(); + + final EventsContext eventsContext = EventsContext.builder() + .auctionId("auctionId") + .auctionTimestamp(1000L) + .build(); + + // when + target.cacheBidsOpenrtb( + asList(bidInfo1, bidInfo2), + givenAuctionContext(), + CacheContext.builder() + .shouldCacheBids(true) + .shouldCacheVideoBids(true) + .build(), + eventsContext); + + // then + verify(metrics, never()).updateCacheCreativeTtl(any(), any(), any()); + } + + @Test + public void cachePutObjectsShouldPrependTraceInfo() { + // given + final BidPutObject firstBidPutObject = BidPutObject.builder() + .type("json") + .bidid("bidId1") + .bidder("bidder1") + .timestamp(1L) + .value(new TextNode("vast")) + .ttlseconds(null) + .build(); + final BidPutObject secondBidPutObject = BidPutObject.builder() + .type("xml") + .bidid("bidId2") + .bidder("bidder2") + .timestamp(1L) + .value(new TextNode("VAST")) + .ttlseconds(null) + .build(); + + given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) + .willReturn(new TextNode("modifiedVast")) + .willReturn(new TextNode("VAST")) + .willReturn(new TextNode("updatedVast")); + + // when + target.cachePutObjects( + asList(firstBidPutObject, secondBidPutObject), + true, + singleton("bidder1"), + "account", + "pbjs", + timeout); + + // then + verify(metrics, never()).updateCacheCreativeTtl(any(), any(), any()); + } + private AuctionContext givenAuctionContext(UnaryOperator accountCustomizer, UnaryOperator bidRequestCustomizer) { diff --git a/src/test/java/org/prebid/server/metric/MetricsTest.java b/src/test/java/org/prebid/server/metric/MetricsTest.java index f40731dcb3c..6f33ceab01f 100644 --- a/src/test/java/org/prebid/server/metric/MetricsTest.java +++ b/src/test/java/org/prebid/server/metric/MetricsTest.java @@ -1532,6 +1532,25 @@ public void shouldIncrementUpdateAccountRequestRejectedByFailedFetchCount() { .isEqualTo(1); } + @Test + public void shouldIncrementPrebidCacheCreativeTtlHistogram() { + // when + metrics.updateCacheCreativeTtl("accountId", 123, MetricName.json); + metrics.updateCacheCreativeTtl("accountId", 456, MetricName.xml); + metrics.updateCacheCreativeTtl("accountId", 789, MetricName.unknown); + + // then + assertThat(metricRegistry.histogram("prebid_cache.creative_ttl.json").getCount()).isEqualTo(1); + assertThat(metricRegistry.histogram("account.accountId.prebid_cache.creative_ttl.json").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.histogram("prebid_cache.creative_ttl.xml").getCount()).isEqualTo(1); + assertThat(metricRegistry.histogram("account.accountId.prebid_cache.creative_ttl.xml").getCount()) + .isEqualTo(1); + assertThat(metricRegistry.histogram("prebid_cache.creative_ttl.unknown").getCount()).isEqualTo(1); + assertThat(metricRegistry.histogram("account.accountId.prebid_cache.creative_ttl.unknown").getCount()) + .isEqualTo(1); + } + private void verifyCreatesConfiguredCounterType(Consumer metricsConsumer) { final EnumMap> counterTypeClasses = new EnumMap<>(CounterType.class); counterTypeClasses.put(CounterType.counter, Counter.class); From b56fe4a8a4d667640552f4aaad4ff31fab69ccb4 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Mon, 17 Feb 2025 23:56:58 -0500 Subject: [PATCH 4/7] Remarks + tests --- .../prebid/server/cache/CoreCacheService.java | 58 ++-- .../server/cache/CoreCacheServiceTest.java | 296 ++++++++++++------ 2 files changed, 224 insertions(+), 130 deletions(-) diff --git a/src/main/java/org/prebid/server/cache/CoreCacheService.java b/src/main/java/org/prebid/server/cache/CoreCacheService.java index a43f242b94c..f38113a2460 100644 --- a/src/main/java/org/prebid/server/cache/CoreCacheService.java +++ b/src/main/java/org/prebid/server/cache/CoreCacheService.java @@ -7,6 +7,7 @@ import io.vertx.core.Future; import io.vertx.core.MultiMap; import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.prebid.server.auction.model.AuctionContext; import org.prebid.server.auction.model.BidInfo; @@ -151,6 +152,7 @@ public String cacheVideoDebugLog(CachedDebugLog cachedDebugLog, Integer videoCac private CachedCreative makeDebugCacheCreative(CachedDebugLog videoCacheDebugLog, String hbCacheId, Integer videoCacheTtl) { + final JsonNode value = mapper.mapper().valueToTree(videoCacheDebugLog.buildCacheBody()); videoCacheDebugLog.setCacheKey(hbCacheId); return CachedCreative.of(BidPutObject.builder() @@ -415,29 +417,26 @@ private CachedCreative createXmlPutObjectOpenrtb(CacheBid cacheBid, String requestId, String hbCacheId, String accountId) { + final BidInfo bidInfo = cacheBid.getBidInfo(); final Bid bid = bidInfo.getBid(); final String vastXml = bid.getAdm(); - final String resolvedCacheKey = resolveCacheKey(accountId, hbCacheId); - final String customCacheKey = resolveCustomCacheKey(resolvedCacheKey, bidInfo.getCategory()); - final BidPutObject payload = BidPutObject.builder() .aid(requestId) .type("xml") - .key(customCacheKey != null ? customCacheKey : resolvedCacheKey) + .key(resolveCacheKey(accountId, hbCacheId, bidInfo.getCategory())) .value(vastXml != null ? new TextNode(vastXml) : null) .ttlseconds(cacheBid.getTtl()) - .key(customCacheKey) .build(); return CachedCreative.of(payload, creativeSizeFromTextNode(payload.getValue())); } - private static String resolveCustomCacheKey(String hbCacheId, String category) { + private static String formatCategoryMappedCacheKey(String hbCacheId, String category) { return StringUtils.isNoneEmpty(category, hbCacheId) ? "%s_%s".formatted(category, hbCacheId) - : null; + : hbCacheId; } private String generateWinUrl(String bidId, @@ -537,20 +536,15 @@ private static String resolveVideoBidUuid(String uuid, String hbCacheId) { private void updateCreativeMetrics(String accountId, List cachedCreatives) { for (CachedCreative cachedCreative : cachedCreatives) { - final MetricName creativeType = resolveCreativeTypeName(cachedCreative.getPayload()); - final Integer creativeTtl = cachedCreative.getPayload().getTtlseconds() != null - ? cachedCreative.getPayload().getTtlseconds() - : cachedCreative.getPayload().getExpiry(); + final BidPutObject payload = cachedCreative.getPayload(); + final MetricName creativeType = resolveCreativeTypeName(payload); + final Integer creativeTtl = ObjectUtils.defaultIfNull(payload.getTtlseconds(), payload.getExpiry()); if (creativeTtl != null) { - metrics.updateCacheCreativeTtl(accountId, - creativeTtl, - creativeType); + metrics.updateCacheCreativeTtl(accountId, creativeTtl, creativeType); } - metrics.updateCacheCreativeSize(accountId, - cachedCreative.getSize(), - creativeType); + metrics.updateCacheCreativeSize(accountId, cachedCreative.getSize(), creativeType); } } @@ -586,17 +580,21 @@ private BidCacheRequest toBidCacheRequest(List cachedCreatives) .toList()); } + private String resolveCacheKey(String accountId, String existingKey, String category) { + final String resolvedCacheKey = resolveCacheKey(accountId, existingKey); + return formatCategoryMappedCacheKey(resolvedCacheKey, category); + + } + private String resolveCacheKey(String accountId) { return resolveCacheKey(accountId, null); } - // hbCacheId is accepted here to have a backwards-compatibility with video category mapping - private String resolveCacheKey(String accountId, String hbCacheId) { - if (!appendTraceInfoToCacheId) { - return hbCacheId; + private String resolveCacheKey(String accountId, String existingCacheKey) { + if (!appendTraceInfoToCacheId || existingCacheKey != null) { + return existingCacheKey; } - // no need to have additional separator if datacenter name won't be added final boolean isDatacenterNamePopulated = StringUtils.isNotBlank(datacenterRegion); final int separatorCount = isDatacenterNamePopulated ? 2 : 1; final int accountIdLength = accountId.length(); @@ -604,15 +602,15 @@ private String resolveCacheKey(String accountId, String hbCacheId) { ? accountIdLength + datacenterRegion.length() + separatorCount : accountIdLength + separatorCount; - final String cacheKey = hbCacheId != null ? hbCacheId : idGenerator.generateId(); - if (cacheKey != null && traceInfoLength < cacheKey.length()) { - final String substring = cacheKey.substring(0, cacheKey.length() - traceInfoLength); - return isDatacenterNamePopulated - ? accountId + TRACE_INFO_SEPARATOR + datacenterRegion + TRACE_INFO_SEPARATOR + substring - : accountId + TRACE_INFO_SEPARATOR + substring; - } else { - return hbCacheId; + final String cacheKey = idGenerator.generateId(); + if (cacheKey == null || traceInfoLength >= cacheKey.length()) { + return null; } + + final String substring = cacheKey.substring(0, cacheKey.length() - traceInfoLength); + return isDatacenterNamePopulated + ? accountId + TRACE_INFO_SEPARATOR + datacenterRegion + TRACE_INFO_SEPARATOR + substring + : accountId + TRACE_INFO_SEPARATOR + substring; } private String normalizeDatacenterRegion(String datacenterRegion) { diff --git a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java index 56e34c24aa6..daace6a8d32 100644 --- a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java +++ b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java @@ -470,7 +470,6 @@ public void cacheBidsOpenrtbShouldPerformHttpRequestWithExpectedBody() throws IO verify(metrics).updateCacheCreativeTtl(eq("accountId"), eq(1), eq(MetricName.json)); verify(metrics).updateCacheCreativeTtl(eq("accountId"), eq(2), eq(MetricName.json)); - final Bid bid1 = bidInfo1.getBid(); final Bid bid2 = bidInfo2.getBid(); @@ -859,7 +858,7 @@ public void cachePutObjectsShouldUseApiKeyWhenProvided() throws MalformedURLExce } @Test - public void cachePutObjectsShouldPrependTraceInfoWhenEnabled() throws IOException { + public void cacheBidsOpenrtbShouldPrependTraceInfoWhenEnabled() throws IOException { // given target = new CoreCacheService( httpClient, @@ -877,38 +876,157 @@ public void cachePutObjectsShouldPrependTraceInfoWhenEnabled() throws IOExceptio idGenerator, jacksonMapper); - given(idGenerator.generateId()).willReturn("high-entropy-cache-id"); + given(idGenerator.generateId()) + .willReturn("1-high-entropy-cache-id") + .willReturn("2-high-entropy-cache-id") + .willReturn("3-high-entropy-cache-id"); - final BidPutObject firstBidPutObject = BidPutObject.builder() - .type("json") - .bidid("bidId1") - .bidder("bidder1") - .timestamp(1L) - .value(new TextNode("vast")) + final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") + .toBuilder() .build(); - final BidPutObject secondBidPutObject = BidPutObject.builder() - .type("xml") - .bidid("bidId2") - .bidder("bidder2") - .timestamp(1L) - .value(new TextNode("VAST")) + final BidInfo bidInfo2 = givenBidInfo(builder -> builder.id("bidId2").adm("adm"), BidType.video, "bidder2") + .toBuilder() .build(); - final BidPutObject thirdBidPutObject = BidPutObject.builder() + + final EventsContext eventsContext = EventsContext.builder() + .auctionId("auctionId") + .auctionTimestamp(1000L) + .build(); + + // when + final Future future = target.cacheBidsOpenrtb( + asList(bidInfo1, bidInfo2), + givenAuctionContext(), + CacheContext.builder() + .shouldCacheBids(true) + .shouldCacheVideoBids(true) + .build(), + eventsContext); + + // then + assertThat(captureBidCacheRequest().getPuts()) + .containsExactly( + BidPutObject.builder() + .aid("auctionId") + .type("json") + .key("accountId-1-high-entrop") + .value(mapper.valueToTree(bidInfo1.getBid())) + .build(), + BidPutObject.builder() + .aid("auctionId") + .type("json") + .key("accountId-2-high-entrop") + .value(mapper.valueToTree(bidInfo2.getBid())) + .build(), + BidPutObject.builder() + .aid("auctionId") + .type("xml") + .key("accountId-3-high-entrop") + .value(new TextNode("adm")) + .build()); + } + + @Test + public void cacheBidsOpenrtbShouldPrependTraceInfoWithDatacenterWhenEnabled() throws IOException { + // given + target = new CoreCacheService( + httpClient, + new URL("http://cache-service/cache"), + "http://cache-service-host/cache?uuid=", + 100L, + "ApiKey", + false, + true, + "apacific", + vastModifier, + eventsService, + metrics, + clock, + idGenerator, + jacksonMapper); + + given(idGenerator.generateId()) + .willReturn("1-high-entropy-cache-id") + .willReturn("2-high-entropy-cache-id") + .willReturn("3-high-entropy-cache-id"); + + final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") + .toBuilder() + .build(); + final BidInfo bidInfo2 = givenBidInfo(builder -> builder.id("bidId2").adm("adm"), BidType.video, "bidder2") + .toBuilder() + .build(); + + final EventsContext eventsContext = EventsContext.builder() + .auctionId("auctionId") + .auctionTimestamp(1000L) + .build(); + + // when + final Future future = target.cacheBidsOpenrtb( + asList(bidInfo1, bidInfo2), + givenAuctionContext(), + CacheContext.builder() + .shouldCacheBids(true) + .shouldCacheVideoBids(true) + .build(), + eventsContext); + + // then + assertThat(captureBidCacheRequest().getPuts()) + .containsExactly( + BidPutObject.builder() + .aid("auctionId") + .type("json") + .key("accountId-apac-1-high-e") + .value(mapper.valueToTree(bidInfo1.getBid())) + .build(), + BidPutObject.builder() + .aid("auctionId") + .type("json") + .key("accountId-apac-2-high-e") + .value(mapper.valueToTree(bidInfo2.getBid())) + .build(), + BidPutObject.builder() + .aid("auctionId") + .type("xml") + .key("accountId-apac-3-high-e") + .value(new TextNode("adm")) + .build()); + } + + @Test + public void cachePutObjectsShouldPrependTraceInfoWhenEnabled() throws IOException { + // given + target = new CoreCacheService( + httpClient, + new URL("http://cache-service/cache"), + "http://cache-service-host/cache?uuid=", + 100L, + "ApiKey", + false, + true, + null, + vastModifier, + eventsService, + metrics, + clock, + idGenerator, + jacksonMapper); + + given(idGenerator.generateId()).willReturn("high-entropy-cache-id"); + + final BidPutObject bidPutObject = BidPutObject.builder() .type("text") - .bidid("bidId3") - .bidder("bidder3") - .timestamp(1L) .value(new TextNode("VAST")) .build(); given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) - .willReturn(new TextNode("modifiedVast")) - .willReturn(new TextNode("VAST")) - .willReturn(new TextNode("updatedVast")); + .willReturn(new TextNode("modifiedVast")); // when target.cachePutObjects( - asList(firstBidPutObject, secondBidPutObject, thirdBidPutObject), + singletonList(bidPutObject), true, singleton("bidder1"), "account", @@ -916,29 +1034,12 @@ public void cachePutObjectsShouldPrependTraceInfoWhenEnabled() throws IOExceptio timeout); // then - final BidPutObject modifiedFirstBidPutObject = firstBidPutObject.toBuilder() - .bidid(null) - .bidder(null) - .timestamp(null) + final BidPutObject modifiedBidPutObject = bidPutObject.toBuilder() .key("account-high-entropy-") .value(new TextNode("modifiedVast")) .build(); - final BidPutObject modifiedSecondBidPutObject = secondBidPutObject.toBuilder() - .bidid(null) - .bidder(null) - .timestamp(null) - .key("account-high-entropy-") - .build(); - final BidPutObject modifiedThirdBidPutObject = thirdBidPutObject.toBuilder() - .bidid(null) - .bidder(null) - .timestamp(null) - .key("account-high-entropy-") - .value(new TextNode("updatedVast")) - .build(); - assertThat(captureBidCacheRequest().getPuts()) - .containsExactly(modifiedFirstBidPutObject, modifiedSecondBidPutObject, modifiedThirdBidPutObject); + assertThat(captureBidCacheRequest().getPuts()).containsExactly(modifiedBidPutObject); } @Test @@ -962,36 +1063,17 @@ public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() thr given(idGenerator.generateId()).willReturn("high-entropy-cache-id"); - final BidPutObject firstBidPutObject = BidPutObject.builder() - .type("json") - .bidid("bidId1") - .bidder("bidder1") - .timestamp(1L) - .value(new TextNode("vast")) - .build(); - final BidPutObject secondBidPutObject = BidPutObject.builder() - .type("xml") - .bidid("bidId2") - .bidder("bidder2") - .timestamp(1L) - .value(new TextNode("VAST")) - .build(); - final BidPutObject thirdBidPutObject = BidPutObject.builder() + final BidPutObject bidPutObject = BidPutObject.builder() .type("text") - .bidid("bidId3") - .bidder("bidder3") - .timestamp(1L) .value(new TextNode("VAST")) .build(); given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) - .willReturn(new TextNode("modifiedVast")) - .willReturn(new TextNode("VAST")) - .willReturn(new TextNode("updatedVast")); + .willReturn(new TextNode("modifiedVast")); // when target.cachePutObjects( - asList(firstBidPutObject, secondBidPutObject, thirdBidPutObject), + singletonList(bidPutObject), true, singleton("bidder1"), "account", @@ -999,29 +1081,59 @@ public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() thr timeout); // then - final BidPutObject modifiedFirstBidPutObject = firstBidPutObject.toBuilder() - .bidid(null) - .bidder(null) - .timestamp(null) + final BidPutObject modifiedBidPutObject = bidPutObject.toBuilder() .key("account-apac-high-ent") .value(new TextNode("modifiedVast")) .build(); - final BidPutObject modifiedSecondBidPutObject = secondBidPutObject.toBuilder() - .bidid(null) - .bidder(null) - .timestamp(null) - .key("account-apac-high-ent") + + assertThat(captureBidCacheRequest().getPuts()).containsExactly(modifiedBidPutObject); + } + + @Test + public void cachePutObjectsShouldNotPrependTraceInfoToPassedInKey() throws IOException { + // given + target = new CoreCacheService( + httpClient, + new URL("http://cache-service/cache"), + "http://cache-service-host/cache?uuid=", + 100L, + "ApiKey", + false, + true, + null, + vastModifier, + eventsService, + metrics, + clock, + idGenerator, + jacksonMapper); + + final BidPutObject bidPutObject = BidPutObject.builder() + .type("text") + .key("passed-in-key") + .value(new TextNode("VAST")) .build(); - final BidPutObject modifiedThirdBidPutObject = thirdBidPutObject.toBuilder() - .bidid(null) - .bidder(null) - .timestamp(null) - .key("account-apac-high-ent") - .value(new TextNode("updatedVast")) + + given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) + .willReturn(new TextNode("modifiedVast")); + + // when + target.cachePutObjects( + singletonList(bidPutObject), + true, + singleton("bidder1"), + "account", + "pbjs", + timeout); + + // then + final BidPutObject modifiedBidPutObject = bidPutObject.toBuilder() + .key("passed-in-key") + .value(new TextNode("modifiedVast")) .build(); - assertThat(captureBidCacheRequest().getPuts()) - .containsExactly(modifiedFirstBidPutObject, modifiedSecondBidPutObject, modifiedThirdBidPutObject); + assertThat(captureBidCacheRequest().getPuts()).containsExactly(modifiedBidPutObject); + verify(idGenerator, never()).generateId(); } @Test @@ -1029,11 +1141,9 @@ public void cacheBidsOpenrtbShouldNotEmitEmptyTtlMetrics() { // given final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") .toBuilder() - .ttl(null) .build(); final BidInfo bidInfo2 = givenBidInfo(builder -> builder.id("bidId2"), BidType.video, "bidder2") .toBuilder() - .ttl(null) .build(); final EventsContext eventsContext = EventsContext.builder() @@ -1056,33 +1166,19 @@ public void cacheBidsOpenrtbShouldNotEmitEmptyTtlMetrics() { } @Test - public void cachePutObjectsShouldPrependTraceInfo() { + public void cachePutObjectsShouldNotEmitEmptyTtlMetrics() { // given final BidPutObject firstBidPutObject = BidPutObject.builder() .type("json") - .bidid("bidId1") - .bidder("bidder1") - .timestamp(1L) .value(new TextNode("vast")) - .ttlseconds(null) - .build(); - final BidPutObject secondBidPutObject = BidPutObject.builder() - .type("xml") - .bidid("bidId2") - .bidder("bidder2") - .timestamp(1L) - .value(new TextNode("VAST")) - .ttlseconds(null) .build(); given(vastModifier.modifyVastXml(any(), any(), any(), any(), anyString())) - .willReturn(new TextNode("modifiedVast")) - .willReturn(new TextNode("VAST")) - .willReturn(new TextNode("updatedVast")); + .willReturn(new TextNode("modifiedVast")); // when target.cachePutObjects( - asList(firstBidPutObject, secondBidPutObject), + singletonList(firstBidPutObject), true, singleton("bidder1"), "account", From 65cc1f326d63d7c352735b5d4f9e2becc300aa99 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Tue, 18 Feb 2025 09:17:14 -0500 Subject: [PATCH 5/7] Add a safeguard against making UUID entirely not unique --- .../prebid/server/cache/CoreCacheService.java | 2 +- .../server/cache/CoreCacheServiceTest.java | 66 ++++++++++++++++--- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/prebid/server/cache/CoreCacheService.java b/src/main/java/org/prebid/server/cache/CoreCacheService.java index f38113a2460..001b8bc06c1 100644 --- a/src/main/java/org/prebid/server/cache/CoreCacheService.java +++ b/src/main/java/org/prebid/server/cache/CoreCacheService.java @@ -603,7 +603,7 @@ private String resolveCacheKey(String accountId, String existingCacheKey) { : accountIdLength + separatorCount; final String cacheKey = idGenerator.generateId(); - if (cacheKey == null || traceInfoLength >= cacheKey.length()) { + if (cacheKey == null || traceInfoLength >= (cacheKey.length() / 2)) { return null; } diff --git a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java index daace6a8d32..ebb573edbe0 100644 --- a/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java +++ b/src/test/java/org/prebid/server/cache/CoreCacheServiceTest.java @@ -946,9 +946,9 @@ public void cacheBidsOpenrtbShouldPrependTraceInfoWithDatacenterWhenEnabled() th jacksonMapper); given(idGenerator.generateId()) - .willReturn("1-high-entropy-cache-id") - .willReturn("2-high-entropy-cache-id") - .willReturn("3-high-entropy-cache-id"); + .willReturn("1-high-entropy-cache-id-foo-bar-") + .willReturn("2-high-entropy-cache-id-foo-bar-") + .willReturn("3-high-entropy-cache-id-foo-bar-"); final BidInfo bidInfo1 = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") .toBuilder() @@ -978,23 +978,73 @@ public void cacheBidsOpenrtbShouldPrependTraceInfoWithDatacenterWhenEnabled() th BidPutObject.builder() .aid("auctionId") .type("json") - .key("accountId-apac-1-high-e") + .key("accountId-apac-1-high-entropy-ca") .value(mapper.valueToTree(bidInfo1.getBid())) .build(), BidPutObject.builder() .aid("auctionId") .type("json") - .key("accountId-apac-2-high-e") + .key("accountId-apac-2-high-entropy-ca") .value(mapper.valueToTree(bidInfo2.getBid())) .build(), BidPutObject.builder() .aid("auctionId") .type("xml") - .key("accountId-apac-3-high-e") + .key("accountId-apac-3-high-entropy-ca") .value(new TextNode("adm")) .build()); } + @Test + public void cacheBidsOpenrtbShouldNotPrependTraceInfoToLowEntoryCacheIds() throws IOException { + // given + target = new CoreCacheService( + httpClient, + new URL("http://cache-service/cache"), + "http://cache-service-host/cache?uuid=", + 100L, + "ApiKey", + false, + true, + "apacific", + vastModifier, + eventsService, + metrics, + clock, + idGenerator, + jacksonMapper); + + given(idGenerator.generateId()).willReturn("low-entropy"); + + final BidInfo bidInfo = givenBidInfo(builder -> builder.id("bidId1"), BidType.banner, "bidder1") + .toBuilder() + .build(); + + final EventsContext eventsContext = EventsContext.builder() + .auctionId("auctionId") + .auctionTimestamp(1000L) + .build(); + + // when + final Future future = target.cacheBidsOpenrtb( + singletonList(bidInfo), + givenAuctionContext(), + CacheContext.builder() + .shouldCacheBids(true) + .shouldCacheVideoBids(true) + .build(), + eventsContext); + + // then + assertThat(captureBidCacheRequest().getPuts()) + .containsExactly( + BidPutObject.builder() + .aid("auctionId") + .type("json") + .value(mapper.valueToTree(bidInfo.getBid())) + .build()); + } + @Test public void cachePutObjectsShouldPrependTraceInfoWhenEnabled() throws IOException { // given @@ -1061,7 +1111,7 @@ public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() thr idGenerator, jacksonMapper); - given(idGenerator.generateId()).willReturn("high-entropy-cache-id"); + given(idGenerator.generateId()).willReturn("high-entropy-cache-id-foo-bar"); final BidPutObject bidPutObject = BidPutObject.builder() .type("text") @@ -1082,7 +1132,7 @@ public void cachePutObjectsShouldPrependTraceInfoWithDatacenterWhenEnabled() thr // then final BidPutObject modifiedBidPutObject = bidPutObject.toBuilder() - .key("account-apac-high-ent") + .key("account-apac-high-entropy-cac") .value(new TextNode("modifiedVast")) .build(); From 69bf63da45db9385dfdab3a0b64c897aeccaa3c3 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhevedenko <720803+Net-burst@users.noreply.github.com> Date: Wed, 19 Feb 2025 10:56:46 -0500 Subject: [PATCH 6/7] Remarks --- src/main/java/org/prebid/server/cache/CoreCacheService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/prebid/server/cache/CoreCacheService.java b/src/main/java/org/prebid/server/cache/CoreCacheService.java index 001b8bc06c1..d3a8b198b24 100644 --- a/src/main/java/org/prebid/server/cache/CoreCacheService.java +++ b/src/main/java/org/prebid/server/cache/CoreCacheService.java @@ -613,7 +613,7 @@ private String resolveCacheKey(String accountId, String existingCacheKey) { : accountId + TRACE_INFO_SEPARATOR + substring; } - private String normalizeDatacenterRegion(String datacenterRegion) { + private static String normalizeDatacenterRegion(String datacenterRegion) { if (datacenterRegion == null) { return null; } From 4d9449a4aaa7845408fa4cb00cf7b92676940c34 Mon Sep 17 00:00:00 2001 From: osulzhenko <125548596+osulzhenko@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:20:09 +0200 Subject: [PATCH 7/7] Functional tests for prebid cache traceability (#3763) * Add functional tests for prebid cache trace ability * update test * Update after review * Update after review * Update after review --- .../model/request/cache/BidCachePut.groovy | 1 + .../server/functional/tests/CacheSpec.groovy | 289 +++++++++++++++--- 2 files changed, 247 insertions(+), 43 deletions(-) diff --git a/src/test/groovy/org/prebid/server/functional/model/request/cache/BidCachePut.groovy b/src/test/groovy/org/prebid/server/functional/model/request/cache/BidCachePut.groovy index 45556c3a4d1..054cc76663b 100644 --- a/src/test/groovy/org/prebid/server/functional/model/request/cache/BidCachePut.groovy +++ b/src/test/groovy/org/prebid/server/functional/model/request/cache/BidCachePut.groovy @@ -14,4 +14,5 @@ class BidCachePut implements ObjectMapperWrapper { String bidder Long timestamp String aid + String key } diff --git a/src/test/groovy/org/prebid/server/functional/tests/CacheSpec.groovy b/src/test/groovy/org/prebid/server/functional/tests/CacheSpec.groovy index 4f8dcf7675e..e145e5a972f 100644 --- a/src/test/groovy/org/prebid/server/functional/tests/CacheSpec.groovy +++ b/src/test/groovy/org/prebid/server/functional/tests/CacheSpec.groovy @@ -19,11 +19,23 @@ import static org.prebid.server.functional.model.response.auction.MediaType.VIDE class CacheSpec extends BaseSpec { - private final static String PBS_API_HEADER = 'x-pbc-api-key' + private static final String PBS_API_HEADER = 'x-pbc-api-key' + private static final Integer MAX_DATACENTER_REGION_LENGTH = 4 + private static final Integer DEFAULT_UUID_LENGTH = 36 + + private static final String XML_CREATIVE_SIZE_ACCOUNT_METRIC = "account.%s.prebid_cache.creative_size.xml" + private static final String JSON_CREATIVE_SIZE_ACCOUNT_METRIC = "account.%s.prebid_cache.creative_size.json" + private static final String XML_CREATIVE_TTL_ACCOUNT_METRIC = "account.%s.prebid_cache.creative_ttl.xml" + private static final String JSON_CREATIVE_TTL_ACCOUNT_METRIC = "account.%s.prebid_cache.creative_ttl.json" + private static final String CACHE_REQUEST_OK_ACCOUNT_METRIC = "account.%s.prebid_cache.requests.ok" + + private static final String XML_CREATIVE_SIZE_GLOBAL_METRIC = "prebid_cache.creative_size.xml" + private static final String JSON_CREATIVE_SIZE_GLOBAL_METRIC = "prebid_cache.creative_size.json" + private static final String CACHE_REQUEST_OK_GLOBAL_METRIC = "prebid_cache.requests.ok" def "PBS should update prebid_cache.creative_size.xml metric when xml creative is received"() { given: "Current value of metric prebid_cache.requests.ok" - def initialValue = getCurrentMetricValue(defaultPbsService, "prebid_cache.requests.ok") + def initialValue = getCurrentMetricValue(defaultPbsService, CACHE_REQUEST_OK_GLOBAL_METRIC) and: "Default VtrackRequest" def accountId = PBSUtils.randomNumber.toString() @@ -36,21 +48,22 @@ class CacheSpec extends BaseSpec { then: "prebid_cache.creative_size.xml metric should be updated" def metrics = defaultPbsService.sendCollectedMetricsRequest() def creativeSize = creative.bytes.length - assert metrics["prebid_cache.creative_size.xml"] == creativeSize - assert metrics["prebid_cache.requests.ok"] == initialValue + 1 + assert metrics[CACHE_REQUEST_OK_GLOBAL_METRIC] == initialValue + 1 + assert metrics[XML_CREATIVE_SIZE_GLOBAL_METRIC] == creativeSize and: "account..prebid_cache.creative_size.xml should be updated" - assert metrics["account.${accountId}.prebid_cache.creative_size.xml" as String] == creativeSize - assert metrics["account.${accountId}.prebid_cache.requests.ok" as String] == 1 + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(accountId)] == 1 + assert metrics[XML_CREATIVE_SIZE_ACCOUNT_METRIC.formatted(accountId)] == creativeSize } def "PBS should update prebid_cache.creative_size.json metric when json creative is received"() { given: "Current value of metric prebid_cache.requests.ok" - def initialValue = getCurrentMetricValue(defaultPbsService, "prebid_cache.requests.ok") + def initialValue = getCurrentMetricValue(defaultPbsService, CACHE_REQUEST_OK_GLOBAL_METRIC) and: "Default BidRequest with cache, targeting" - def bidRequest = BidRequest.defaultBidRequest - bidRequest.enableCache() + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + } and: "Default basic bid with banner creative" def asset = new Asset(id: PBSUtils.randomNumber) @@ -64,25 +77,26 @@ class CacheSpec extends BaseSpec { when: "PBS processes auction request" defaultPbsService.sendAuctionRequest(bidRequest) - and: "PBS processes collected metrics request" - def metrics = defaultPbsService.sendCollectedMetricsRequest() - then: "prebid_cache.creative_size.json should be update" def adm = bidResponse.seatbid[0].bid[0].getAdm() def creativeSize = adm.bytes.length - assert metrics["prebid_cache.creative_size.json"] == creativeSize - assert metrics["prebid_cache.requests.ok"] == initialValue + 1 + + and: "prebid_cache.creative_size.json metric should be updated" + def metrics = defaultPbsService.sendCollectedMetricsRequest() + assert metrics[CACHE_REQUEST_OK_GLOBAL_METRIC] == initialValue + 1 + assert metrics[JSON_CREATIVE_SIZE_GLOBAL_METRIC] == creativeSize and: "account..prebid_cache.creative_size.json should be update" - def accountId = bidRequest.site.publisher.id - assert metrics["account.${accountId}.prebid_cache.requests.ok" as String] == 1 + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == 1 + assert metrics[JSON_CREATIVE_SIZE_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == creativeSize } def "PBS should cache bids when targeting is specified"() { given: "Default BidRequest with cache, targeting" - def bidRequest = BidRequest.defaultBidRequest - bidRequest.enableCache() - bidRequest.ext.prebid.targeting = new Targeting() + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } when: "PBS processes auction request" defaultPbsService.sendAuctionRequest(bidRequest) @@ -100,15 +114,15 @@ class CacheSpec extends BaseSpec { def pbsService = pbsServiceFactory.getService(['pbc.api.key': apiKey, 'cache.api-key-secured': 'false']) and: "Default BidRequest with cache, targeting" - def bidRequest = BidRequest.defaultBidRequest - bidRequest.enableCache() - bidRequest.ext.prebid.targeting = new Targeting() + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } when: "PBS processes auction request" pbsService.sendAuctionRequest(bidRequest) then: "PBS should call PBC" - prebidCache.getRequest() assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 and: "PBS call shouldn't include api-key" @@ -121,26 +135,215 @@ class CacheSpec extends BaseSpec { def pbsService = pbsServiceFactory.getService(['pbc.api.key': apiKey, 'cache.api-key-secured': 'true']) and: "Default BidRequest with cache, targeting" - def bidRequest = BidRequest.defaultBidRequest - bidRequest.enableCache() - bidRequest.ext.prebid.targeting = new Targeting() + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } when: "PBS processes auction request" pbsService.sendAuctionRequest(bidRequest) then: "PBS should call PBC" - prebidCache.getRequest() assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 and: "PBS call should include api-key" assert prebidCache.getRequestHeaders(bidRequest.imp[0].id)[PBS_API_HEADER] == [apiKey] } + def "PBS should cache banner bids with cache key that include account and datacenter short name when append-trace-info-to-cache-id enabled"() { + given: "Pbs config with append-trace-info-to-cache-id" + def serverDataCenter = PBSUtils.randomString + def bannerHostTtl = PBSUtils.getRandomNumber(300, 1500) + def pbsConfig = ['cache.default-ttl-seconds.banner' : bannerHostTtl.toString(), + 'datacenter-region' : serverDataCenter, + 'cache.append-trace-info-to-cache-id': 'true' + ] + def pbsService = pbsServiceFactory.getService(pbsConfig) + + and: "Default BidRequest with cache, targeting" + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } + + when: "PBS processes auction request" + pbsService.sendAuctionRequest(bidRequest) + + then: "PBS should call PBC" + assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 + + and: "PBS cache key should start with account and datacenter short name" + def cacheKey = prebidCache.getRecordedRequests(bidRequest.imp.id.first).puts.flatten().first.key + assert cacheKey.startsWith("${bidRequest.accountId}-${serverDataCenter.take(MAX_DATACENTER_REGION_LENGTH)}") + + and: "PBS cache key should have length equal to default UUID" + assert cacheKey.length() == DEFAULT_UUID_LENGTH + + and: "PBS should include metrics for request" + def metrics = pbsService.sendCollectedMetricsRequest() + assert metrics[JSON_CREATIVE_TTL_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == bannerHostTtl + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + + def "PBS should cache video bids with cache key that include account and datacenter short name when append-trace-info-to-cache-id enabled"() { + given: "Pbs config with append-trace-info-to-cache-id" + def serverDataCenter = PBSUtils.randomString + def videoHostTtl = PBSUtils.getRandomNumber(300, 1500) + def pbsConfig = ['cache.default-ttl-seconds.video' : videoHostTtl.toString(), + 'datacenter-region' : serverDataCenter, + 'cache.append-trace-info-to-cache-id': 'true' + ] + def pbsService = pbsServiceFactory.getService(pbsConfig) + + and: "Default BidRequest with cache, targeting" + def bidRequest = BidRequest.defaultVideoRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } + + and: "Set bidder response" + def bidResponse = BidResponse.getDefaultBidResponse(bidRequest) + bidder.setResponse(bidRequest.id, bidResponse) + + when: "PBS processes auction request" + pbsService.sendAuctionRequest(bidRequest) + + then: "PBS should call PBC" + assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 + + and: "PBS cache key should start with account and datacenter short name" + def cacheKey = prebidCache.getRecordedRequests(bidRequest.imp.id.first).puts.flatten().first.key + assert cacheKey.startsWith("${bidRequest.accountId}-${serverDataCenter.take(MAX_DATACENTER_REGION_LENGTH)}") + + and: "PBS cache key should have length equal to default UUID" + assert cacheKey.length() == DEFAULT_UUID_LENGTH + + and: "PBS should include metrics for request" + def metrics = pbsService.sendCollectedMetricsRequest() + assert metrics[JSON_CREATIVE_TTL_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == videoHostTtl + assert metrics[XML_CREATIVE_TTL_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == videoHostTtl + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + + def "PBS should cache bids with cache key that include account when append-trace-info-to-cache-id enabled and datacenter is null"() { + given: "Pbs config with append-trace-info-to-cache-id" + def bannerHostTtl = PBSUtils.getRandomNumber(300, 1500) + def pbsConfig = ['cache.default-ttl-seconds.banner' : bannerHostTtl.toString(), + 'datacenter-region' : null, + 'cache.append-trace-info-to-cache-id': 'true' + ] + def pbsService = pbsServiceFactory.getService(pbsConfig) + + and: "Default BidRequest with cache, targeting" + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } + + when: "PBS processes auction request" + pbsService.sendAuctionRequest(bidRequest) + + then: "PBS should call PBC" + assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 + + and: "PBS cache key should start with account and datacenter short name" + def cacheKey = prebidCache.getRecordedRequests(bidRequest.imp.id.first).puts.flatten().first.key + assert cacheKey.startsWith("${bidRequest.accountId}-") + + and: "PBS cache key should have length equal to default UUID" + assert cacheKey.length() == DEFAULT_UUID_LENGTH + + and: "PBS should include metrics for request" + def metrics = pbsService.sendCollectedMetricsRequest() + assert metrics[JSON_CREATIVE_TTL_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == bannerHostTtl + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + + def "PBS should cache bids without cache key when account ID is too large"() { + given: "Pbs config with append-trace-info-to-cache-id" + def serverDataCenter = PBSUtils.randomString + def bannerHostTtl = PBSUtils.getRandomNumber(300, 1500) + def pbsConfig = ['cache.default-ttl-seconds.banner' : bannerHostTtl.toString(), + 'datacenter-region' : serverDataCenter, + 'cache.append-trace-info-to-cache-id': 'true' + ] + def pbsService = pbsServiceFactory.getService(pbsConfig) + + and: "Default BidRequest with cache, targeting and large account ID" + def accountOverflowLength = DEFAULT_UUID_LENGTH - MAX_DATACENTER_REGION_LENGTH - 2 + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + it.setAccountId(PBSUtils.getRandomString(accountOverflowLength)) + } + + when: "PBS processes auction request" + pbsService.sendAuctionRequest(bidRequest) + + then: "PBS should call PBC" + assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 + + and: "PBS shouldn't contain cache key" + assert !prebidCache.getRecordedRequests(bidRequest.imp.id.first).puts.flatten().first.key + + and: "PBS should include metrics for request" + def metrics = pbsService.sendCollectedMetricsRequest() + assert metrics[JSON_CREATIVE_TTL_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == bannerHostTtl + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + + def "PBS should cache bids without cache key when append-trace-info-to-cache-id disabled"() { + given: "Pbs config with append-trace-info-to-cache-id" + def bannerHostTtl = PBSUtils.getRandomNumber(300, 1500) + def serverDataCenter = PBSUtils.randomString + def pbsConfig = ['cache.default-ttl-seconds.banner' : bannerHostTtl.toString(), + 'datacenter-region' : serverDataCenter, + 'cache.append-trace-info-to-cache-id': 'false' + ] + def pbsService = pbsServiceFactory.getService(pbsConfig) + + and: "Default BidRequest with cache, targeting" + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = new Targeting() + } + + when: "PBS processes auction request" + pbsService.sendAuctionRequest(bidRequest) + + then: "PBS should call PBC" + assert prebidCache.getRequestCount(bidRequest.imp[0].id) == 1 + + and: "PBS shouldn't contain cache key" + assert !prebidCache.getRecordedRequests(bidRequest.imp.id.first).puts.flatten().first.key + + and: "PBS should include metrics for request" + def metrics = pbsService.sendCollectedMetricsRequest() + assert metrics[JSON_CREATIVE_TTL_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == bannerHostTtl + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(bidRequest.accountId)] == 1 + + cleanup: "Stop and remove pbs container" + pbsServiceFactory.removeContainer(pbsConfig) + } + def "PBS should not cache bids when targeting isn't specified"() { given: "Default BidRequest with cache" - def bidRequest = BidRequest.defaultBidRequest - bidRequest.enableCache() - bidRequest.ext.prebid.targeting = null + def bidRequest = BidRequest.defaultBidRequest.tap { + it.enableCache() + it.ext.prebid.targeting = null + } when: "PBS processes auction request" defaultPbsService.sendAuctionRequest(bidRequest) @@ -152,8 +355,8 @@ class CacheSpec extends BaseSpec { def "PBS shouldn't response with seatbid.bid.adm in response when ext.prebid.cache.bids.returnCreative=false"() { given: "Default BidRequest with cache" def bidRequest = BidRequest.defaultBidRequest.tap { - enableCache() - ext.prebid.cache.bids.returnCreative = false + it.enableCache() + it.ext.prebid.cache.bids.returnCreative = false } and: "Default basic bid with banner creative" @@ -174,8 +377,8 @@ class CacheSpec extends BaseSpec { def "PBS should response with seatbid.bid.adm in response when ext.prebid.cache.bids.returnCreative=true"() { given: "Default BidRequest with cache" def bidRequest = BidRequest.defaultBidRequest.tap { - enableCache() - ext.prebid.cache.bids.returnCreative = true + it.enableCache() + it.ext.prebid.cache.bids.returnCreative = true } and: "Default basic bid with banner creative" @@ -196,9 +399,9 @@ class CacheSpec extends BaseSpec { def "PBS shouldn't response with seatbid.bid.adm in response when ext.prebid.cache.vastXml.returnCreative=false and video request"() { given: "Default BidRequest with cache" def bidRequest = BidRequest.defaultBidRequest.tap { - imp[0] = Imp.getDefaultImpression(VIDEO) - enableCache() - ext.prebid.cache.vastXml.returnCreative = false + it.enableCache() + it.imp[0] = Imp.getDefaultImpression(VIDEO) + it.ext.prebid.cache.vastXml.returnCreative = false } and: "Default basic bid with banner creative" @@ -219,9 +422,9 @@ class CacheSpec extends BaseSpec { def "PBS should response with seatbid.bid.adm in response when ext.prebid.cache.vastXml.returnCreative=#returnCreative and imp.#mediaType"() { given: "Default BidRequest with cache" def bidRequest = BidRequest.defaultBidRequest.tap { - enableCache() - imp[0] = Imp.getDefaultImpression(mediaType) - ext.prebid.cache.vastXml.returnCreative = returnCreative + it.enableCache() + it.imp[0] = Imp.getDefaultImpression(mediaType) + it.ext.prebid.cache.vastXml.returnCreative = returnCreative } and: "Default basic bid with banner creative" @@ -246,7 +449,7 @@ class CacheSpec extends BaseSpec { def "PBS should update prebid_cache.creative_size.xml metric and adding tracking xml when xml creative contain #wrapper and impression are valid xml value"() { given: "Current value of metric prebid_cache.requests.ok" - def initialValue = getCurrentMetricValue(defaultPbsService, "prebid_cache.requests.ok") + def initialValue = getCurrentMetricValue(defaultPbsService, CACHE_REQUEST_OK_GLOBAL_METRIC) and: "Create and save enabled events config in account" def accountId = PBSUtils.randomNumber.toString() @@ -275,10 +478,10 @@ class CacheSpec extends BaseSpec { and: "prebid_cache.creative_size.xml metric should be updated" def metrics = defaultPbsService.sendCollectedMetricsRequest() - assert metrics["prebid_cache.requests.ok"] == initialValue + 1 + assert metrics[CACHE_REQUEST_OK_GLOBAL_METRIC] == initialValue + 1 and: "account..prebid_cache.creative_size.xml should be updated" - assert metrics["account.${accountId}.prebid_cache.requests.ok" as String] == 1 + assert metrics[CACHE_REQUEST_OK_ACCOUNT_METRIC.formatted(accountId) as String] == 1 where: wrapper | impression