-
Notifications
You must be signed in to change notification settings - Fork 284
Pipes through statsContext to evaluateLiquid directive function #3024
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 all commits
6e6149c
9ce22c5
856634a
5508da6
28b117c
084b860
8850513
1964c15
c2d8704
b29a712
349c8cb
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 |
---|---|---|
|
@@ -8,10 +8,12 @@ import validate from './validate' | |
import { arrify } from '../arrify' | ||
import { flattenObject } from './flatten' | ||
import { evaluateLiquid } from './liquid-directive' | ||
import { StatsContext } from '../destination-kit' | ||
import { isLiquidDirective } from './value-keys' | ||
|
||
export type InputData = { [key: string]: unknown } | ||
export type Features = { [key: string]: boolean } | ||
type Directive = (options: JSONValue, payload: JSONObject) => JSONLike | ||
type Directive = (options: JSONValue, payload: JSONObject, statsContext?: StatsContext | undefined) => JSONLike | ||
type StringDirective = (value: string, payload: JSONObject) => JSONLike | ||
|
||
interface Directives { | ||
|
@@ -42,7 +44,7 @@ function registerStringDirective(name: string, fn: StringDirective): void { | |
}) | ||
} | ||
|
||
function runDirective(obj: JSONObject, payload: JSONObject): JSONLike { | ||
function runDirective(obj: JSONObject, payload: JSONObject, statsContext?: StatsContext | undefined): JSONLike { | ||
const name = Object.keys(obj).find((key) => key.startsWith('@')) as string | ||
const directiveFn = directives[name] | ||
const value = obj[name] | ||
|
@@ -51,6 +53,10 @@ function runDirective(obj: JSONObject, payload: JSONObject): JSONLike { | |
throw new Error(`${name} is not a valid directive, got ${realTypeOf(directiveFn)}`) | ||
} | ||
|
||
if (name === '@liquid') { | ||
return directiveFn(value, payload, statsContext) | ||
} | ||
|
||
return directiveFn(value, payload) | ||
} | ||
|
||
|
@@ -326,8 +332,8 @@ registerDirective('@excludeWhenNull', (value, payload) => { | |
return cleanNulls(resolved) | ||
}) | ||
|
||
registerDirective('@liquid', (opts, payload) => { | ||
return evaluateLiquid(opts, payload) | ||
registerDirective('@liquid', (opts, payload, statsContext) => { | ||
return evaluateLiquid(opts, payload, statsContext) | ||
}) | ||
|
||
// Recursively remove all null values from an object | ||
|
@@ -381,23 +387,40 @@ function getMappingToProcess(mapping: JSONLikeObject): JSONLikeObject { | |
* @param payload - the input data to apply to the mapping directives | ||
* @todo support arrays or array directives? | ||
*/ | ||
function resolve(mapping: JSONLike, payload: JSONObject): JSONLike { | ||
function resolve(mapping: JSONLike, payload: JSONObject, statsContext?: StatsContext | undefined): JSONLike { | ||
if (!isObject(mapping) && !isArray(mapping)) { | ||
return mapping | ||
} | ||
|
||
if (isDirective(mapping)) { | ||
if (isLiquidDirective(mapping)) { | ||
// Only include stats, and therefore extra fieldKey tags, if the mapping is a liquid directive to save on costs | ||
return runDirective(mapping, payload, statsContext) | ||
} | ||
|
||
return runDirective(mapping, payload) | ||
} | ||
|
||
if (Array.isArray(mapping)) { | ||
return mapping.map((value) => resolve(value, payload)) | ||
return mapping.map((value) => resolve(value, payload, statsContext)) | ||
} | ||
|
||
const resolved: JSONLikeObject = {} | ||
|
||
for (const key of Object.keys(mapping)) { | ||
resolved[key] = resolve(mapping[key], payload) | ||
let originalTags: string[] = [] | ||
const statsTagsExist = statsContext?.tags !== undefined | ||
|
||
if (statsTagsExist) { | ||
originalTags = statsContext.tags | ||
statsContext.tags = [...statsContext.tags, `fieldKey:${key}`] | ||
} | ||
|
||
resolved[key] = resolve(mapping[key], payload, statsContext) | ||
|
||
if (statsTagsExist) { | ||
statsContext.tags = originalTags | ||
} | ||
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 manual tag management creates potential for state corruption if an exception occurs during resolve(). Consider using a try/finally block or creating a scoped statsContext copy to ensure tags are always restored. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
} | ||
|
||
return resolved | ||
|
@@ -409,7 +432,11 @@ function resolve(mapping: JSONLike, payload: JSONObject): JSONLike { | |
* @param mapping - the directives and raw values | ||
* @param data - the input data to apply to directives | ||
*/ | ||
function transform(mapping: JSONLikeObject, data: InputData | undefined = {}): JSONObject { | ||
function transform( | ||
mapping: JSONLikeObject, | ||
data: InputData | undefined = {}, | ||
statsContext?: StatsContext | undefined | ||
): JSONObject { | ||
const realType = realTypeOf(data) | ||
if (realType !== 'object') { | ||
throw new Error(`data must be an object, got ${realType}`) | ||
|
@@ -420,7 +447,7 @@ function transform(mapping: JSONLikeObject, data: InputData | undefined = {}): J | |
// throws if the mapping config is invalid | ||
validate(mappingToProcess) | ||
|
||
const resolved = resolve(mappingToProcess, data as JSONObject) | ||
const resolved = resolve(mappingToProcess, data as JSONObject, statsContext) | ||
const cleaned = removeUndefined(resolved) | ||
|
||
// Cast because we know there are no `undefined` values anymore | ||
|
@@ -432,7 +459,11 @@ function transform(mapping: JSONLikeObject, data: InputData | undefined = {}): J | |
* @param mapping - the directives and raw values | ||
* @param data - the array input data to apply to directives | ||
*/ | ||
function transformBatch(mapping: JSONLikeObject, data: Array<InputData> | undefined = []): JSONObject[] { | ||
function transformBatch( | ||
mapping: JSONLikeObject, | ||
data: Array<InputData> | undefined = [], | ||
statsContext?: StatsContext | undefined | ||
): JSONObject[] { | ||
const realType = realTypeOf(data) | ||
if (!isArray(data)) { | ||
throw new Error(`data must be an array, got ${realType}`) | ||
|
@@ -443,7 +474,7 @@ function transformBatch(mapping: JSONLikeObject, data: Array<InputData> | undefi | |
// throws if the mapping config is invalid | ||
validate(mappingToProcess) | ||
|
||
const resolved = data.map((d) => resolve(mappingToProcess, d as JSONObject)) | ||
const resolved = data.map((d) => resolve(mappingToProcess, d as JSONObject, statsContext)) | ||
|
||
// Cast because we know there are no `undefined` values after `removeUndefined` | ||
return removeUndefined(resolved) as JSONObject[] | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,4 +1,5 @@ | ||||||
import { Liquid } from 'liquidjs' | ||||||
import { StatsContext } from '../destination-kit' | ||||||
|
||||||
const liquidEngine = new Liquid({ | ||||||
renderLimit: 500, // 500 ms | ||||||
|
@@ -58,7 +59,7 @@ export function getLiquidKeys(liquidValue: string): string[] { | |||||
return liquidEngine.fullVariablesSync(liquidValue) | ||||||
} | ||||||
|
||||||
export function evaluateLiquid(liquidValue: any, event: any): string { | ||||||
export function evaluateLiquid(liquidValue: any, event: any, statsContext?: StatsContext | undefined): string { | ||||||
nick-Ag marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (typeof liquidValue !== 'string') { | ||||||
// type checking of @liquid directive is done in validate.ts as well | ||||||
throw new Error('liquid template value must be a string') | ||||||
|
@@ -72,7 +73,22 @@ export function evaluateLiquid(liquidValue: any, event: any): string { | |||||
throw new Error('liquid template values are limited to 1000 characters') | ||||||
} | ||||||
|
||||||
const res = liquidEngine.parseAndRenderSync(liquidValue, event) | ||||||
let res: string | ||||||
const start = Date.now() | ||||||
let status: 'success' | 'fail' = 'success' | ||||||
|
||||||
try { | ||||||
res = liquidEngine.parseAndRenderSync(liquidValue, event) | ||||||
} catch (e) { | ||||||
status = 'fail' | ||||||
throw e | ||||||
} finally { | ||||||
const duration = Date.now() - start | ||||||
statsContext?.statsClient?.histogram('liquid.template.evaluation_ms', duration, [ | ||||||
...statsContext.tags, | ||||||
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 will throw a runtime error if
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
`result:${status}` | ||||||
]) | ||||||
} | ||||||
|
||||||
if (typeof res !== 'string') { | ||||||
return 'error' | ||||||
|
Uh oh!
There was an error while loading. Please reload this page.