Skip to content

Commit a0806e8

Browse files
CTMBNaraCTMBNara
authored
Revert "Price Floor Logs Update (#3924)"
This reverts commit 1a55dc4.
1 parent 84cb276 commit a0806e8

16 files changed

Lines changed: 398 additions & 1183 deletions

‎src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java‎

Lines changed: 44 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
import org.prebid.server.log.ConditionalLogger;
2222
import org.prebid.server.log.Logger;
2323
import org.prebid.server.log.LoggerFactory;
24-
import org.prebid.server.metric.MetricName;
25-
import org.prebid.server.metric.Metrics;
2624
import org.prebid.server.proto.openrtb.ext.request.ExtImpPrebidFloors;
2725
import org.prebid.server.proto.openrtb.ext.request.ExtRequest;
2826
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid;
@@ -54,23 +52,17 @@ public class BasicPriceFloorProcessor implements PriceFloorProcessor {
5452

5553
private final PriceFloorFetcher floorFetcher;
5654
private final PriceFloorResolver floorResolver;
57-
private final Metrics metrics;
5855
private final JacksonMapper mapper;
59-
private final double logSamplingRate;
6056

6157
private final RandomWeightedEntrySupplier<PriceFloorModelGroup> modelPicker;
6258

6359
public BasicPriceFloorProcessor(PriceFloorFetcher floorFetcher,
6460
PriceFloorResolver floorResolver,
65-
Metrics metrics,
66-
JacksonMapper mapper,
67-
double logSamplingRate) {
61+
JacksonMapper mapper) {
6862

6963
this.floorFetcher = Objects.requireNonNull(floorFetcher);
7064
this.floorResolver = Objects.requireNonNull(floorResolver);
71-
this.metrics = Objects.requireNonNull(metrics);
7265
this.mapper = Objects.requireNonNull(mapper);
73-
this.logSamplingRate = logSamplingRate;
7466

7567
modelPicker = new RandomPositiveWeightedEntrySupplier<>(BasicPriceFloorProcessor::resolveModelGroupWeight);
7668
}
@@ -90,7 +82,7 @@ public BidRequest enrichWithPriceFloors(BidRequest bidRequest,
9082
return disableFloorsForRequest(bidRequest);
9183
}
9284

93-
final PriceFloorRules floors = resolveFloors(account, bidRequest, warnings);
85+
final PriceFloorRules floors = resolveFloors(account, bidRequest, errors);
9486
return updateBidRequestWithFloors(bidRequest, bidder, floors, errors, warnings);
9587
}
9688

@@ -130,13 +122,49 @@ private static PriceFloorRules extractRequestFloors(BidRequest bidRequest) {
130122
return ObjectUtil.getIfNotNull(prebid, ExtRequestPrebid::getFloors);
131123
}
132124

133-
private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, List<String> warnings) {
125+
private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, List<String> errors) {
134126
final PriceFloorRules requestFloors = extractRequestFloors(bidRequest);
135127

136128
final FetchResult fetchResult = floorFetcher.fetch(account);
137-
final FetchStatus fetchStatus = fetchResult.getFetchStatus();
129+
final FetchStatus fetchStatus = ObjectUtil.getIfNotNull(fetchResult, FetchResult::getFetchStatus);
138130

139-
final boolean isUsingDynamicDataAllowed = Optional.ofNullable(account.getAuction())
131+
if (fetchResult != null && fetchStatus == FetchStatus.success && shouldUseDynamicData(account, fetchResult)) {
132+
final PriceFloorRules mergedFloors = mergeFloors(requestFloors, fetchResult.getRulesData());
133+
return createFloorsFrom(mergedFloors, fetchStatus, PriceFloorLocation.fetch);
134+
}
135+
136+
if (requestFloors != null) {
137+
try {
138+
final Optional<AccountPriceFloorsConfig> priceFloorsConfig = Optional.of(account)
139+
.map(Account::getAuction)
140+
.map(AccountAuctionConfig::getPriceFloors);
141+
142+
final Long maxRules = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxRules)
143+
.orElse(null);
144+
final Long maxDimensions = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxSchemaDims)
145+
.orElse(null);
146+
147+
PriceFloorRulesValidator.validateRules(
148+
requestFloors,
149+
PriceFloorsConfigResolver.resolveMaxValue(maxRules),
150+
PriceFloorsConfigResolver.resolveMaxValue(maxDimensions));
151+
152+
return createFloorsFrom(requestFloors, fetchStatus, PriceFloorLocation.request);
153+
} catch (PreBidException e) {
154+
errors.add("Failed to parse price floors from request, with a reason: %s".formatted(e.getMessage()));
155+
conditionalLogger.error(
156+
"Failed to parse price floors from request with id: '%s', with a reason: %s"
157+
.formatted(bidRequest.getId(), e.getMessage()),
158+
0.01d);
159+
}
160+
}
161+
162+
return createFloorsFrom(null, fetchStatus, PriceFloorLocation.noData);
163+
}
164+
165+
private static boolean shouldUseDynamicData(Account account, FetchResult fetchResult) {
166+
final boolean isUsingDynamicDataAllowed = Optional.of(account)
167+
.map(Account::getAuction)
140168
.map(AccountAuctionConfig::getPriceFloors)
141169
.map(AccountPriceFloorsConfig::getUseDynamicData)
142170
.map(BooleanUtils::isNotFalse)
@@ -147,73 +175,12 @@ private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, Li
147175
.map(rate -> ThreadLocalRandom.current().nextInt(USE_FETCH_DATA_RATE_MAX) < rate)
148176
.orElse(true);
149177

150-
if (fetchStatus == FetchStatus.success && isUsingDynamicDataAllowed && shouldUseDynamicData) {
151-
final PriceFloorRules mergedFloors = mergeFloors(requestFloors, fetchResult.getRulesData());
152-
return createFloorsFrom(mergedFloors, fetchStatus, PriceFloorLocation.fetch);
153-
}
154-
155-
final String fetchErrorMessage = resolveFetchErrorMessage(fetchResult, isUsingDynamicDataAllowed);
156-
return requestFloors == null
157-
? noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), fetchErrorMessage, warnings)
158-
: getPriceFloorRules(bidRequest, account, requestFloors, fetchStatus, fetchErrorMessage, warnings);
159-
}
160-
161-
private static String resolveFetchErrorMessage(FetchResult fetchResult, boolean isUsingDynamicDataAllowed) {
162-
return switch (fetchResult.getFetchStatus()) {
163-
case inprogress -> null;
164-
case error, timeout, none -> fetchResult.getErrorMessage();
165-
case success -> isUsingDynamicDataAllowed ? null : "Using dynamic data is not allowed";
166-
};
167-
}
168-
169-
private PriceFloorRules noPriceFloorData(FetchStatus fetchStatus,
170-
String accountId,
171-
String requestId,
172-
String errorMessage,
173-
List<String> warnings) {
174-
175-
if (errorMessage != null) {
176-
warnings.add(errorMessage);
177-
conditionalLogger.error("No price floor data for account %s and request %s, reason: %s"
178-
.formatted(accountId, requestId, errorMessage), logSamplingRate);
179-
metrics.updateAlertsMetrics(MetricName.general);
180-
}
181-
182-
return createFloorsFrom(null, fetchStatus, PriceFloorLocation.noData);
178+
return isUsingDynamicDataAllowed && shouldUseDynamicData;
183179
}
184180

185-
private PriceFloorRules getPriceFloorRules(BidRequest bidRequest,
186-
Account account,
187-
PriceFloorRules requestFloors,
188-
FetchStatus fetchStatus,
189-
String fetchErrorMessage,
190-
List<String> warnings) {
191-
192-
try {
193-
final Optional<AccountPriceFloorsConfig> priceFloorsConfig = Optional.of(account.getAuction())
194-
.map(AccountAuctionConfig::getPriceFloors);
195-
196-
final Long maxRules = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxRules)
197-
.orElse(null);
198-
final Long maxDimensions = priceFloorsConfig.map(AccountPriceFloorsConfig::getMaxSchemaDims)
199-
.orElse(null);
200-
201-
PriceFloorRulesValidator.validateRules(
202-
requestFloors,
203-
PriceFloorsConfigResolver.resolveMaxValue(maxRules),
204-
PriceFloorsConfigResolver.resolveMaxValue(maxDimensions));
205-
206-
return createFloorsFrom(requestFloors, fetchStatus, PriceFloorLocation.request);
207-
} catch (PreBidException e) {
208-
final String errorMessage = fetchErrorMessage == null
209-
? null
210-
: "%s. Following parsing of request price floors is failed: %s"
211-
.formatted(fetchErrorMessage, e.getMessage());
212-
return noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), errorMessage, warnings);
213-
}
214-
}
181+
private PriceFloorRules mergeFloors(PriceFloorRules requestFloors,
182+
PriceFloorData providerRulesData) {
215183

216-
private PriceFloorRules mergeFloors(PriceFloorRules requestFloors, PriceFloorData providerRulesData) {
217184
final Price floorMinPrice = resolveFloorMinPrice(requestFloors);
218185

219186
return (requestFloors != null ? requestFloors.toBuilder() : PriceFloorRules.builder())

‎src/main/java/org/prebid/server/floors/PriceFloorFetcher.java‎

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,7 @@ public FetchResult fetch(Account account) {
9090
final AccountFetchContext accountFetchContext = fetchedData.get(account.getId());
9191

9292
return accountFetchContext != null
93-
? FetchResult.of(
94-
accountFetchContext.getRulesData(),
95-
accountFetchContext.getFetchStatus(),
96-
accountFetchContext.getErrorMessage())
93+
? FetchResult.of(accountFetchContext.getRulesData(), accountFetchContext.getFetchStatus())
9794
: fetchPriceFloorData(account);
9895
}
9996

@@ -102,19 +99,20 @@ private FetchResult fetchPriceFloorData(Account account) {
10299
final Boolean fetchEnabled = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getEnabled);
103100

104101
if (BooleanUtils.isFalse(fetchEnabled)) {
105-
return FetchResult.none("Fetching is disabled");
102+
return FetchResult.of(null, FetchStatus.none);
106103
}
107104

108105
final String accountId = account.getId();
109106
final String fetchUrl = ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getUrl);
110107
if (!isUrlValid(fetchUrl)) {
111-
return FetchResult.error("Malformed fetch.url '%s' passed".formatted(fetchUrl));
108+
logger.error("Malformed fetch.url: '%s', passed for account %s".formatted(fetchUrl, accountId));
109+
return FetchResult.of(null, FetchStatus.error);
112110
}
113111
if (!fetchInProgress.contains(accountId)) {
114112
fetchPriceFloorDataAsynchronous(fetchConfig, accountId);
115113
}
116114

117-
return FetchResult.inProgress();
115+
return FetchResult.of(null, FetchStatus.inprogress);
118116
}
119117

120118
private boolean isUrlValid(String url) {
@@ -150,8 +148,8 @@ private void fetchPriceFloorDataAsynchronous(AccountPriceFloorsFetchConfig fetch
150148

151149
fetchInProgress.add(accountId);
152150
httpClient.get(fetchUrl, timeout, resolveMaxFileSize(maxFetchFileSizeKb))
153-
.map(httpClientResponse -> parseFloorResponse(httpClientResponse, fetchConfig))
154-
.recover(throwable -> recoverFromFailedFetching(throwable, fetchUrl))
151+
.map(httpClientResponse -> parseFloorResponse(httpClientResponse, fetchConfig, accountId))
152+
.recover(throwable -> recoverFromFailedFetching(throwable, fetchUrl, accountId))
155153
.map(cacheInfo -> updateCache(cacheInfo, fetchConfig, accountId))
156154
.map(priceFloorData -> createPeriodicTimerForRulesFetch(priceFloorData, fetchConfig, accountId));
157155
}
@@ -161,38 +159,40 @@ private static long resolveMaxFileSize(Long maxSizeInKBytes) {
161159
}
162160

163161
private ResponseCacheInfo parseFloorResponse(HttpClientResponse httpClientResponse,
164-
AccountPriceFloorsFetchConfig fetchConfig) {
162+
AccountPriceFloorsFetchConfig fetchConfig,
163+
String accountId) {
165164

166165
final int statusCode = httpClientResponse.getStatusCode();
167166
if (statusCode != HttpStatus.SC_OK) {
168-
throw new PreBidException("Failed to request, provider respond with status %s".formatted(statusCode));
167+
throw new PreBidException("Failed to request for account %s, provider respond with status %s"
168+
.formatted(accountId, statusCode));
169169
}
170170
final String body = httpClientResponse.getBody();
171171

172172
if (StringUtils.isBlank(body)) {
173-
throw new PreBidException("Failed to parse price floor response, response body can not be empty");
173+
throw new PreBidException(
174+
"Failed to parse price floor response for account %s, response body can not be empty"
175+
.formatted(accountId));
174176
}
175177

176-
final PriceFloorData priceFloorData = parsePriceFloorData(body);
177-
178+
final PriceFloorData priceFloorData = parsePriceFloorData(body, accountId);
178179
PriceFloorRulesValidator.validateRulesData(
179180
priceFloorData,
180181
PriceFloorsConfigResolver.resolveMaxValue(fetchConfig.getMaxRules()),
181182
PriceFloorsConfigResolver.resolveMaxValue(fetchConfig.getMaxSchemaDims()));
182183

183184
return ResponseCacheInfo.of(priceFloorData,
184185
FetchStatus.success,
185-
null,
186186
cacheTtlFromResponse(httpClientResponse, fetchConfig.getUrl()));
187187
}
188188

189-
private PriceFloorData parsePriceFloorData(String body) {
189+
private PriceFloorData parsePriceFloorData(String body, String accountId) {
190190
final PriceFloorData priceFloorData;
191191
try {
192192
priceFloorData = mapper.decodeValue(body, PriceFloorData.class);
193193
} catch (DecodeException e) {
194-
throw new PreBidException(
195-
"Failed to parse price floor response, cause: %s".formatted(ExceptionUtils.getMessage(e)));
194+
throw new PreBidException("Failed to parse price floor response for account %s, cause: %s"
195+
.formatted(accountId, ExceptionUtils.getMessage(e)));
196196
}
197197
return priceFloorData;
198198
}
@@ -220,11 +220,8 @@ private PriceFloorData updateCache(ResponseCacheInfo cacheInfo,
220220
String accountId) {
221221

222222
final long maxAgeTimerId = createMaxAgeTimer(accountId, resolveCacheTtl(cacheInfo, fetchConfig));
223-
final AccountFetchContext fetchContext = AccountFetchContext.of(
224-
cacheInfo.getRulesData(),
225-
cacheInfo.getFetchStatus(),
226-
cacheInfo.getErrorMessage(),
227-
maxAgeTimerId);
223+
final AccountFetchContext fetchContext =
224+
AccountFetchContext.of(cacheInfo.getRulesData(), cacheInfo.getFetchStatus(), maxAgeTimerId);
228225

229226
if (cacheInfo.getFetchStatus() == FetchStatus.success || !fetchedData.containsKey(accountId)) {
230227
fetchedData.put(accountId, fetchContext);
@@ -270,27 +267,30 @@ private Long createMaxAgeTimer(String accountId, long cacheTtl) {
270267
return vertx.setTimer(TimeUnit.SECONDS.toMillis(effectiveCacheTtl), id -> fetchedData.remove(accountId));
271268
}
272269

273-
private Future<ResponseCacheInfo> recoverFromFailedFetching(Throwable throwable, String fetchUrl) {
270+
private Future<ResponseCacheInfo> recoverFromFailedFetching(Throwable throwable,
271+
String fetchUrl,
272+
String accountId) {
273+
274274
metrics.updatePriceFloorFetchMetric(MetricName.failure);
275275

276276
final FetchStatus fetchStatus;
277-
final String errorMessage;
278277
if (throwable instanceof TimeoutException || throwable instanceof ConnectTimeoutException) {
279278
fetchStatus = FetchStatus.timeout;
280-
errorMessage = "Fetch price floor request timeout for fetch.url '%s' exceeded.".formatted(fetchUrl);
279+
logger.error("Fetch price floor request timeout for fetch.url: '%s', account %s exceeded."
280+
.formatted(fetchUrl, accountId));
281281
} else {
282282
fetchStatus = FetchStatus.error;
283-
errorMessage = "Failed to fetch price floor from provider for fetch.url '%s', with a reason: %s"
284-
.formatted(fetchUrl, throwable.getMessage());
283+
logger.error(
284+
"Failed to fetch price floor from provider for fetch.url: '%s', account = %s with a reason : %s "
285+
.formatted(fetchUrl, accountId, throwable.getMessage()));
285286
}
286287

287-
return Future.succeededFuture(ResponseCacheInfo.withError(fetchStatus, errorMessage));
288+
return Future.succeededFuture(ResponseCacheInfo.withStatus(fetchStatus));
288289
}
289290

290291
private PriceFloorData createPeriodicTimerForRulesFetch(PriceFloorData priceFloorData,
291292
AccountPriceFloorsFetchConfig fetchConfig,
292293
String accountId) {
293-
294294
final long accountPeriodicTimeSec =
295295
ObjectUtil.getIfNotNull(fetchConfig, AccountPriceFloorsFetchConfig::getPeriodSec);
296296
final long periodicTimeSec =
@@ -318,8 +318,6 @@ private static class AccountFetchContext {
318318

319319
FetchStatus fetchStatus;
320320

321-
String errorMessage;
322-
323321
Long maxAgeTimerId;
324322
}
325323

@@ -330,12 +328,10 @@ private static class ResponseCacheInfo {
330328

331329
FetchStatus fetchStatus;
332330

333-
String errorMessage;
334-
335331
Long cacheTtl;
336332

337-
public static ResponseCacheInfo withError(FetchStatus status, String errorMessage) {
338-
return ResponseCacheInfo.of(null, status, errorMessage, null);
333+
public static ResponseCacheInfo withStatus(FetchStatus status) {
334+
return ResponseCacheInfo.of(null, status, null);
339335
}
340336
}
341337
}

‎src/main/java/org/prebid/server/floors/proto/FetchResult.java‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,4 @@ public class FetchResult {
99
PriceFloorData rulesData;
1010

1111
FetchStatus fetchStatus;
12-
13-
String errorMessage;
14-
15-
public static FetchResult none(String errorMessage) {
16-
return FetchResult.of(null, FetchStatus.none, errorMessage);
17-
}
18-
19-
public static FetchResult error(String errorMessage) {
20-
return FetchResult.of(null, FetchStatus.error, errorMessage);
21-
}
22-
23-
public static FetchResult inProgress() {
24-
return FetchResult.of(null, FetchStatus.inprogress, null);
25-
}
2612
}

‎src/main/java/org/prebid/server/spring/config/PriceFloorsConfiguration.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.prebid.server.metric.Metrics;
2121
import org.prebid.server.settings.ApplicationSettings;
2222
import org.prebid.server.vertx.httpclient.HttpClient;
23-
import org.springframework.beans.factory.annotation.Value;
2423
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
2524
import org.springframework.boot.context.properties.ConfigurationProperties;
2625
import org.springframework.context.annotation.Bean;
@@ -85,11 +84,9 @@ PriceFloorResolver noOpPriceFloorResolver() {
8584
@ConditionalOnProperty(prefix = "price-floors", name = "enabled", havingValue = "true")
8685
PriceFloorProcessor basicPriceFloorProcessor(PriceFloorFetcher floorFetcher,
8786
PriceFloorResolver floorResolver,
88-
Metrics metrics,
89-
JacksonMapper mapper,
90-
@Value("${logging.sampling-rate:0.01}") double logSamplingRate) {
87+
JacksonMapper mapper) {
9188

92-
return new BasicPriceFloorProcessor(floorFetcher, floorResolver, metrics, mapper, logSamplingRate);
89+
return new BasicPriceFloorProcessor(floorFetcher, floorResolver, mapper);
9390
}
9491

9592
@Bean

0 commit comments

Comments
 (0)