-
Notifications
You must be signed in to change notification settings - Fork 0
optable-targeting: Hid plus id5 Mobile In-app resolver params #3
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
base: optable-targeting-non-blocking-early-network-call
Are you sure you want to change the base?
Changes from all commits
aede90f
81c8810
a66497e
8902c11
073b1d5
65de1b6
bceaf21
41429ca
c59b63f
a54cecd
ea21f42
903aebb
9f5bef2
144d1e6
b8441d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package org.prebid.server.hooks.modules.optable.targeting.model; | ||
|
|
||
| import lombok.Value; | ||
|
|
||
| @Value(staticConstructor = "of") | ||
| public class App { | ||
|
|
||
| String bundle; | ||
|
|
||
| String ver; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,4 +23,8 @@ public class OptableAttributes { | |
| String userAgent; | ||
|
|
||
| Long timeout; | ||
|
|
||
| App app; | ||
|
|
||
| String id5Signature; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,4 +17,6 @@ public class ExtUserOptable extends FlexibleExtension { | |
| String zip; | ||
|
|
||
| String vid; | ||
|
|
||
| String id5Signature; | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.modules.optable.targeting.model.openrtb.TargetingResult; | ||||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.modules.optable.targeting.v1.core.AnalyticTagsResolver; | ||||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.modules.optable.targeting.v1.core.BidderRequestEnricher; | ||||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.modules.optable.targeting.v1.core.Id5Resolver; | ||||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.v1.InvocationAction; | ||||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.v1.InvocationResult; | ||||||||||||||||||||||||||||||||||||||||
| import org.prebid.server.hooks.v1.InvocationStatus; | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -61,6 +62,7 @@ private Future<InvocationResult<BidderRequestPayload>> enrichedPayload(Targeting | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if (hasData) { | ||||||||||||||||||||||||||||||||||||||||
| moduleContext.setTargeting(targetingResult.getAudience()); | ||||||||||||||||||||||||||||||||||||||||
| moduleContext.setId5Signature(Id5Resolver.resolveId5Signature(targetingResult)); | ||||||||||||||||||||||||||||||||||||||||
| moduleContext.setEnrichRequestStatus(EnrichmentStatus.success()); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
|
|
@@ -82,7 +84,7 @@ private Future<InvocationResult<BidderRequestPayload>> failedAction(ModuleContex | |||||||||||||||||||||||||||||||||||||||
| final Tags analyticsTags = AnalyticTagsResolver.toBidderEnrichRequestAnalyticTags( | ||||||||||||||||||||||||||||||||||||||||
| bidder, outcome, executionTime); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return noAction(moduleContext, analyticsTags); | ||||||||||||||||||||||||||||||||||||||||
| return noActionResponse(moduleContext, analyticsTags); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| private static boolean hasEnrichmentData(TargetingResult targetingResult) { | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -98,6 +100,27 @@ private static long calcExecutionTime(ModuleContext moduleContext) { | |||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| private Future<InvocationResult<BidderRequestPayload>> noAction(ModuleContext moduleContext, Tags analyticsTags) { | ||||||||||||||||||||||||||||||||||||||||
| final Future<TargetingResult> targetingCall = moduleContext.getOptableTargetingCall(); | ||||||||||||||||||||||||||||||||||||||||
| if (targetingCall != null) { | ||||||||||||||||||||||||||||||||||||||||
| return targetingCall.compose(targetingResult -> { | ||||||||||||||||||||||||||||||||||||||||
| moduleContext.setId5Signature(Id5Resolver.resolveId5Signature(targetingResult)); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return Future.succeededFuture( | ||||||||||||||||||||||||||||||||||||||||
| InvocationResultImpl.<BidderRequestPayload>builder() | ||||||||||||||||||||||||||||||||||||||||
| .status(InvocationStatus.success) | ||||||||||||||||||||||||||||||||||||||||
| .action(InvocationAction.no_action) | ||||||||||||||||||||||||||||||||||||||||
| .analyticsTags(analyticsTags) | ||||||||||||||||||||||||||||||||||||||||
| .moduleContext(moduleContext) | ||||||||||||||||||||||||||||||||||||||||
| .build()); | ||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+105
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Adding
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return noActionResponse(moduleContext, analyticsTags); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| private Future<InvocationResult<BidderRequestPayload>> noActionResponse(ModuleContext moduleContext, | ||||||||||||||||||||||||||||||||||||||||
| Tags analyticsTags) { | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return Future.succeededFuture( | ||||||||||||||||||||||||||||||||||||||||
| InvocationResultImpl.<BidderRequestPayload>builder() | ||||||||||||||||||||||||||||||||||||||||
| .status(InvocationStatus.success) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package org.prebid.server.hooks.modules.optable.targeting.v1.core; | ||
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import org.prebid.server.hooks.modules.optable.targeting.model.openrtb.ExtUserOptable; | ||
| import org.prebid.server.json.ObjectMapperProvider; | ||
| import org.prebid.server.log.ConditionalLogger; | ||
| import org.prebid.server.log.LoggerFactory; | ||
|
|
||
| public class ExtUserOptableResolver { | ||
|
|
||
| private static final ConditionalLogger conditionalLogger = | ||
| new ConditionalLogger(LoggerFactory.getLogger(ExtUserOptableResolver.class)); | ||
|
|
||
| private ExtUserOptableResolver() { | ||
| } | ||
|
|
||
| public static ExtUserOptable resolveExtUserOptable(JsonNode node, double logSamplingRate) { | ||
| try { | ||
| return ObjectMapperProvider.mapper().treeToValue(node, ExtUserOptable.class); | ||
| } catch (JsonProcessingException e) { | ||
| conditionalLogger.warn("Can't parse $.ext.user.Optable tag", logSamplingRate); | ||
| return null; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New issue introduced by the refactor: the signature is silently dropped in the raw+bidder plan when per-bidder enrichment is off.
This is now the only place the signature is set in that plan, but the hook returns early at lines 37-39 when
properties.isPerBidderEnrichmentEnabled()is false - and that is false for the default config, since it requiresenrichmentPercentage != 100 || !bidderEnrichmentPercentages.isEmpty()(OptableTargetingProperties.java:59-61). With both hooks in the execution plan,OptableTargetingProcessedAuctionRequestHook.java:76-78also returns without setting it.So with
enrichment-percentage: 100and no per-bidder map,moduleContext.getId5Signature()is null at response time and the signature never reaches the client. At073b1d543the response hook resolved it from the future'sTargetingResult, so it still got through. Behavior regression from the fix-up, and no test covers this combination.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed