chore(flamegraph/models): make it mandatory to handle all spyNames

grafana · Jul 22, 2022 · 7474764 · 7474764
1 parent c163b37
commit 7474764
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 22 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -109,6 +109,8 @@ module.exports = {
 
     // makes it easier to check what are local variables computated dynamically and what are static props
     'react/destructuring-assignment': 'off',
+
+    '@typescript-eslint/switch-exhaustiveness-check': 'error',
   },
   env: {
     browser: true,

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/Flamegraph.ts b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/Flamegraph.ts
@@ -5,6 +5,7 @@ import {
   Flamebearer,
   singleFF,
   doubleFF,
+  SpyName,
 } from '@pyroscope/models/src';
 import { PX_PER_LEVEL, BAR_HEIGHT, COLLAPSE_THRESHOLD } from './constants';
 import type { FlamegraphPalette } from './colorPalette';
@@ -78,7 +79,9 @@ export default class Flamegraph {
       sampleRate: this.flamebearer.sampleRate,
       names: this.flamebearer.names,
       levels: this.flamebearer.levels,
-      spyName: this.flamebearer.spyName,
+      // keep type narrow https://stackoverflow.com/q/54333982
+      // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
+      spyName: this.flamebearer.spyName as SpyName,
       units: this.flamebearer.units,
       maxSelf: this.flamebearer.maxSelf,
 

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/Flamegraph_render.ts b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/Flamegraph_render.ts
@@ -22,7 +22,7 @@ THIS SOFTWARE.
 */
 
 /* eslint-disable no-continue */
-import { createFF, Flamebearer } from '@pyroscope/models/src';
+import { createFF, Flamebearer, SpyName } from '@pyroscope/models/src';
 import {
   formatPercent,
   getFormatter,
@@ -240,7 +240,9 @@ export default function RenderCanvas(props: CanvasRendererConfig) {
           selectedLevel,
           highlightModeOn,
           isHighlighted,
-          spyName,
+          // keep type narrow https://stackoverflow.com/q/54333982
+          // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
+          spyName: spyName as SpyName,
           palette,
         };
 
@@ -368,7 +370,7 @@ type getColorCfg = {
   highlightModeOn: boolean;
   isHighlighted: boolean;
   names: string[];
-  spyName: string;
+  spyName: SpyName;
   palette: FlamegraphPalette;
 };
 

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/color.ts b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphComponent/color.ts
@@ -1,6 +1,7 @@
 /* eslint-disable camelcase */
 import Color from 'color';
 import { scaleLinear } from 'd3-scale';
+import type { SpyName } from '@pyroscope/models';
 import murmurhash3_32_gc from './murmur3';
 import type { FlamegraphPalette } from './colorPalette';
 
@@ -71,8 +72,8 @@ export function colorGreyscale(v: number, a: number) {
   return Color.rgb(v, v, v).alpha(a);
 }
 
-// TODO use @pyroscope/models?
-function spyToRegex(spyName: string) {
+function spyToRegex(spyName: SpyName): RegExp {
+  // eslint-disable-next-line default-case
   switch (spyName) {
     case 'dotnetspy':
       return /^(?<packageName>.+)\.(.+)\.(.+)\(.*\)$/;
@@ -100,16 +101,16 @@ function spyToRegex(spyName: string) {
       return /^(?<packageName>.+\/)(?<filename>.+\.)(?<functionName>.+)$/;
     case 'pyroscope-rs':
       return /^(?<packageName>[^::]+)/;
-
-    // we don't have enough information
-    default:
+    case 'unknown':
       return /^(?<packageName>.+)$/;
   }
+
+  return /^(?<packageName>.+)$/;
 }
 
 // TODO spy names?
 export function getPackageNameFromStackTrace(
-  spyName: string,
+  spyName: SpyName,
   stackTrace: string
 ) {
   if (stackTrace.length === 0) {

diff --git a/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphRenderer.tsx b/packages/pyroscope-flamegraph/src/FlameGraph/FlameGraphRenderer.tsx
@@ -53,7 +53,7 @@ function mountFlamebearer(p: { profile?: Profile; flamebearer?: Flamebearer }) {
     names: [],
     units: '',
     levels: [[]],
-    spyName: '',
+    spyName: 'unknown',
     numTicks: 0,
     sampleRate: 0,
     maxSelf: 0,

diff --git a/packages/pyroscope-models/src/flamebearer.ts b/packages/pyroscope-models/src/flamebearer.ts
@@ -1,3 +1,4 @@
+import { SpyName } from './spyName';
 /**
  * @deprecated one should use the Profile model
  */
@@ -31,14 +32,7 @@ export type Flamebearer = {
     | 'lock_nanoseconds'
     | 'trace_samples'
     | '';
-  spyName:
-    | 'dotneyspy'
-    | 'ebpfspy'
-    | 'gospy'
-    | 'phpspy'
-    | 'pyspy'
-    | 'rbspy'
-    | string;
+  spyName: SpyName;
   // format: 'double' | 'single';
   //  leftTicks?: number;
   //  rightTicks?: number;

diff --git a/packages/pyroscope-models/src/index.ts b/packages/pyroscope-models/src/index.ts
@@ -2,3 +2,4 @@ export * from './profile';
 export * from './flamebearer';
 export * from './trace';
 export * from './decode';
+export * from './spyName';
diff --git a/packages/pyroscope-models/src/profile.ts b/packages/pyroscope-models/src/profile.ts
@@ -1,4 +1,5 @@
 import { z } from 'zod';
+import { SpyNameSchema } from './spyName';
 
 export const FlamebearerSchema = z.object({
   names: z.array(z.string().nonempty()),
@@ -40,9 +41,7 @@ export const MetadataSchema = z.object({
 
   format: z.enum(['single', 'double']),
   sampleRate: z.number(),
-  spyName: z
-    .enum(['dotnetspy', 'ebpfspy', 'gospy', 'phpspy', 'pyspy', 'rbspy'])
-    .or(z.string()),
+  spyName: SpyNameSchema,
 
   units: UnitsSchema,
 });

diff --git a/packages/pyroscope-models/src/spyName.ts b/packages/pyroscope-models/src/spyName.ts
@@ -0,0 +1,21 @@
+import { z } from 'zod';
+
+export const SpyNameSchema = z
+  .enum([
+    'dotnetspy',
+    'ebpfspy',
+    'gospy',
+    'phpspy',
+    'pyspy',
+    'rbspy',
+    'nodespy',
+    'javaspy',
+    'pyroscope-rs',
+
+    'scrape', // for compability purposes, it should be golang
+    'tracing',
+    'unknown',
+  ])
+  .default('unknown');
+
+export type SpyName = z.infer<typeof SpyNameSchema>;