fix: traceids not random when using webcrypto (#825)

Co-authored-by: Patrick Housley <[email protected]>

Author: paddyo

src/common/ids/unique-id.js

@@ -43,13 +43,14 @@ export function generateUuid () {
   let randomValueTable
   let randomValueIndex = 0
   if (crypto && crypto.getRandomValues) {
+    // For a UUID, we only need 30 characters since two characters are pre-defined
     // eslint-disable-next-line
-    randomValueTable = crypto.getRandomValues(new Uint8Array(31))
+    randomValueTable = crypto.getRandomValues(new Uint8Array(30))
   }
 
   return uuidv4Template.split('').map(templateInput => {
     if (templateInput === 'x') {
-      return getRandomValue(randomValueTable, ++randomValueIndex).toString(16)
+      return getRandomValue(randomValueTable, randomValueIndex++).toString(16)
     } else if (templateInput === 'y') {
       // this is the uuid variant per spec (8, 9, a, b)
       // % 4, then shift to get values 8-11
@@ -73,7 +74,7 @@ export function generateRandomHexString (length) {
   let randomValueIndex = 0
   if (crypto && crypto.getRandomValues) {
     // eslint-disable-next-line
-    randomValueTable = crypto.getRandomValues(new Uint8Array(31))
+    randomValueTable = crypto.getRandomValues(new Uint8Array(length))
   }
 
   const chars = []

src/common/ids/unique-id.test.js

@@ -56,3 +56,96 @@ test('generateTraceId should generate a 32 character hex string', () => {
 
   expect(/^[0-9a-f]{32}$/.test(id)).toEqual(true)
 })
+
+describe('#826 trace id should be unique', () => {
+  test('should not generate subsequent ids that are the same', () => {
+    expect(uniqueId.generateTraceId()).not.toEqual(uniqueId.generateTraceId())
+  })
+})
+
+describe('rough randomness tests', () => {
+  /**
+   * This tests the distribution of each hex character. The distribution of each character
+   * should approach 1.0 to show that each character is being used as evenly as possible to
+   * produce the resulting string.
+   */
+  test('should have sufficient distribution of 8 bit character usage', () => {
+    const tally = new Map()
+    const sampleSize = 100000 // A large sample size but not so large that it causes the test to take forever
+
+    for (let i = 0; i < sampleSize; i++) {
+      const chars = uniqueId.generateTraceId().split('')
+      chars.forEach(char => {
+        if (!tally.has(char)) {
+          tally.set(char, 1)
+        } else {
+          tally.set(char, (tally.get(char)) + 1)
+        }
+      })
+    }
+
+    tally.forEach((distributionCount) => {
+      /*
+        Take the square root of the distribution count as a variance stabilization:
+        https://en.wikipedia.org/wiki/Variance-stabilizing_transformation
+
+        Divide by the total number of possible values accounting for placement within the
+        resulting string:
+        16 === the number of unique hex characters
+        32 === the length of the string returned by our random function
+        sampleSize === the total number of times we are running the function
+
+        Subtract 1 to make the result into a percentage.
+       */
+      const distributionDeviation = 1 - Math.sqrt(distributionCount) / (16 * 32 * sampleSize)
+      expect(distributionDeviation).toBeGreaterThanOrEqual(0.999)
+    })
+  })
+
+  /**
+   * This tests the distribution of each hex character. The distribution of each character
+   * should approach 1.0 to show that each character is being used as evenly as possible to
+   * produce the resulting string.
+   */
+  test('should have sufficient distribution of sequential characters', () => {
+    const tally = new Map()
+    const sampleSize = 100000 // A large sample size but not so large that it causes the test to take forever
+
+    for (let i = 0; i < sampleSize; i++) {
+      const chars = uniqueId.generateTraceId().split('')
+
+      chars.forEach((char, index) => {
+        let charSequence
+
+        if (index === 0) {
+          charSequence = `_${char}`
+        } else {
+          charSequence = `${chars[index - 1]}${char}`
+        }
+
+        if (!tally.has(charSequence)) {
+          tally.set(charSequence, 1)
+        } else {
+          tally.set(charSequence, (tally.get(charSequence)) + 1)
+        }
+      })
+    }
+
+    tally.forEach((distributionCount) => {
+      /*
+        Take the square root of the distribution count as a variance stabilization:
+        https://en.wikipedia.org/wiki/Variance-stabilizing_transformation
+
+        Divide by the total number of possible values accounting for placement within the
+        resulting string:
+        16 === the number of unique hex characters
+        32 === the length of the string returned by our random function
+        sampleSize === the total number of times we are running the function
+
+        Subtract 1 to make the result into a percentage.
+       */
+      const distributionDeviation = 1 - Math.sqrt(distributionCount) / (16 * 32 * sampleSize)
+      expect(distributionDeviation).toBeGreaterThanOrEqual(0.999)
+    })
+  })
+})