Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Greenbids: Add account level config for modules RTD, analytics #3596

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.apache.commons.lang3.StringUtils;
import org.prebid.server.analytics.reporter.greenbids.model.ExplorationResult;
import org.prebid.server.analytics.reporter.greenbids.model.Ortb2ImpExtResult;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.Partner;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.GreenbidsConfig;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.result.AnalyticsResult;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.result.GreenbidsInvocationResult;
import org.prebid.server.hooks.v1.InvocationAction;
Expand All @@ -23,12 +23,12 @@ public class GreenbidsInvocationService {
private static final int RANGE_16_BIT_INTEGER_DIVISION_BASIS = 0x10000;

public GreenbidsInvocationResult createGreenbidsInvocationResult(
Partner partner,
GreenbidsConfig greenbidsConfig,
BidRequest bidRequest,
Map<String, Map<String, Boolean>> impsBiddersFilterMap) {

final String greenbidsId = UUID.randomUUID().toString();
final boolean isExploration = isExploration(partner, greenbidsId);
final boolean isExploration = isExploration(greenbidsConfig, greenbidsId);

final BidRequest updatedBidRequest = isExploration
? bidRequest
Expand All @@ -49,10 +49,10 @@ public GreenbidsInvocationResult createGreenbidsInvocationResult(
return GreenbidsInvocationResult.of(updatedBidRequest, invocationAction, analyticsResult);
}

private Boolean isExploration(Partner partner, String greenbidsId) {
private Boolean isExploration(GreenbidsConfig greenbidsConfig, String greenbidsId) {
final int hashInt = Integer.parseInt(
greenbidsId.substring(greenbidsId.length() - 4), 16);
return hashInt < partner.getExplorationRate() * RANGE_16_BIT_INTEGER_DIVISION_BASIS;
return hashInt < greenbidsConfig.getExplorationRate() * RANGE_16_BIT_INTEGER_DIVISION_BASIS;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I can see it's never checked that properties of configs are required or can't be null, so this is a NPE potentially

check other properties as well and handle them if it's necessary

}

private List<Imp> updateImps(BidRequest bidRequest, Map<String, Map<String, Boolean>> impsBiddersFilterMap) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.prebid.server.hooks.modules.greenbids.real.time.data.core;

import io.vertx.core.Future;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.Partner;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.GreenbidsConfig;

import java.util.Objects;

Expand All @@ -18,14 +18,14 @@ public OnnxModelRunnerWithThresholds(
this.thresholdCache = Objects.requireNonNull(thresholdCache);
}

public Future<OnnxModelRunner> retrieveOnnxModelRunner(Partner partner) {
final String onnxModelPath = "models_pbuid=" + partner.getPbuid() + ".onnx";
return modelCache.get(onnxModelPath, partner.getPbuid());
public Future<OnnxModelRunner> retrieveOnnxModelRunner(GreenbidsConfig greenbidsConfig) {
final String onnxModelPath = "models_pbuid=" + greenbidsConfig.getPbuid() + ".onnx";
return modelCache.get(onnxModelPath, greenbidsConfig.getPbuid());
}

public Future<Double> retrieveThreshold(Partner partner) {
final String thresholdJsonPath = "thresholds_pbuid=" + partner.getPbuid() + ".json";
return thresholdCache.get(thresholdJsonPath, partner.getPbuid())
.map(partner::getThreshold);
public Future<Double> retrieveThreshold(GreenbidsConfig greenbidsConfig) {
final String thresholdJsonPath = "thresholds_pbuid=" + greenbidsConfig.getPbuid() + ".json";
return thresholdCache.get(thresholdJsonPath, greenbidsConfig.getPbuid())
.map(greenbidsConfig::getThreshold);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.util.stream.IntStream;

@Value(staticConstructor = "of")
public class Partner {
public class GreenbidsConfig {

String pbuid;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.prebid.server.hooks.modules.greenbids.real.time.data.core.GreenbidsInvocationService;
import org.prebid.server.hooks.modules.greenbids.real.time.data.core.OnnxModelRunner;
import org.prebid.server.hooks.modules.greenbids.real.time.data.core.OnnxModelRunnerWithThresholds;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.Partner;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.GreenbidsConfig;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.ThrottlingMessage;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.result.AnalyticsResult;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.result.GreenbidsInvocationResult;
Expand All @@ -35,6 +35,8 @@
import org.prebid.server.hooks.v1.auction.ProcessedAuctionRequestHook;
import org.prebid.server.proto.openrtb.ext.request.ExtRequest;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid;
import org.prebid.server.settings.model.Account;
import org.prebid.server.settings.model.AccountHooksConfiguration;

import java.util.Collections;
import java.util.List;
Expand All @@ -44,10 +46,10 @@

public class GreenbidsRealTimeDataProcessedAuctionRequestHook implements ProcessedAuctionRequestHook {

private static final String BID_REQUEST_ANALYTICS_EXTENSION_NAME = "greenbids-rtd";
private static final String CODE = "greenbids-real-time-data-processed-auction-request";
private static final String ACTIVITY = "greenbids-filter";
private static final String SUCCESS_STATUS = "success";
private static final String BID_REQUEST_ANALYTICS_EXTENSION_NAME = "greenbids-rtd";

private final ObjectMapper mapper;
private final FilterService filterService;
Expand Down Expand Up @@ -75,51 +77,57 @@ public Future<InvocationResult<AuctionRequestPayload>> call(

final AuctionContext auctionContext = invocationContext.auctionContext();
final BidRequest bidRequest = auctionContext.getBidRequest();
final Partner partner = parseBidRequestExt(bidRequest);

if (partner == null) {
return Future.succeededFuture(toInvocationResult(
bidRequest, null, InvocationAction.no_action));
}
final GreenbidsConfig greenbidsConfig = Optional.ofNullable(parseBidRequestExt(auctionContext))
.orElse(parseAccountConfig(auctionContext.getAccount()));
Comment on lines +80 to +81
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the execution shouldn't proceed of the config is null

yes, it shouldn't be the problem if the account config is required to invoke the module, but I guess it's still better to return the failed future in the null case


return Future.all(
onnxModelRunnerWithThresholds.retrieveOnnxModelRunner(partner),
onnxModelRunnerWithThresholds.retrieveThreshold(partner))
onnxModelRunnerWithThresholds.retrieveOnnxModelRunner(greenbidsConfig),
onnxModelRunnerWithThresholds.retrieveThreshold(greenbidsConfig))
.compose(compositeFuture -> toInvocationResult(
bidRequest,
partner,
greenbidsConfig,
compositeFuture.resultAt(0),
compositeFuture.resultAt(1)))
.recover(throwable -> Future.succeededFuture(toInvocationResult(
bidRequest, null, InvocationAction.no_action)));
}

private Partner parseBidRequestExt(BidRequest bidRequest) {
return Optional.ofNullable(bidRequest)
private GreenbidsConfig parseBidRequestExt(AuctionContext auctionContext) {
return Optional.ofNullable(auctionContext)
.map(AuctionContext::getBidRequest)
.map(BidRequest::getExt)
.map(ExtRequest::getPrebid)
.map(ExtRequestPrebid::getAnalytics)
.filter(this::isNotEmptyObjectNode)
.map(analytics -> (ObjectNode) analytics.get(BID_REQUEST_ANALYTICS_EXTENSION_NAME))
.map(this::toPartner)
.map(this::toGreenbidsConfig)
.orElse(null);
}

private boolean isNotEmptyObjectNode(JsonNode analytics) {
return analytics != null && analytics.isObject() && !analytics.isEmpty();
}

private Partner toPartner(ObjectNode adapterNode) {
private GreenbidsConfig parseAccountConfig(Account account) {
return Optional.ofNullable(account)
.map(Account::getHooks)
.map(AccountHooksConfiguration::getModules)
.map(modules -> modules.get(name()))
.map(this::toGreenbidsConfig)
.orElse(null);
}

private GreenbidsConfig toGreenbidsConfig(ObjectNode adapterNode) {
try {
return mapper.treeToValue(adapterNode, Partner.class);
return mapper.treeToValue(adapterNode, GreenbidsConfig.class);
} catch (JsonProcessingException e) {
return null;
}
}

private Future<InvocationResult<AuctionRequestPayload>> toInvocationResult(
BidRequest bidRequest,
Partner partner,
GreenbidsConfig greenbidsConfig,
OnnxModelRunner onnxModelRunner,
Double threshold) {

Expand All @@ -138,7 +146,7 @@ private Future<InvocationResult<AuctionRequestPayload>> toInvocationResult(
}

final GreenbidsInvocationResult greenbidsInvocationResult = greenbidsInvocationService
.createGreenbidsInvocationResult(partner, bidRequest, impsBiddersFilterMap);
.createGreenbidsInvocationResult(greenbidsConfig, bidRequest, impsBiddersFilterMap);

return Future.succeededFuture(toInvocationResult(
greenbidsInvocationResult.getUpdatedBidRequest(),
Expand Down Expand Up @@ -207,4 +215,8 @@ private ObjectNode toObjectNode(Map<String, Ortb2ImpExtResult> values) {
public String code() {
return CODE;
}

public String name() {
return "greenbids";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void extractThrottlingMessagesFromBidRequestShouldReturnValidThrottlingMe
.banner(banner)
.build();
final Device device = givenDevice(identity());
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device, null);
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device);

final CountryResponse countryResponse = mock(CountryResponse.class);

Expand Down Expand Up @@ -115,7 +115,7 @@ public void extractThrottlingMessagesFromBidRequestShouldReturnValidThrottlingMe
.banner(banner)
.build();
final Device device = givenDevice(identity(), "FRA");
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device, null);
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device);

final ZonedDateTime timestamp = ZonedDateTime.now(ZoneId.of("UTC"));
final Integer expectedHourBucket = timestamp.getHour();
Expand Down Expand Up @@ -152,7 +152,7 @@ public void extractThrottlingMessagesFromBidRequestShouldHandleMissingIp() {
.banner(banner)
.build();
final Device device = givenDeviceWithoutIp(identity());
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device, null);
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device);

final ZonedDateTime timestamp = ZonedDateTime.now(ZoneId.of("UTC"));
final Integer expectedHourBucket = timestamp.getHour();
Expand Down Expand Up @@ -188,7 +188,7 @@ public void extractThrottlingMessagesFromBidRequestShouldThrowPreBidExceptionWhe
.banner(banner)
.build();
final Device device = givenDevice(identity());
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device, null);
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device);

when(databaseReader.country(any(InetAddress.class))).thenThrow(new GeoIp2Exception("GeoIP failure"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
import org.prebid.server.analytics.reporter.greenbids.model.Ortb2ImpExtResult;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.Partner;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.data.GreenbidsConfig;
import org.prebid.server.hooks.modules.greenbids.real.time.data.model.result.GreenbidsInvocationResult;
import org.prebid.server.hooks.v1.InvocationAction;

Expand Down Expand Up @@ -45,13 +45,13 @@ public void createGreenbidsInvocationResultShouldReturnUpdateBidRequestWhenNotEx
.banner(banner)
.build();
final Device device = givenDevice(identity());
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device, null);
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device);
final Map<String, Map<String, Boolean>> impsBiddersFilterMap = givenImpsBiddersFilterMap();
final Partner partner = givenPartner(0.0);
final GreenbidsConfig greenbidsConfig = givenPartner(0.0);

// when
final GreenbidsInvocationResult result = target.createGreenbidsInvocationResult(
partner, bidRequest, impsBiddersFilterMap);
greenbidsConfig, bidRequest, impsBiddersFilterMap);

// then
final JsonNode updatedBidRequestExtPrebidBidders = result.getUpdatedBidRequest().getImp().getFirst().getExt()
Expand Down Expand Up @@ -82,13 +82,13 @@ public void createGreenbidsInvocationResultShouldReturnNoActionWhenExploration()
.banner(banner)
.build();
final Device device = givenDevice(identity());
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device, null);
final BidRequest bidRequest = givenBidRequest(request -> request, List.of(imp), device);
final Map<String, Map<String, Boolean>> impsBiddersFilterMap = givenImpsBiddersFilterMap();
final Partner partner = givenPartner(1.0);
final GreenbidsConfig greenbidsConfig = givenPartner(1.0);

// when
final GreenbidsInvocationResult result = target.createGreenbidsInvocationResult(
partner, bidRequest, impsBiddersFilterMap);
greenbidsConfig, bidRequest, impsBiddersFilterMap);

// then
final JsonNode updatedBidRequestExtPrebidBidders = result.getUpdatedBidRequest().getImp().getFirst().getExt()
Expand Down Expand Up @@ -120,7 +120,7 @@ private Map<String, Map<String, Boolean>> givenImpsBiddersFilterMap() {
return impsBiddersFilterMap;
}

private Partner givenPartner(Double explorationRate) {
return Partner.of("test-pbuid", 0.60, explorationRate);
private GreenbidsConfig givenPartner(Double explorationRate) {
return GreenbidsConfig.of("test-pbuid", 0.60, explorationRate);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.iab.openrtb.request.Site;
import org.prebid.server.json.ObjectMapperProvider;
import org.prebid.server.proto.openrtb.ext.request.ExtRequest;
import org.prebid.server.proto.openrtb.ext.request.ExtRequestPrebid;

import java.util.Collections;
import java.util.List;
Expand All @@ -26,15 +27,41 @@ private TestBidRequestProvider() { }
public static BidRequest givenBidRequest(
UnaryOperator<BidRequest.BidRequestBuilder> bidRequestCustomizer,
List<Imp> imps,
Device device,
ExtRequest extRequest) {
Device device) {

return bidRequestCustomizer.apply(BidRequest.builder()
.id("request")
.imp(imps)
.site(givenSite(site -> site))
.device(device)
.ext(extRequest)).build();
.device(device)).build();
}

public static BidRequest givenBidRequestWithExtension(
UnaryOperator<BidRequest.BidRequestBuilder> bidRequestCustomizer,
List<Imp> imps) {
final BidRequest.BidRequestBuilder bidRequestBuilder = BidRequest.builder()
.id("request")
.imp(imps)
.site(givenSite(site -> site))
.device(givenDevice(device -> device))
.ext(givenExtRequest());

return bidRequestCustomizer.apply(bidRequestBuilder).build();
}

public static ExtRequest givenExtRequest() {
final ObjectNode greenbidsNode = new ObjectMapper().createObjectNode();
greenbidsNode.put("pbuid", "leparisien");
greenbidsNode.put("greenbidsSampling", 1.0);

final ObjectNode analyticsNode = new ObjectMapper().createObjectNode();
analyticsNode.set("greenbids", greenbidsNode);

return ExtRequest.of(
ExtRequestPrebid
.builder()
.analytics(analyticsNode)
.build());
}

public static Site givenSite(UnaryOperator<Site.SiteBuilder> siteCustomizer) {
Expand Down
Loading
Loading