[#242] Extract Class refactoring from ActiveTrace to ActiveRequestRepository

[#242] Extract Class refactoring from ActiveTrace to ActiveRequestRepository

Author: feelform

lib/client/grpc-data-sender.js

@@ -350,12 +350,12 @@ class GrpcDataSender {
     try {
       const pStatMessage = dataConvertor.convertStat(stat)
       if (log.isDebug()) {
-        log.debug(`sendStats pStatMessage: ${stat}`)
+        log.debug('sendStats pStatMessage: ', stat)
       }
       this.statStream.write(pStatMessage)
     } catch (e) {
       if (e && e.stack) {
-        log.error(`sendStat(stat) Error: ${e.stack}`)
+        log.error('sendStat(stat) Error: ', e)
       }
     }
   }

lib/context/trace-context.js

@@ -23,7 +23,7 @@ const AsyncSpanChunkBuilder = require('./trace/async-span-chunk-builder')
 const ChildTraceBuilder = require('./trace/child-trace-builder')
 const DisableChildTrace = require('./trace/disable-child-trace')
 const disableAsyncId = require('./trace/disable-async-id')
-const ActiveTraceRepository = require('../metric/active-trace-repository')
+const activeRequestRepository = require('../metric/active-request-repository')
 
 class TraceContext {
   constructor(agentInfo, dataSender, config) {
@@ -35,7 +35,6 @@ class TraceContext {
       this.enableSampling = config.sampling
     }
     this.traceSampler = new TraceSampler(agentInfo, config)
-    this.activeRequestRepository = new ActiveTraceRepository()
   }
 
   getAgentInfo() {
@@ -81,10 +80,10 @@ class TraceContext {
     if (!trace) {
       return
     }
+
     try {
       trace.close()
-      this.activeRequestRepository.remove(trace.getTraceRoot())
-      // activeTrace.remove(trace)
+      activeRequestRepository.remove(trace.getTraceRoot())
     } catch (e) {
       log.error('Fail to complete trace object', e)
     }
@@ -118,7 +117,7 @@ class TraceContext {
   }
 
   newLocalTrace(traceRoot) {
-    this.activeRequestRepository.register(traceRoot)
+    activeRequestRepository.register(traceRoot)
     return new DisableTrace(traceRoot)
   }
 
@@ -137,7 +136,7 @@ class TraceContext {
     const spanBuilder = new SpanBuilder(traceRoot)
     const spanChunkBuilder = new SpanChunkBuilder(traceRoot)
     const repository = new SpanRepository(spanChunkBuilder, this.dataSender, this.agentInfo)
-    this.activeRequestRepository.register(traceRoot)
+    activeRequestRepository.register(traceRoot)
     return new Trace2(spanBuilder, repository)
   }
 

lib/metric/active-request-repository.js

@@ -0,0 +1,48 @@
+/**
+ * Pinpoint Node.js Agent
+ * Copyright 2020-present NAVER Corp.
+ * Apache License v2.0
+ */
+
+'use strict'
+
+const SimpleCache = require('../utils/simple-cache')
+const ActiveTraceHistogram = require('./active-trace-histogram')
+const HistogramSchema = require('./histogram-schema')
+
+// DefaultActiveTraceRepository.java
+class ActiveRequestRepository {
+    constructor() {
+        this.activeTraceCache = new SimpleCache()
+    }
+
+    register(localTraceRoot) {
+        const id = localTraceRoot.getTransactionId()
+        if (typeof id !== 'string' || id.length < 1) {
+            return
+        }
+
+        this.activeTraceCache.put(id, localTraceRoot)
+    }
+
+    remove(localTraceRoot) {
+        const id = localTraceRoot.getTransactionId()
+        this.activeTraceCache.delete(id)
+    }
+
+    getCurrentActiveTraceHistogram() {
+        const currentTime = Date.now()
+        return this.getActiveTraceHistogram(currentTime)
+    }
+
+    getActiveTraceHistogram(currentTime) {
+        const histogram = new ActiveTraceHistogram(HistogramSchema.NORMAL_SCHEMA)
+        this.activeTraceCache.getAll().forEach((traceRoot) => {
+            const elapsedTime = currentTime - traceRoot.getTraceStartTime()
+            histogram.increase(elapsedTime)
+        })
+        return histogram
+    }
+}
+
+module.exports = new ActiveRequestRepository()

lib/metric/active-trace-repository.js

@@ -8,6 +8,7 @@
 
 const ResourceStatsCollector = require('./resource-stats-collector')
 const log = require('../utils/logger')
+const activeRequestRepository = require('../metric/active-request-repository')
 
 class AgentStatsMonitor {
   constructor(dataSender, agentId, agentStartTime) {
@@ -47,7 +48,7 @@ class AgentStatsMonitor {
       collectInterval: 1000,
       memory: this.resourceStatCollector.getMemoryStats(),
       cpu: cpuStatus,
-      // activeTrace: activeTrace.getCurrentActiveTraceHistogram(),
+      activeTrace: activeRequestRepository.getCurrentActiveTraceHistogram(),
     }
   }
 }

lib/metric/agent-stats-monitor.js

@@ -6,59 +6,20 @@
 
 const test = require('tape')
 const axios = require('axios')
-const { fixture, util } = require('../test-helper')
-const activeTrace = require('../../lib/metric/active-trace')
 const agent = require('../support/agent-singleton-mock')
 const express = require('express')
 const http = require('http')
 const grpc = require('@grpc/grpc-js')
 const services = require('../../lib/data/v1/Service_grpc_pb')
 const spanMessages = require('../../lib/data/v1/Span_pb')
+const activeRequestRepository = require('../../lib/metric/active-request-repository')
 
 const TEST_ENV = {
   host: 'localhost',
   port: 5005,
 }
 const getServerUrl = (path) => `http://${TEST_ENV.host}:${TEST_ENV.port}${path}`
 
-test(`Should record active trace in multiple call`, function (t) {
-  agent.bindHttp()
-
-  const PATH = '/active-trace'
-  const LASTONE_PATH = '/active-trace/lastone'
-  const SHUTDOWN = '/shutdown'
-  const app = new express()
-
-  app.get(PATH, async (req, res) => {
-    await util.sleep(2000)
-    res.send('ok get')
-  })
-
-  app.get(LASTONE_PATH, async (req, res) => {
-    res.send('ok get')
-  })
-
-  const server = app.listen(TEST_ENV.port, async function () {
-    Promise.all([
-      axios.get(getServerUrl(PATH), { httpAgent: new http.Agent({ keepAlive: false }) }),
-      axios.get(getServerUrl(PATH), { httpAgent: new http.Agent({ keepAlive: false }) }),
-      axios.get(getServerUrl(LASTONE_PATH), { httpAgent: new http.Agent({ keepAlive: false }) }),
-    ]).then((result) => {
-      t.equal(activeTrace.getAllTraces().length, 0)
-      t.equal('' + agent.mockAgentStartTime, agent.agentInfo.startTimestamp, "startTimestamp equals")
-
-      server.close()
-      t.end()
-    }).catch(() => {
-      server.close()
-      t.end()
-    })
-  })
-
-  t.equal(agent.mockAgentId, fixture.config.agentId, "Agent ID equals")
-  t.equal(agent.agentInfo, agent.pinpointClient.agentInfo, "AgentInfo equals")
-})
-
 test(`Active trace should be recorded with HTTP call`, function (t) {
   const collectorServer = new grpc.Server()
   collectorServer.addService(services.MetadataService, {
@@ -73,7 +34,7 @@ test(`Active trace should be recorded with HTTP call`, function (t) {
     const app = new express()
     app.get('/active-trace', async (req, res) => {
       agent.callbackTraceClose((trace) => {
-        const actualCached = agent.getTraceContext().activeRequestRepository.activeTraceCache.get(trace.getTraceRoot().getTransactionId())
+        const actualCached = activeRequestRepository.activeTraceCache.get(trace.getTraceRoot().getTransactionId())
         t.equal(actualCached, trace.getTraceRoot(), 'active trace traceRoot is cached')
       })
       setTimeout(() => {
@@ -85,7 +46,7 @@ test(`Active trace should be recorded with HTTP call`, function (t) {
       const result = await axios.get(getServerUrl('/active-trace'), { httpAgent: new http.Agent({ keepAlive: false }) })
       t.equal(result.status, 200, 'status code is 200')
       server.close(() => {
-        const cacheSize = agent.getTraceContext().activeRequestRepository.activeTraceCache.cache.size
+        const cacheSize = activeRequestRepository.activeTraceCache.cache.size
         t.equal(cacheSize, 0, 'active trace cache is empty')
         t.end()
       })

test/stats/active-trace.test.js

@@ -23,6 +23,7 @@ const transactionIdGenerator = require('../../lib/context/sequence-generators').
 const closedTraceWrapped = Symbol('closedTraceWrapped')
 const stringMetaService = require('../../lib/context/string-meta-service')
 const apiMetaService = require('../../lib/context/api-meta-service')
+const activeRequestRepository = require('../../lib/metric/active-request-repository')
 
 let traces = []
 const resetTraces = () => {
@@ -111,7 +112,7 @@ class MockAgent extends Agent {
         if (sampler.getSamplingCountGenerator()) {
             sampler.getSamplingCountGenerator().reset()
         }
-        this.traceContext.activeRequestRepository.activeTraceCache.cache.clear()
+        activeRequestRepository.activeTraceCache.cache.clear()
         transactionIdGenerator.reset()
 
         httpShared.clearPathMatcher()