Use logger instead of conosle.log (#22)

* replace console logs with plugin logger * poor man's functions tracing via logs Signed-off-by: inge4pres <[email protected]>
elastic · Jul 5, 2022 · af3b208 · af3b208
1 parent 344d619
commit af3b208
Show file tree

Hide file tree

Showing 5 changed files with 187 additions and 174 deletions.
diff --git a/src/plugins/profiling/common/index.ts b/src/plugins/profiling/common/index.ts
@@ -47,7 +47,7 @@ function toMilliseconds(seconds: string): number {
 export function getTopN(obj) {
   const data = [];
 
-  if (obj.topN?.histogram?.buckets !== undefined) {
+  if (obj.topN?.histogram?.buckets!) {
     // needed for data served from Elasticsearch
     for (let i = 0; i < obj.topN.histogram.buckets.length; i++) {
       const bucket = obj.topN.histogram.buckets[i];
@@ -56,13 +56,15 @@ export function getTopN(obj) {
         data.push({ x: bucket.key, y: v.Count.value, g: v.key });
       }
     }
-  } else if (obj.TopN !== undefined) {
+  } else if (obj.TopN!) {
     // needed for data served from fixtures
     for (const x in obj.TopN) {
-      const values = obj.TopN[x];
-      for (let i = 0; i < values.length; i++) {
-        const v = values[i];
-        data.push({ x: toMilliseconds(x), y: v.Count, g: v.Value });
+      if (obj.TopN.hasOwnProperty(x)) {
+        const values = obj.TopN[x];
+        for (let i = 0; i < values.length; i++) {
+          const v = values[i];
+          data.push({ x: toMilliseconds(x), y: v.Count, g: v.Value });
+        }
       }
     }
   }
@@ -88,18 +90,3 @@ export function timeRangeFromRequest(request: any): [number, number] {
   const timeTo = parseInt(request.query.timeTo!, 10);
   return [timeFrom, timeTo];
 }
-
-export const now = (label?: string) => {
-  if (label) {
-    console.log('Started', label);
-  }
-  return new Date().getTime();
-};
-
-export const elapsed = (start: number, label?: string) => {
-  const duration = new Date().getTime() - start;
-  if (label) {
-    console.log(label, `${duration / 1000}s`);
-  }
-  return duration;
-};
diff --git a/src/plugins/profiling/server/plugin.ts b/src/plugins/profiling/server/plugin.ts
@@ -5,13 +5,7 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
-import {
-  PluginInitializerContext,
-  CoreSetup,
-  CoreStart,
-  Plugin,
-  Logger,
-} from '../../../core/server';
+import { PluginInitializerContext, CoreSetup, CoreStart, Plugin, Logger } from 'kibana/server';
 
 import type { DataRequestHandlerContext } from '../../data/server';
 
@@ -46,7 +40,7 @@ export class ProfilingPlugin
     core.getStartServices().then(([_, depsStart]) => {
       const myStrategy = mySearchStrategyProvider(depsStart.data);
       deps.data.search.registerSearchStrategy('myStrategy', myStrategy);
-      registerRoutes(router);
+      registerRoutes(router, this.logger);
     });
 
     return {};

diff --git a/src/plugins/profiling/server/routes/flamegraph.ts b/src/plugins/profiling/server/routes/flamegraph.ts
@@ -5,40 +5,9 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
+import { Logger } from 'kibana/server';
 import { StackTraceID, StackFrameID, FileID, StackTrace, StackFrame, Executable } from './types';
 
-function getExeFileName(exe: any, type: number) {
-  if (exe?.FileName === undefined) {
-    console.log('MISSING EXE');
-    return '';
-  }
-  if (exe.FileName !== '') {
-    return exe.FileName;
-  }
-  switch (type) {
-    case 0:
-      return '<unsymbolized frame>';
-    case 1:
-      return 'Python';
-    case 2:
-      return 'PHP';
-    case 3:
-      return 'Native';
-    case 4:
-      return 'Kernel';
-    case 5:
-      return 'JVM/Hotspot';
-    case 6:
-      return 'Ruby';
-    case 7:
-      return 'Perl';
-    case 8:
-      return 'JavaScript';
-    default:
-      return '';
-  }
-}
-
 function checkIfStringHasParentheses(s: string) {
   return /\(|\)/.test(s);
 }
@@ -49,16 +18,6 @@ function getFunctionName(frame: any) {
     : frame.FunctionName;
 }
 
-// Generates the label for a flamegraph node
-//
-// This is slightly modified from the original code in elastic/prodfiler_ui
-function getLabel(frame: any, executable: any, type: number) {
-  if (frame.FunctionName !== '') {
-    return `${getExeFileName(executable, type)}: ${getFunctionName(frame)} in #${frame.LineNumber}`;
-  }
-  return getExeFileName(executable, type);
-}
-
 export class FlameGraph {
   // sampleRate is 1/5^N, with N being the downsampled index the events were fetched from.
   // N=0: full events table (sampleRate is 1)
@@ -77,20 +36,68 @@ export class FlameGraph {
   stackframes: Map<StackFrameID, StackFrame>;
   executables: Map<FileID, Executable>;
 
+  private readonly logger: Logger;
+
   constructor(
     sampleRate: number,
     totalCount: number,
     events: Map<StackTraceID, number>,
     stackTraces: Map<StackTraceID, StackTrace>,
     stackFrames: Map<StackFrameID, StackFrame>,
-    executables: Map<FileID, Executable>
+    executables: Map<FileID, Executable>,
+    logger: Logger
   ) {
     this.sampleRate = sampleRate;
     this.totalCount = totalCount;
     this.events = events;
     this.stacktraces = stackTraces;
     this.stackframes = stackFrames;
     this.executables = executables;
+    this.logger = logger;
+  }
+
+  private getExeFileName(exe: any, type: number) {
+    if (exe?.FileName === undefined) {
+      this.logger.warn('missing executable FileName');
+      return '';
+    }
+    if (exe.FileName !== '') {
+      return exe.FileName;
+    }
+    switch (type) {
+      case 0:
+        return '<unsymbolized frame>';
+      case 1:
+        return 'Python';
+      case 2:
+        return 'PHP';
+      case 3:
+        return 'Native';
+      case 4:
+        return 'Kernel';
+      case 5:
+        return 'JVM/Hotspot';
+      case 6:
+        return 'Ruby';
+      case 7:
+        return 'Perl';
+      case 8:
+        return 'JavaScript';
+      default:
+        return '';
+    }
+  }
+
+  // Generates the label for a flamegraph node
+  //
+  // This is slightly modified from the original code in elastic/prodfiler_ui
+  private getLabel(frame: any, executable: any, type: number) {
+    if (frame.FunctionName !== '') {
+      return `${this.getExeFileName(executable, type)}: ${getFunctionName(frame)} in #${
+        frame.LineNumber
+      }`;
+    }
+    return this.getExeFileName(executable, type);
   }
 
   toElastic() {
@@ -100,7 +107,7 @@ export class FlameGraph {
     for (const trace of this.stacktraces.values()) {
       const path = ['root'];
       for (let i = trace.FrameID.length - 1; i >= 0; i--) {
-        const label = getLabel(
+        const label = this.getLabel(
           this.stackframes.get(trace.FrameID[i]),
           this.executables.get(trace.FileID[i]),
           parseInt(trace.Type[i], 10)

diff --git a/src/plugins/profiling/server/routes/index.ts b/src/plugins/profiling/server/routes/index.ts
@@ -5,7 +5,7 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
-import type { IRouter } from 'kibana/server';
+import type { IRouter, Logger } from 'kibana/server';
 import { DataRequestHandlerContext } from '../../../data/server';
 import { registerFlameChartElasticRoute, registerFlameChartPixiRoute } from './load_flamechart';
 import {
@@ -25,7 +25,7 @@ import {
   registerTraceEventsTopNThreadsSearchRoute,
 } from './search_topn';
 
-export function registerRoutes(router: IRouter<DataRequestHandlerContext>) {
+export function registerRoutes(router: IRouter<DataRequestHandlerContext>, logger?: Logger) {
   registerFlameChartElasticRoute(router);
   registerFlameChartPixiRoute(router);
   registerTraceEventsTopNContainersRoute(router);
@@ -34,7 +34,7 @@ export function registerRoutes(router: IRouter<DataRequestHandlerContext>) {
   registerTraceEventsTopNStackTracesRoute(router);
   registerTraceEventsTopNThreadsRoute(router);
 
-  registerFlameChartSearchRoute(router);
+  registerFlameChartSearchRoute(router, logger!);
   registerTraceEventsTopNContainersSearchRoute(router);
   registerTraceEventsTopNDeploymentsSearchRoute(router);
   registerTraceEventsTopNHostsSearchRoute(router);