Skip to content

Fix Price Floors Logs #3946

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.

Sign up for GitHub

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

Closed
AntoxaAntoxic wants to merge 1 commit into from
Closed

Fix Price Floors Logs #3946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 34 additions & 35 deletions src/main/java/org/prebid/server/floors/BasicPriceFloorProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,41 +152,17 @@ private PriceFloorRules resolveFloors(Account account, BidRequest bidRequest, Li
return createFloorsFrom(mergedFloors, fetchStatus, PriceFloorLocation.fetch);
}

final String fetchErrorMessage = resolveFetchErrorMessage(fetchResult, isUsingDynamicDataAllowed);
return requestFloors == null
? noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), fetchErrorMessage, warnings)
: getPriceFloorRules(bidRequest, account, requestFloors, fetchStatus, fetchErrorMessage, warnings);
}

private static String resolveFetchErrorMessage(FetchResult fetchResult, boolean isUsingDynamicDataAllowed) {
return switch (fetchResult.getFetchStatus()) {
case inprogress -> null;
case error, timeout, none -> fetchResult.getErrorMessage();
case success -> isUsingDynamicDataAllowed ? null : "Using dynamic data is not allowed";
};
}

private PriceFloorRules noPriceFloorData(FetchStatus fetchStatus,
String accountId,
String requestId,
String errorMessage,
List<String> warnings) {

if (errorMessage != null) {
warnings.add(errorMessage);
conditionalLogger.error("No price floor data for account %s and request %s, reason: %s"
.formatted(accountId, requestId, errorMessage), logSamplingRate);
metrics.updateAlertsMetrics(MetricName.general);
}

return createFloorsFrom(null, fetchStatus, PriceFloorLocation.noData);
return requestFloors == null || BooleanUtils.isNotTrue(requestFloors.getEnabled())
? createFloorsFrom(null, fetchStatus, PriceFloorLocation.noData)
: getPriceFloorRules(
bidRequest, account, requestFloors, fetchResult, isUsingDynamicDataAllowed, warnings);
}

private PriceFloorRules getPriceFloorRules(BidRequest bidRequest,
Account account,
PriceFloorRules requestFloors,
FetchStatus fetchStatus,
String fetchErrorMessage,
FetchResult fetchResult,
boolean isUsingDynamicDataAllowed,
List<String> warnings) {

try {
Expand All @@ -203,13 +179,36 @@ private PriceFloorRules getPriceFloorRules(BidRequest bidRequest,
PriceFloorsConfigResolver.resolveMaxValue(maxRules),
PriceFloorsConfigResolver.resolveMaxValue(maxDimensions));

return createFloorsFrom(requestFloors, fetchStatus, PriceFloorLocation.request);
return createFloorsFrom(requestFloors, fetchResult.getFetchStatus(), PriceFloorLocation.request);
} catch (PreBidException e) {
final String errorMessage = fetchErrorMessage == null
logErrorMessage(fetchResult, isUsingDynamicDataAllowed, e, account.getId(), bidRequest.getId(), warnings);
return createFloorsFrom(null, fetchResult.getFetchStatus(), PriceFloorLocation.noData);
}
}

private void logErrorMessage(FetchResult fetchResult,
boolean isUsingDynamicDataAllowed,
PreBidException requestFloorsValidationException,
String accountId,
String requestId,
List<String> warnings) {

final String validationErrorMessage = requestFloorsValidationException.getMessage();
final String errorMessage = switch (fetchResult.getFetchStatus()) {
case inprogress -> null;
case error, timeout, none -> "%s. Following parsing of request price floors is failed: %s"
.formatted(fetchResult.getErrorMessage(), validationErrorMessage);
case success -> isUsingDynamicDataAllowed
? null
: "%s. Following parsing of request price floors is failed: %s"
.formatted(fetchErrorMessage, e.getMessage());
return noPriceFloorData(fetchStatus, account.getId(), bidRequest.getId(), errorMessage, warnings);
: "Using dynamic data is not allowed. Following parsing of request price floors is failed: %s"
.formatted(validationErrorMessage);
};

if (errorMessage != null) {
warnings.add(errorMessage);
conditionalLogger.error("No price floor data for account %s and request %s, reason: %s"
.formatted(accountId, requestId, errorMessage), logSamplingRate);
metrics.updateAlertsMetrics(MetricName.general);
}
}

Expand Down
17 changes: 9 additions & 8 deletions src/test/java/org/prebid/server/floors/BasicPriceFloorProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ public void shouldNotUseFloorsFromProviderIfUseDynamicDataIsFalse() {
.skipped(false)
.location(PriceFloorLocation.noData)
.build());
verify(metrics).updateAlertsMetrics(MetricName.general);
assertThat(warnings).containsExactly("Using dynamic data is not allowed");
verifyNoInteractions(metrics);
assertThat(warnings).isEmpty();
}

@Test
Expand Down Expand Up @@ -366,8 +366,8 @@ public void shouldNotUseFloorsWhenProviderFetchingIsDisabled() {
.skipped(false)
.location(PriceFloorLocation.noData)
.build());
verify(metrics).updateAlertsMetrics(MetricName.general);
assertThat(warnings).containsExactly("errorMessage");
verifyNoInteractions(metrics);
assertThat(warnings).isEmpty();
}

@Test
Expand Down Expand Up @@ -417,8 +417,8 @@ public void shouldNotUseFloorsWhenProviderFetchingIsFailed() {
.skipped(false)
.location(PriceFloorLocation.noData)
.build());
verify(metrics).updateAlertsMetrics(MetricName.general);
assertThat(warnings).containsExactly("errorMessage");
verifyNoInteractions(metrics);
assertThat(warnings).isEmpty();
}

@Test
Expand Down Expand Up @@ -469,8 +469,8 @@ public void shouldNotUseFloorsWhenProviderFetchingIsFailedWithTimeout() {
.skipped(false)
.location(PriceFloorLocation.noData)
.build());
verify(metrics).updateAlertsMetrics(MetricName.general);
assertThat(warnings).containsExactly("errorMessage");
verifyNoInteractions(metrics);
assertThat(warnings).isEmpty();
}

@Test
Expand Down Expand Up @@ -1086,6 +1086,7 @@ private static PriceFloorRules givenFloors(
UnaryOperator<PriceFloorRules.PriceFloorRulesBuilder> floorsCustomizer) {

return floorsCustomizer.apply(PriceFloorRules.builder()
.enabled(true)
.data(PriceFloorData.builder()
.modelGroups(singletonList(PriceFloorModelGroup.builder()
.value("someKey", BigDecimal.ONE)
Expand Down
Loading