-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Apollo Engine Reporting Duration Histogram #3671
Changes from all commits
9df3571
9a1aa14
b98aac3
e7b9751
8246d32
20d46f6
493d2f8
cb4b871
a428dfa
e63a74d
0a6d283
1c999f2
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 |
---|---|---|
|
@@ -23,3 +23,6 @@ node_modules/ | |
|
||
# Mac OS | ||
.DS_Store | ||
|
||
# Intellij Configuration Files | ||
.idea/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
import { DurationHistogram } from "../durationHistogram"; | ||
|
||
|
||
describe("Duration histogram tests", () => { | ||
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. I think there are a lot more DurationHistogramTest.kt tests that could be ported (eg all of testSerialize) 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. Added most of the ones I felt were relevant. I left out the builder tests, and the de-serialization tests. |
||
it("generateEmptyHistogram", () => { | ||
let emptyDurationHistogram = new DurationHistogram(); | ||
expect([]).toEqual(emptyDurationHistogram.toArray()); | ||
}); | ||
|
||
it("nonEmptyHistogram", () => { | ||
let nonEmptyDurationHistogram = new DurationHistogram(); | ||
nonEmptyDurationHistogram.incrementBucket(100); | ||
expect([-100, 1]).toEqual(nonEmptyDurationHistogram.toArray()); | ||
|
||
nonEmptyDurationHistogram.incrementBucket(102); | ||
expect([-100, 1, 0, 1]).toEqual(nonEmptyDurationHistogram.toArray()); | ||
|
||
nonEmptyDurationHistogram.incrementBucket(382); | ||
expect([-100, 1, 0, 1, -279, 1]).toEqual(nonEmptyDurationHistogram.toArray()); | ||
}); | ||
|
||
it("testToArray", () => { | ||
function assertInitArrayHelper(expected: number[], buckets: number[], initSize = 118) { | ||
expect(new DurationHistogram({initSize, buckets}).toArray()).toEqual(expected); | ||
} | ||
|
||
function assertInsertValueHelper(expected: number[], buckets: number[], initSize = 118) { | ||
let histogram = new DurationHistogram({initSize}); | ||
buckets.forEach((val, bucket) => { | ||
histogram.incrementBucket(bucket, val); | ||
} | ||
); | ||
expect(histogram.toArray()).toEqual(expected); | ||
} | ||
|
||
function metaToArrayFuzzer(assertToArrayHelper: any, initSize = 118) { | ||
assertToArrayHelper([], [], initSize); | ||
assertToArrayHelper([], [0], initSize); | ||
assertToArrayHelper([], [0, 0, 0, 0], initSize); | ||
assertToArrayHelper([1], [1], initSize); | ||
assertToArrayHelper([100_000_000_000], [100_000_000_000], initSize); | ||
assertToArrayHelper([1, 0, 5], [1, 0, 5], initSize); | ||
assertToArrayHelper([1, -2, 5], [1, 0, 0, 5], initSize); | ||
assertToArrayHelper([0, 5], [0, 5], initSize); | ||
assertToArrayHelper([0, 1, -2, 2, -3, 3, -2, 4, 0, 5], [0, 1, 0, 0, 2, 0, 0, 0, 3, 0, 0, 4, 0, 5, 0], initSize); | ||
assertToArrayHelper([-2, 5], [0, 0, 5], initSize); | ||
assertToArrayHelper([-3, 5], [0, 0, 0, 5], initSize); | ||
assertToArrayHelper([-2, 5, -3, 10], [0, 0, 5, 0, 0, 0, 10], initSize); | ||
} | ||
|
||
metaToArrayFuzzer(assertInitArrayHelper); | ||
metaToArrayFuzzer(assertInitArrayHelper, 1); | ||
metaToArrayFuzzer(assertInitArrayHelper, 5); | ||
|
||
metaToArrayFuzzer(assertInsertValueHelper); | ||
metaToArrayFuzzer(assertInsertValueHelper, 1); | ||
metaToArrayFuzzer(assertInsertValueHelper, 5); | ||
}); | ||
|
||
it("combineHistogram", () => { | ||
let firstHistogram = new DurationHistogram({initSize:0}); | ||
firstHistogram.incrementBucket(20); | ||
let secondHistogram = new DurationHistogram(); | ||
secondHistogram.incrementBucket(40); | ||
secondHistogram.incrementBucket(100, 10); | ||
|
||
firstHistogram.combine(secondHistogram); | ||
|
||
expect([-20, 1, -19, 1, -59, 10]).toEqual(firstHistogram.toArray()); | ||
}); | ||
|
||
it("bucketZeroToOne", () => { | ||
expect(DurationHistogram.durationToBucket(-1)).toEqual(0); | ||
expect(DurationHistogram.durationToBucket(0)).toEqual(0); | ||
expect(DurationHistogram.durationToBucket(1)).toEqual(0); | ||
expect(DurationHistogram.durationToBucket(999)).toEqual(0); | ||
expect(DurationHistogram.durationToBucket(1000)).toEqual(0); | ||
expect(DurationHistogram.durationToBucket(1001)).toEqual(1); | ||
}); | ||
|
||
it("bucketOneToTwo", () => { | ||
expect(DurationHistogram.durationToBucket(1100)).toEqual(1); | ||
expect(DurationHistogram.durationToBucket(1101)).toEqual(2); | ||
}); | ||
|
||
it("bucketToThreshold", () => { | ||
expect(DurationHistogram.durationToBucket(10000)).toEqual(25); | ||
expect(DurationHistogram.durationToBucket(10834)).toEqual(25); | ||
expect(DurationHistogram.durationToBucket(10835)).toEqual(26); | ||
}); | ||
|
||
it("bucketForCommonTimes", () => { | ||
expect(DurationHistogram.durationToBucket(1e5)).toEqual(49); | ||
expect(DurationHistogram.durationToBucket(1e6)).toEqual(73); | ||
expect(DurationHistogram.durationToBucket(1e9)).toEqual(145); | ||
}); | ||
|
||
it("testLastBucket", () => { | ||
// Test an absurdly large number gets stuck in the last bucket | ||
expect(DurationHistogram.durationToBucket(1e64)).toEqual(383); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
export interface DurationHistogramOptions { | ||
initSize?: number; | ||
buckets?: number[]; | ||
} | ||
export class DurationHistogram { | ||
private readonly buckets: number[]; | ||
public static readonly BUCKET_COUNT = 384; | ||
public static readonly EXPONENT_LOG = Math.log(1.1); | ||
|
||
public toArray(): number[] { | ||
let bufferedZeroes = 0; | ||
const outputArray: number[] = []; | ||
|
||
for (const value of this.buckets) { | ||
if (value === 0) { | ||
bufferedZeroes++; | ||
} else { | ||
if (bufferedZeroes === 1) { | ||
outputArray.push(0); | ||
} else if (bufferedZeroes !== 0) { | ||
outputArray.push(-bufferedZeroes); | ||
} | ||
outputArray.push(value); | ||
bufferedZeroes = 0; | ||
} | ||
} | ||
return outputArray; | ||
} | ||
|
||
static durationToBucket(durationNs: number): number { | ||
const log = Math.log(durationNs / 1000.0); | ||
const unboundedBucket = Math.ceil(log / DurationHistogram.EXPONENT_LOG); | ||
|
||
// Compare <= 0 to catch -0 and -infinity | ||
return unboundedBucket <= 0 || Number.isNaN(unboundedBucket) | ||
? 0 | ||
: unboundedBucket >= DurationHistogram.BUCKET_COUNT | ||
? DurationHistogram.BUCKET_COUNT - 1 | ||
: unboundedBucket; | ||
} | ||
|
||
public incrementDuration(durationNs: number) { | ||
this.incrementBucket(DurationHistogram.durationToBucket(durationNs)); | ||
} | ||
|
||
public incrementBucket(bucket: number, value = 1) { | ||
if (bucket >= DurationHistogram.BUCKET_COUNT) { | ||
// Since we don't have fixed size arrays I'd rather throw the error manually | ||
throw Error('Bucket is out of bounds of the buckets array'); | ||
} | ||
|
||
// Extend the array if we haven't gotten it long enough to handle the new bucket | ||
if (bucket >= this.buckets.length) { | ||
const oldLength = this.buckets.length; | ||
this.buckets.length = bucket + 1; | ||
this.buckets.fill(0, oldLength); | ||
} | ||
|
||
this.buckets[bucket] += value; | ||
} | ||
|
||
public combine(otherHistogram: DurationHistogram) { | ||
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 should call incrementBucket so that it does the expansion stuff. |
||
for (let i = 0; i < otherHistogram.buckets.length; i++) { | ||
this.incrementBucket(i, otherHistogram.buckets[i]); | ||
} | ||
} | ||
|
||
constructor(options?: DurationHistogramOptions) { | ||
const initSize = options?.initSize || 74; | ||
const buckets = options?.buckets; | ||
|
||
const arrayInitSize = Math.max(buckets?.length || 0, initSize); | ||
|
||
this.buckets = Array<number>(arrayInitSize).fill(0); | ||
|
||
if (buckets) { | ||
buckets.forEach((val, index) => (this.buckets[index] = val)); | ||
} | ||
} | ||
} |
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.
I don't actually know that this is necessary, since there's no user-visible effect yet. We can add a CHANGELOG once the code is not dead.