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

feat: enrich with geo location data during destination transformation #2803

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
5bdb63b
feat: geo location enrichment in destination transformations
Nov 6, 2023
0f44408
chore: add unit-tests for geoLocation logic
Nov 7, 2023
4bcc8db
chore: provide flexibility for an optional rudderMessage schema
Nov 7, 2023
0225128
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 7, 2023
d789f0e
feat: add tests for unit-testing enrichment logic
Nov 7, 2023
d3d6b72
fix: address and context.geo not present case
Nov 7, 2023
2249441
fix: logic to include geolocation info in first found key
Nov 7, 2023
6dd5c8b
fix: add unit-tests for getting addressKey
Nov 7, 2023
53fdb76
fix: remove unnecessary operation for getKeyAndValueFromMessage
Nov 7, 2023
8fb7668
chore: remove unnecessary logic in getKeyAndValueFromMessage
Nov 8, 2023
c2363d0
feat: add geo location middleware to destination transformation endpo…
Nov 8, 2023
5bff71f
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 8, 2023
5bdc2c6
fix: addressing issues
Nov 9, 2023
a825926
fix: name of the returned object & name of the function
Nov 9, 2023
b782b90
fix: addressing minor issues related to naming, used lodash.isNil ins…
Nov 9, 2023
687f2fa
fix: correcting test-case
Nov 9, 2023
e4c6bb2
fix: cloneDeep for all, add test-cases to reflect same referencing & …
Nov 9, 2023
2c44999
fix: address comments in test-cases
Nov 9, 2023
ee08e4b
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 9, 2023
8928020
fix: unnecessary test-case data for enricher unit-tests
Nov 10, 2023
25fb0d7
fix: addressing comments - 2
Nov 10, 2023
9c9c9f3
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 10, 2023
5ef41f5
fix: update the default value to be returned to be undefined
Nov 10, 2023
58fcdce
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 10, 2023
a61fb53
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 13, 2023
67aed69
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 13, 2023
3365332
fix: addressing comments - 3
Nov 14, 2023
67f9217
fix: expanded type name & extra optional chaining removal
Nov 14, 2023
a2f0747
Merge branch 'develop' of github.com:rudderlabs/rudder-transformer in…
Nov 14, 2023
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
54 changes: 54 additions & 0 deletions src/helpers/geoLocation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import cloneDeep from 'lodash/cloneDeep';
import isEmpty from 'lodash/isEmpty';
import set from 'set-value';
import { FixMe } from '../util/types';
abhimanyubabbar marked this conversation as resolved.
Show resolved Hide resolved
import { getFirstMatchingKeyAndValue } from '../v0/util';
import * as GenericFieldMappingJson from '../v0/util/data/GenericFieldMapping.json';

export default class GeoLocationHelper {
sanpj2292 marked this conversation as resolved.
Show resolved Hide resolved
public static getAddressKeyAndValue(message: Record<string, FixMe>): {
key: string;
value: Record<string, FixMe>;
} {
const { value, key: foundKey } = getFirstMatchingKeyAndValue(
message,
GenericFieldMappingJson.address,
);
const { key: traitsKey } = getFirstMatchingKeyAndValue(message, GenericFieldMappingJson.traits);
const addressKey =
foundKey || (traitsKey ? `${traitsKey}.address` : GenericFieldMappingJson.address[0]);
return { key: addressKey, value };
}

public static enrichMessageAddressWithGeoData(
message: Record<string, FixMe>,
): Record<string, FixMe> {
let msg = message;
if (isEmpty(msg?.context?.geo)) {
// geo-location data was not sent
return {};
}
const { value: address, key: addressKey } = GeoLocationHelper.getAddressKeyAndValue(msg);
const addressFieldToGeoFieldMap = {
city: 'city',
country: 'country',
postalCode: 'postal',
state: 'region',
};

const mappedAddress = Object.entries(addressFieldToGeoFieldMap).reduce(
(agg, [addressFieldKey, geoFieldKey]) => {
if (!address?.[addressFieldKey] && msg?.context?.geo?.[geoFieldKey]) {
koladilip marked this conversation as resolved.
Show resolved Hide resolved
return { [addressFieldKey]: msg.context.geo[geoFieldKey], ...agg };
}
return agg;
},
{},
);
if (!isEmpty(address) || !isEmpty(mappedAddress)) {
msg = cloneDeep(message || {});
sanpj2292 marked this conversation as resolved.
Show resolved Hide resolved
set(msg, addressKey, { ...address, ...mappedAddress });
}
return msg;
}
}
54 changes: 54 additions & 0 deletions src/middlewares/enricher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Context, Next } from 'koa';
import {
ProcessorTransformationRequest,
RouterTransformationRequest,
RouterTransformationRequestData,
} from '../types';
import GeoLocationHelper from '../helpers/geoLocation';

export type DTRequest = RouterTransformationRequest | ProcessorTransformationRequest[];
koladilip marked this conversation as resolved.
Show resolved Hide resolved

export default class GeoEnricher {
private static enrichWithGeoInfo(
sanpj2292 marked this conversation as resolved.
Show resolved Hide resolved
data: RouterTransformationRequestData[],
): RouterTransformationRequestData[] {
return data.map((inputEvent) => {
const geoEnrichedMessage = GeoLocationHelper.enrichMessageAddressWithGeoData(
inputEvent.message,
);
return {
...inputEvent,
message: {
...inputEvent.message,
...geoEnrichedMessage,
},
};
});
}

public static async enrich(ctx: Context, next: Next) {
const transformationRequest = ctx.request.body;
let transformationReq: DTRequest;
let reqBody: unknown;
const isRouterTransform = Array.isArray(
(transformationRequest as RouterTransformationRequest)?.input,
);
if (isRouterTransform) {
// Router or batch transformation request
transformationReq = transformationRequest as RouterTransformationRequest;
const enrichedEvents: RouterTransformationRequestData[] = GeoEnricher.enrichWithGeoInfo(
transformationReq.input,
);
reqBody = {
input: enrichedEvents,
destType: transformationReq.destType,
};
} else {
// Processor transformation
transformationReq = transformationRequest as ProcessorTransformationRequest[];
reqBody = GeoEnricher.enrichWithGeoInfo(transformationReq);
}
ctx.request.body = reqBody;
await next();
}
}
4 changes: 4 additions & 0 deletions src/routes/destination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import DestinationController from '../controllers/destination';
import RegulationController from '../controllers/regulation';
import FeatureFlagController from '../middlewares/featureFlag';
import RouteActivationController from '../middlewares/routeActivation';
import GeoEnricher from '../middlewares/enricher';

const router = new Router();

Expand All @@ -11,20 +12,23 @@ router.post(
RouteActivationController.isDestinationRouteActive,
RouteActivationController.destinationProcFilter,
FeatureFlagController.handle,
GeoEnricher.enrich,
DestinationController.destinationTransformAtProcessor,
);
router.post(
'/routerTransform',
RouteActivationController.isDestinationRouteActive,
RouteActivationController.destinationRtFilter,
FeatureFlagController.handle,
GeoEnricher.enrich,
DestinationController.destinationTransformAtRouter,
);
router.post(
'/batch',
RouteActivationController.isDestinationRouteActive,
RouteActivationController.destinationBatchFilter,
FeatureFlagController.handle,
GeoEnricher.enrich,
DestinationController.batchProcess,
);

Expand Down
40 changes: 32 additions & 8 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,44 @@ type UserTransformationLibrary = {
VersionID: string;
};

type ProcessorTransformationRequest = {
request?: object;
message: object;
metadata: Metadata;
destination: Destination;
libraries: UserTransformationLibrary[];
};
type RudderTimestamp = Date | string;
type RudderMessageType =
| 'identify'
| 'track'
| 'page'
| 'screen'
| 'group'
| 'alias'
| 'audiencelist';

type RudderMessageSchema = Partial<{
Copy link
Collaborator

Choose a reason for hiding this comment

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

there's RudderMessage too -

type RudderMessage = {

Do we need both of them ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used RudderMessage itself but there were too many changes that were needed in order for the tests to pass & compile. So, kind of yes, we need both of them.
[Backlog] We have to plan to get rid of both of these types & stick with only one type.

userId: string;
anonymousId: string;
type: RudderMessageType;
channel: string;
context: object;
originalTimestamp: RudderTimestamp;
sentAt: RudderTimestamp;
timestamp: RudderTimestamp;
event: string;
integrations: object;
messageId: string;
properties: object;
traits: object;
[key: string]: FixMe;
}>;

type RouterTransformationRequestData = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is low level struct used in both process & routerTransform flow, so shouldn't the name be neutral ?

Suggested change
type RouterTransformationRequestData = {
type EventRequestData = {

Copy link
Contributor Author

@sanpj2292 sanpj2292 Nov 9, 2023

Choose a reason for hiding this comment

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

Basically it was named as RouterTransformationRequestData previously but since the elements in the type were common, I used it as a low-level struct.

Tried to change name, there are many files which are using this type. I feel this is a little bigger change & out of scope of this task. May be we can handle this in a backlog item and make a change independent of this task. But yeah, let me know if we are ok with making this change as part of this PR.

request?: object;
message: object;
message: RudderMessageSchema;
metadata: Metadata;
destination: Destination;
};

type ProcessorTransformationRequest = {
libraries: UserTransformationLibrary[];
} & RouterTransformationRequestData;

type RouterTransformationRequest = {
input: RouterTransformationRequestData[];
destType: string;
Expand Down Expand Up @@ -275,4 +298,5 @@ export {
ComparatorInput,
SourceInput,
Source,
RudderMessageSchema,
};
3 changes: 2 additions & 1 deletion src/v0/destinations/hs/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,8 @@ const splitEventsForCreateUpdate = async (inputs, destination) => {
const { destinationExternalId } = getDestinationExternalIDInfoForRetl(message, DESTINATION);

const filteredInfo = updateHubspotIds.filter(
(update) => update.property.toString().toLowerCase() === destinationExternalId.toString().toLowerCase(),
(update) =>
update.property.toString().toLowerCase() === destinationExternalId.toString().toLowerCase(),
);

if (filteredInfo.length > 0) {
Expand Down
5 changes: 4 additions & 1 deletion src/v0/sources/shopify/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ const getDataFromRedis = async (key, metricMetadata) => {
...metricMetadata,
});
const redisData = await RedisDB.getVal(key);
if (redisData === null || (typeof redisData === "object" && Object.keys(redisData).length === 0)) {
if (
redisData === null ||
(typeof redisData === 'object' && Object.keys(redisData).length === 0)
) {
stats.increment('shopify_redis_no_val', {
...metricMetadata,
});
Expand Down
28 changes: 28 additions & 0 deletions src/v0/util/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,33 @@ const getValueFromMessage = (message, sourceKeys) => {
return null;
};

const getFirstMatchingKeyAndValue = (message, sourceKeys) => {
let srcKeys = sourceKeys;
if (
typeof message !== 'object' ||
(!Array.isArray(sourceKeys) && typeof sourceKeys !== 'string')
) {
// sourceKeys => boolean, number, object, function etc,.
// message => anything other than object,.
return { key: '' };
}
if (typeof sourceKeys === 'string') {
srcKeys = [sourceKeys];
}
// got the possible sourceKeys
// eslint-disable-next-line no-restricted-syntax
for (const sourceKey of srcKeys) {
const val = get(message, sourceKey);
if (!lodash.isNil(val)) {
// return only if the value is valid.
// else look for next possible source in precedence
return { value: val, key: sourceKey };
}
}

return { value: null, key: '' };
abhimanyubabbar marked this conversation as resolved.
Show resolved Hide resolved
};

// get a field value from message.
// if sourceFromGenericMap is true get its value from GenericFieldMapping.json and use it as sourceKey
// else use sourceKey from `data/message.json` for actual field precedence
Expand Down Expand Up @@ -2192,4 +2219,5 @@ module.exports = {
isValidInteger,
isNewStatusCodesAccepted,
IsGzipSupported,
getFirstMatchingKeyAndValue,
};
2 changes: 1 addition & 1 deletion test/apitests/data_scenarios/source/v1/pipedream.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@
}
}
]
}
}
Loading
Loading