-
Notifications
You must be signed in to change notification settings - Fork 247
New Module: LiveIntent Omni-channel Identity #3938
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
Changes from 18 commits
bf8c1fa
8429f34
459028d
c846d1c
8348d46
c0f6566
4f90c17
c6dc20e
a863d0a
62a4508
e45ccbc
3dbb46a
69fc413
c1ad22f
26288b7
4192fa8
1f1a7cf
c9659ad
3ed7ee8
bedb969
d94e06f
61ceeed
fa66d18
09450d2
b588f6a
1ae6da8
f597339
5f15bf1
d6dfcc7
53ca621
b61986a
e458a4c
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,46 @@ | ||
| # Overview | ||
|
|
||
| This module enriches bid requests with user EIDs. | ||
|
|
||
| The user EIDs to be enriched are configured per partner as part of the LiveIntent HIRO onboarding process. As part of this onboarding process, partners will also be provided with the `identity-resolution-endpoint` URL as well as with the `auth-token`. | ||
|
|
||
| `treatment-rate` is a value between 0.0 and 1.0 (including 0.0 and 1.0) and defines the percentage of requests for which identity enrichment should be performed. This value can be freely picked. We recommend a value between 0.9 and 0.95 | ||
|
|
||
| ## Configuration | ||
|
|
||
| To start using the LiveIntent Omni Channel Identity module you have to enable it and add configuration: | ||
|
|
||
| ```yaml | ||
| hooks: | ||
| liveintent-omni-channel-identity: | ||
| enabled: true | ||
| host-execution-plan: > | ||
| { | ||
| "endpoints": { | ||
| "/openrtb2/auction": { | ||
| "stages": { | ||
| "processed-auction-request": { | ||
| "groups": [ | ||
| { | ||
| "timeout": 100, | ||
| "hook-sequence": [ | ||
| { | ||
| "module-code": "liveintent-omni-channel-identity", | ||
| "hook-impl-code": "liveintent-omni-channel-identity-enrichment-hook" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| modules: | ||
| liveintent-omni-channel-identity: | ||
| request-timeout-ms: 2000 | ||
| identity-resolution-endpoint: "https://liveintent.com/idx" | ||
| auth-token: "secret-token" | ||
| treatment-rate: 0.9 | ||
| ``` | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <parent> | ||
| <groupId>org.prebid.server.hooks.modules</groupId> | ||
| <artifactId>all-modules</artifactId> | ||
| <version>3.26.0-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>live-intent-omni-channel-identity</artifactId> | ||
|
|
||
| <name>live-intent-omni-channel-identity</name> | ||
| <description>LiveIntent Omni-Channel Identity</description> | ||
|
|
||
| <dependencies> | ||
| </dependencies> | ||
| </project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.config; | ||
|
|
||
| import org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.config.ModuleConfig; | ||
| import org.prebid.server.hooks.modules.liveintent.omni.channel.identity.v1.LiveIntentOmniChannelIdentityModule; | ||
| import org.prebid.server.hooks.modules.liveintent.omni.channel.identity.v1.hooks.LiveIntentOmniChannelIdentityProcessedAuctionRequestHook; | ||
| import org.prebid.server.hooks.v1.Hook; | ||
| import org.prebid.server.hooks.v1.InvocationContext; | ||
| import org.prebid.server.hooks.v1.Module; | ||
| import org.prebid.server.json.JacksonMapper; | ||
| import org.prebid.server.vertx.httpclient.HttpClient; | ||
| import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; | ||
| import org.springframework.boot.context.properties.ConfigurationProperties; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| import java.util.Set; | ||
|
|
||
| @Configuration | ||
| @ConditionalOnProperty( | ||
| prefix = "hooks." + LiveIntentOmniChannelIdentityModule.CODE, name = "enabled", havingValue = "true" | ||
| ) | ||
| public class LiveIntentOmniChannelIdentityConfiguration { | ||
|
|
||
| @Bean | ||
| @ConfigurationProperties(prefix = "hooks.modules." + LiveIntentOmniChannelIdentityModule.CODE) | ||
| ModuleConfig moduleConfig() { | ||
| return new ModuleConfig(); | ||
| } | ||
|
|
||
| @Bean | ||
| Module liveIntentOmniChannelIdentityModule(ModuleConfig config, JacksonMapper mapper, HttpClient httpClient) { | ||
| final Set<? extends Hook<?, ? extends InvocationContext>> hooks = Set.of( | ||
| new LiveIntentOmniChannelIdentityProcessedAuctionRequestHook(config, mapper, httpClient)); | ||
| return new LiveIntentOmniChannelIdentityModule(hooks); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.iab.openrtb.request.Eid; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.extern.jackson.Jacksonized; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @Builder | ||
| @Data | ||
| @Jacksonized | ||
|
Collaborator
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. We don't use this annotation. We try to avoid experimental features of Lombok as muck as possible. I also believe that it will work without that annotation.
Author
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. Changed to using |
||
| public class IdResResponse { | ||
|
|
||
| @JsonProperty("eids") | ||
|
Collaborator
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. Useless annotation, please remove. Since you are using default mapper - it will deserialize by name of the field.
Author
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. Done in 3ed7ee8 |
||
| List<Eid> eids; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.config; | ||
|
|
||
| import lombok.Data; | ||
|
|
||
| @Data | ||
| public final class ModuleConfig { | ||
|
|
||
| long requestTimeoutMs; | ||
| String identityResolutionEndpoint; | ||
|
Collaborator
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. Please, add separation lines between all fields.
Author
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. Done in bedb969 |
||
| String authToken; | ||
| float treatmentRate; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.v1; | ||
|
|
||
| import org.prebid.server.hooks.v1.Hook; | ||
| import org.prebid.server.hooks.v1.InvocationContext; | ||
| import org.prebid.server.hooks.v1.Module; | ||
|
|
||
| import java.util.Collection; | ||
|
|
||
| public record LiveIntentOmniChannelIdentityModule( | ||
| Collection<? extends Hook<?, ? extends InvocationContext>> hooks) implements Module { | ||
|
|
||
| public static final String CODE = "liveintent-omni-channel-identity"; | ||
|
|
||
| @Override | ||
| public String code() { | ||
| return CODE; | ||
| } | ||
|
|
||
| @Override | ||
| public Collection<? extends Hook<?, ? extends InvocationContext>> hooks() { | ||
| return hooks; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.v1.hooks; | ||
|
|
||
| import com.iab.openrtb.request.BidRequest; | ||
| import com.iab.openrtb.request.Eid; | ||
| import com.iab.openrtb.request.User; | ||
| import io.vertx.core.Future; | ||
| import io.vertx.core.MultiMap; | ||
| import org.prebid.server.hooks.execution.v1.InvocationResultImpl; | ||
| import org.prebid.server.hooks.execution.v1.auction.AuctionRequestPayloadImpl; | ||
| import org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.IdResResponse; | ||
| import org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.config.ModuleConfig; | ||
| import org.prebid.server.hooks.v1.InvocationAction; | ||
| import org.prebid.server.hooks.v1.InvocationResult; | ||
| import org.prebid.server.hooks.v1.InvocationStatus; | ||
| import org.prebid.server.hooks.v1.auction.AuctionInvocationContext; | ||
| import org.prebid.server.hooks.v1.auction.AuctionRequestPayload; | ||
| import org.prebid.server.hooks.v1.auction.ProcessedAuctionRequestHook; | ||
| import org.prebid.server.json.JacksonMapper; | ||
| import org.prebid.server.log.Logger; | ||
| import org.prebid.server.log.LoggerFactory; | ||
| import org.prebid.server.util.HttpUtil; | ||
| import org.prebid.server.vertx.httpclient.HttpClient; | ||
| import org.prebid.server.vertx.httpclient.model.HttpClientResponse; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.Random; | ||
|
|
||
| public class LiveIntentOmniChannelIdentityProcessedAuctionRequestHook implements ProcessedAuctionRequestHook { | ||
|
|
||
| private static final Logger logger = | ||
| LoggerFactory.getLogger(LiveIntentOmniChannelIdentityProcessedAuctionRequestHook.class); | ||
| private static final String CODE = "liveintent-omni-channel-identity-enrichment-hook"; | ||
|
|
||
| private final ModuleConfig config; | ||
| private final JacksonMapper mapper; | ||
| private final HttpClient httpClient; | ||
| private final Random random; | ||
|
|
||
| public LiveIntentOmniChannelIdentityProcessedAuctionRequestHook( | ||
| ModuleConfig config, | ||
| JacksonMapper mapper, | ||
| HttpClient httpClient) { | ||
|
|
||
| this(config, mapper, httpClient, new Random()); | ||
|
Collaborator
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 one won't work. Default () -> ThreadLocalRandom.current().nextLong()
Author
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. Done in d94e06f |
||
| } | ||
|
|
||
| public LiveIntentOmniChannelIdentityProcessedAuctionRequestHook( | ||
| ModuleConfig config, | ||
| JacksonMapper mapper, | ||
| HttpClient httpClient, | ||
| Random random) { | ||
|
|
||
| this.config = Objects.requireNonNull(config); | ||
| this.mapper = Objects.requireNonNull(mapper); | ||
| this.httpClient = Objects.requireNonNull(httpClient); | ||
| this.random = random; | ||
|
Collaborator
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. Objects.requiereNonNull here
Contributor
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. Done in 5f15bf1 |
||
| } | ||
|
|
||
| @Override | ||
| public Future<InvocationResult<AuctionRequestPayload>> call( | ||
| AuctionRequestPayload auctionRequestPayload, | ||
| AuctionInvocationContext invocationContext | ||
| ) { | ||
|
Collaborator
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. Same here.
Contributor
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. Done in 5f15bf1 |
||
| if (random.nextFloat() < config.getTreatmentRate()) { | ||
| final Future<InvocationResult<AuctionRequestPayload>> update = requestEnrichment(auctionRequestPayload) | ||
| .map(resolutionResult -> | ||
| InvocationResultImpl.<AuctionRequestPayload>builder() | ||
| .status(InvocationStatus.success) | ||
| .action(InvocationAction.update) | ||
| .payloadUpdate(requestPayload -> updatedPayload(requestPayload, resolutionResult)) | ||
| .build() | ||
| ); | ||
|
Collaborator
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. Please, see our styling doc on how we style our code and apply changes accordingly.
Author
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. Done in 61ceeed |
||
|
|
||
| return update.onFailure(throwable -> logger.error("Failed enrichment:", throwable)); | ||
| } else { | ||
|
Collaborator
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. No need for
Contributor
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. Done in 5f15bf1 |
||
| return Future.succeededFuture( | ||
| InvocationResultImpl.<AuctionRequestPayload>builder() | ||
| .status(InvocationStatus.success) | ||
| .action(InvocationAction.no_action) | ||
| .build()); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String code() { | ||
|
Collaborator
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. Same here about method order.
Author
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. Done in fa66d18 |
||
| return CODE; | ||
| } | ||
|
|
||
| private AuctionRequestPayload updatedPayload(AuctionRequestPayload requestPayload, IdResResponse idResResponse) { | ||
| final BidRequest bidRequest = Optional.ofNullable( | ||
| requestPayload.bidRequest()).orElse(BidRequest.builder().build()); | ||
| final User user = Optional.ofNullable(bidRequest.getUser()).orElse(User.builder().build()); | ||
|
Collaborator
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. No need for building useless object and then rebuilding it right away. final User updatedUser = Optional.ofNullable(bidRequest.getUser())
.map(User::toBuilder)
.orElseGet(User::builder)
.eids(`your eids`)
.build();
Contributor
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. Done in 5f15bf1 |
||
|
|
||
| final List<Eid> allEids = new ArrayList<>(); | ||
| allEids.addAll(Optional.ofNullable(user.getEids()).orElse(Collections.emptyList())); | ||
| allEids.addAll(idResResponse.getEids()); | ||
|
Collaborator
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. ListUtils.union
Contributor
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. Done in 5f15bf1 |
||
|
|
||
| final User updatedUser = user.toBuilder().eids(allEids).build(); | ||
| final BidRequest updatedBidRequest = requestPayload.bidRequest().toBuilder().user(updatedUser).build(); | ||
|
|
||
| return AuctionRequestPayloadImpl.of(updatedBidRequest); | ||
| } | ||
|
|
||
| private Future<IdResResponse> requestEnrichment(AuctionRequestPayload auctionRequestPayload) { | ||
| final String bidRequestJson = mapper.encodeToString(auctionRequestPayload.bidRequest()); | ||
| return httpClient.post( | ||
| config.getIdentityResolutionEndpoint(), | ||
| headers(), | ||
| bidRequestJson, | ||
| config.getRequestTimeoutMs()) | ||
| .map(this::processResponse); | ||
| } | ||
|
|
||
| private IdResResponse processResponse(HttpClientResponse response) { | ||
| return mapper.decodeValue(response.getBody(), IdResResponse.class); | ||
| } | ||
|
|
||
| private MultiMap headers() { | ||
| return MultiMap.caseInsensitiveMultiMap() | ||
| .add(HttpUtil.AUTHORIZATION_HEADER, "Bearer " + config.getAuthToken()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.config; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.IdResResponse; | ||
| import org.prebid.server.json.JacksonMapper; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class IdResResponseTest { | ||
|
|
||
| private JacksonMapper jacksonMapper; | ||
|
|
||
| @BeforeEach | ||
| public void setUp() { | ||
| final ObjectMapper mapper = new ObjectMapper(); | ||
| jacksonMapper = new JacksonMapper(mapper); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldDecodeFromString() { | ||
| // given | ||
| final IdResResponse result = jacksonMapper.decodeValue( | ||
| "{\"eids\": [ { \"source\": \"liveintent.com\", " | ||
|
Collaborator
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. Set up data with proper Objects. We don't use raw json strings in unit tests.
Contributor
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. But that's exactly what is tested here: that the json string is parsed correctly.
Collaborator
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. What do you test here?) Jackson object mapper or your Data class? Remove this entirely, as this unit test is useless.
Contributor
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. The goal of the test is to ensure that the API response is deserialized correctly, and the data-class is mapping the API response properly |
||
| + "\"uids\": [ { \"atype\": 3, \"id\" : \"some_id\" } ] } ] }", | ||
| IdResResponse.class); | ||
| // when and then | ||
|
Collaborator
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. Please, add nice empty line before comment to separate test stages. Also, check for similar occurrences.
Author
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. Dine in 09450d2 |
||
| assertThat(result.getEids()).hasSize(1); | ||
| assertThat(result.getEids().getFirst().getSource()).isEqualTo("liveintent.com"); | ||
| assertThat(result.getEids().getFirst().getUids()).hasSize(1); | ||
| assertThat(result.getEids().getFirst().getUids().getFirst().getAtype()).isEqualTo(3); | ||
| assertThat(result.getEids().getFirst().getUids().getFirst().getId()).isEqualTo("some_id"); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package org.prebid.server.hooks.modules.liveintent.omni.channel.identity.model.config; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class ModuleConfigTest { | ||
|
|
||
| @Test | ||
| public void shouldReturnRequestTimeoutMs() { | ||
| final ModuleConfig moduleConfig = new ModuleConfig(); | ||
| moduleConfig.setRequestTimeoutMs(5); | ||
| assertThat(moduleConfig.getRequestTimeoutMs()).isEqualTo(5); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldReturnIdentityResolutionEndpoint() { | ||
| // given | ||
| final ModuleConfig moduleConfig = new ModuleConfig(); | ||
| moduleConfig.setIdentityResolutionEndpoint("https://test.com/idres"); | ||
| // when and then | ||
| assertThat(moduleConfig.getIdentityResolutionEndpoint()).isEqualTo("https://test.com/idres"); | ||
| } | ||
|
|
||
| @Test | ||
| public void shouldReturnAuthToken() { | ||
| // given | ||
| final ModuleConfig moduleConfig = new ModuleConfig(); | ||
| moduleConfig.setAuthToken("secret_token"); | ||
| // when and then | ||
| assertThat(moduleConfig.getAuthToken()).isEqualTo("secret_token"); | ||
| } | ||
| } |
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.
Please, change parantesis placement, look at our code style guidelines.
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.
Done in 5f15bf1