From 9b2a23985227f395b95ccc37a0a7a4769802cbb2 Mon Sep 17 00:00:00 2001
From: Yongseok Kang <yongseok.kang@navercorp.com>
Date: Tue, 19 Oct 2021 18:17:38 +0900
Subject: [PATCH] [#86] fix span recorder error method crash issue

---
 lib/context/api-meta-service.js                |  7 +++----
 lib/context/method-descriptor-builder.js       | 10 ++++++++++
 test/context/method-descriptor-builder.test.js |  1 +
 test/instrumentation/module/express.test.js    | 10 +++++++---
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/context/api-meta-service.js b/lib/context/api-meta-service.js
index 7f96ae1e..a59ccac9 100644
--- a/lib/context/api-meta-service.js
+++ b/lib/context/api-meta-service.js
@@ -44,13 +44,12 @@ class StringMetaCache {
   cacheApiWithBuilder(builder) {
     if (!builder) return
 
-    const fullName = builder.getFullName()
-    const cachedValue = this.cache.get(fullName)
-
+    const cacheId = builder.getCacheId()
+    const cachedValue = this.cache.get(cacheId)
     if (cachedValue === null) {
       builder.setApiId(apiMetaCacheKeyGenerator.next)
       const methodDescriptor = builder.build()
-      this.cache.put(fullName, methodDescriptor)
+      this.cache.put(cacheId, methodDescriptor)
       this.sendApiMetaInfo(methodDescriptor)
       return methodDescriptor
     }
diff --git a/lib/context/method-descriptor-builder.js b/lib/context/method-descriptor-builder.js
index ef585c16..381915e6 100644
--- a/lib/context/method-descriptor-builder.js
+++ b/lib/context/method-descriptor-builder.js
@@ -59,6 +59,7 @@ class MethodDescriptorBuilder {
             .setLineNumber(makeLineNumber(namedGroup))
             .setClassName(namedGroup.type)
             .setLocation(`${namedGroup.location}${namedGroup.fileName}`)
+            .setFileName(namedGroup.fileName)
         return builder
     }
 
@@ -104,6 +105,11 @@ class MethodDescriptorBuilder {
         return this
     }
 
+    setFileName(fileName) {
+        this.fileName = fileName
+        return this
+    }
+
     build() {
         const apiDescriptor = this.apiDescriptor || this.computedApiDescriptor
         const fullName = this.fullName || this.computedFullName
@@ -167,6 +173,10 @@ class MethodDescriptorBuilder {
         this.computedFullName = this.makeFullName(this.moduleName, this.objectPath)
         return this
     }
+
+    getCacheId() {
+        return `${this.getFullName()}:${this.lineNumber}:${this.fileName}`
+    }
 }
 
 function makeFunctionName(namedGroup) {
diff --git a/test/context/method-descriptor-builder.test.js b/test/context/method-descriptor-builder.test.js
index 7736f4c4..ac9ba3f0 100644
--- a/test/context/method-descriptor-builder.test.js
+++ b/test/context/method-descriptor-builder.test.js
@@ -20,6 +20,7 @@ test('callstack', (t) => {
     t.equal(actual.functionName, 'app.get', 'functionName')
     t.equal(actual.className, 'Function', 'className')
     t.equal(actual.lineNumber, 481, 'lineNumber')
+    t.equal(actual.getCacheId(), 'express.app.get():481:application.js', `cache ID check`)
 
     let actualMethodDescriptor = actual.build()
     t.equal(actualMethodDescriptor.getModuleName(), 'express', 'MethodDescriptor moduleName')
diff --git a/test/instrumentation/module/express.test.js b/test/instrumentation/module/express.test.js
index 052ff544..2eae5a47 100644
--- a/test/instrumentation/module/express.test.js
+++ b/test/instrumentation/module/express.test.js
@@ -39,7 +39,9 @@ test(`${testName1} Should record request in basic route`, function (t) {
       t.equal(trace.span.annotations[0].value.stringValue, 'api=test&test1=test', 'HTTP param value match')
 
       const actualBuilder = new MethodDescriptorBuilder('express', 'app.get')
-        .setFullName('express.app.get(path, callback)')
+        .setParameterDescriptor('(path, callback)')
+        .setLineNumber(481)
+        .setFileName('application.js')
       const actualMethodDescriptor = apiMetaService.cacheApiWithBuilder(actualBuilder)
       const spanEvent = trace.storage.storage[1]
       t.equal(actualMethodDescriptor.apiId, spanEvent.apiId, 'apiId')
@@ -63,8 +65,10 @@ test(`${testName1} Should record request in basic route`, function (t) {
       const trace = agent.traceContext.currentTraceObject()
       t.false(trace.span.annotations[0], 'HTTP param undefined case')
 
-      const actualBuilder = new MethodDescriptorBuilder('express', 'app.post()')
-        .setFullName('express.app.post(path, callback)')
+      const actualBuilder = new MethodDescriptorBuilder('express', 'app.post')
+        .setParameterDescriptor('(path, callback)')
+        .setLineNumber(481)
+        .setFileName('application.js')
       const actualMethodDescriptor = apiMetaService.cacheApiWithBuilder(actualBuilder)
       const spanEvent = trace.storage.storage[1]
       t.equal(actualMethodDescriptor.apiId, spanEvent.apiId, 'apiId')