-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Random sampling feature #143221
[Lens] Random sampling feature #143221
Changes from 17 commits
be740b6
dc16774
b45c6de
1448a22
7d05809
c341c02
284e729
1360949
234e4a8
f7b9dd5
862931b
5eb26e0
54c8fc8
3039c5d
8966788
a627ba0
8131b5e
281b8c9
13c8078
4f63f67
f9e1bb9
bbc0fd2
f65125d
165fe55
60a4580
e71845e
c4a2b12
45eb2f9
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 |
---|---|---|
|
@@ -26,6 +26,7 @@ import { AggTypesDependencies, GetConfigFn, getUserTimeZone } from '../..'; | |
import { getTime, calculateBounds } from '../..'; | ||
import type { IBucketAggConfig } from './buckets'; | ||
import { insertTimeShiftSplit, mergeTimeShifts } from './utils/time_splits'; | ||
import { createSamplerAgg, isSamplingEnabled } from './utils/sampler'; | ||
|
||
function removeParentAggs(obj: any) { | ||
for (const prop in obj) { | ||
|
@@ -55,6 +56,8 @@ export interface AggConfigsOptions { | |
hierarchical?: boolean; | ||
aggExecutionContext?: AggTypesDependencies['aggExecutionContext']; | ||
partialRows?: boolean; | ||
probability?: number; | ||
samplerSeed?: number; | ||
} | ||
|
||
export type CreateAggConfigParams = Assign<AggConfigSerialized, { type: string | IAggType }>; | ||
|
@@ -107,6 +110,17 @@ export class AggConfigs { | |
return this.opts.partialRows ?? false; | ||
} | ||
|
||
public get samplerConfig() { | ||
return { probability: this.opts.probability ?? 1, seed: this.opts.samplerSeed }; | ||
} | ||
|
||
isSamplingEnabled() { | ||
return ( | ||
isSamplingEnabled(this.opts.probability) && | ||
this.getRequestAggs().filter((agg) => !agg.type.hasNoDsl).length > 0 | ||
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. Trying to follow...
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. That condition is set to detect a specific case when no sub-agg is available for the sampling, therefore it makes no sense to have it. That's the case of |
||
); | ||
} | ||
|
||
setTimeFields(timeFields: string[] | undefined) { | ||
this.timeFields = timeFields; | ||
} | ||
|
@@ -225,7 +239,7 @@ export class AggConfigs { | |
} | ||
|
||
toDsl(): Record<string, any> { | ||
const dslTopLvl = {}; | ||
const dslTopLvl: Record<string, any> = {}; | ||
let dslLvlCursor: Record<string, any>; | ||
let nestedMetrics: Array<{ config: AggConfig; dsl: Record<string, any> }> | []; | ||
|
||
|
@@ -254,10 +268,21 @@ export class AggConfigs { | |
(config) => 'splitForTimeShift' in config.type && config.type.splitForTimeShift(config, this) | ||
); | ||
|
||
if (this.isSamplingEnabled()) { | ||
dslTopLvl.sampling = createSamplerAgg({ | ||
probability: this.opts.probability ?? 1, | ||
seed: this.opts.samplerSeed, | ||
}); | ||
} | ||
|
||
requestAggs.forEach((config: AggConfig, i: number, list) => { | ||
if (!dslLvlCursor) { | ||
// start at the top level | ||
dslLvlCursor = dslTopLvl; | ||
// when sampling jump directly to the aggs | ||
if (this.isSamplingEnabled()) { | ||
dslLvlCursor = dslLvlCursor.sampling.aggs; | ||
} | ||
} else { | ||
const prevConfig: AggConfig = list[i - 1]; | ||
const prevDsl = dslLvlCursor[prevConfig.id]; | ||
|
@@ -452,7 +477,12 @@ export class AggConfigs { | |
doc_count: response.rawResponse.hits?.total as estypes.AggregationsAggregate, | ||
}; | ||
} | ||
const aggCursor = transformedRawResponse.aggregations!; | ||
const aggCursor = this.isSamplingEnabled() | ||
? (transformedRawResponse.aggregations!.sampling! as Record< | ||
string, | ||
estypes.AggregationsAggregate | ||
>) | ||
: transformedRawResponse.aggregations!; | ||
|
||
mergeTimeShifts(this, aggCursor); | ||
return { | ||
|
@@ -531,6 +561,8 @@ export class AggConfigs { | |
metricsAtAllLevels: this.hierarchical, | ||
partialRows: this.partialRows, | ||
aggs: this.aggs.map((agg) => buildExpression(agg.toExpressionAst())), | ||
probability: this.opts.probability, | ||
samplerSeed: this.opts.samplerSeed, | ||
}), | ||
]).toAst(); | ||
} | ||
|
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.
Debugging test failures was really hard with all warning from
moment.tz
. This fixed the issue assigning a fixed (supported) timezone formoment
.