chore: Wrapped parser in try/catch, updated tests

Signed-off-by: mrickard <maurice@mauricerickard.com>
newrelic · Sep 25, 2023 · 685e9ce · 685e9ce
1 parent a83fb6a
commit 685e9ce
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 21 deletions.
diff --git a/lib/instrumentation/@elastic/elasticsearch.js b/lib/instrumentation/@elastic/elasticsearch.js
@@ -4,6 +4,7 @@
  */
 
 'use strict'
+const logger = require('../../logger').child({ component: 'ElasticSearch' })
 
 module.exports = function initialize(_agent, elastic, _moduleName, shim) {
   shim.setDatastore(shim.ELASTICSEARCH)
@@ -37,22 +38,29 @@ function queryParser(params) {
   let collection
 
   const suffix = actions[params.method]
-  path.forEach((segment, idx) => {
-    const prev = idx - 1
-    let opname
-    if (segment === '_search') {
-      collection = path?.[prev] || defaultCollection
-      operation = `search`
-    } else if (segment[0] === '_') {
-      opname = segment.substring(1)
-      collection = path?.[prev] || defaultCollection
-      operation = `${opname}.${suffix}`
+  try {
+    path.forEach((segment, idx) => {
+      const prev = idx - 1
+      let opname
+      if (segment === '_search') {
+        collection = path?.[prev] || defaultCollection
+        operation = `search`
+      } else if (segment[0] === '_') {
+        opname = segment.substring(1)
+        collection = path?.[prev] || defaultCollection
+        operation = `${opname}.${suffix}`
+      }
+    })
+    if (!operation && !collection) {
+      // likely creating an index--no underscore segments
+      collection = path?.[1] || defaultCollection
+      operation = `index.${suffix}`
     }
-  })
-  if (!operation && !collection) {
-    // likely creating an index--no underscore segments
-    collection = path[1]
-    operation = `index.${suffix}`
+  } catch (e) {
+    logger.warn('Failed to parse path for operation and collection. Using defaults')
+    logger.warn(e)
+    collection = defaultCollection
+    operation = 'unknown'
   }
 
   // the substance of the query may be in querystring or in body.

diff --git a/test/versioned/elastic/elasticsearch.tap.js b/test/versioned/elastic/elasticsearch.tap.js
@@ -68,8 +68,8 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
       const firstChild = trace.root.children[0]
       t.equal(
         firstChild.name,
-        'Datastore/statement/ElasticSearch/test/index.create',
-        'should record index creation'
+        'Datastore/statement/ElasticSearch/test/index.update',
+        'should record index PUT as update'
       )
     })
   })
@@ -306,12 +306,12 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
         'Datastore/allWeb': 5,
         'Datastore/ElasticSearch/all': 5,
         'Datastore/ElasticSearch/allWeb': 5,
-        'Datastore/operation/ElasticSearch/doc.create': 1,
-        'Datastore/operation/ElasticSearch/doc.search': 1,
+        'Datastore/operation/ElasticSearch/doc.update': 1,
+        'Datastore/operation/ElasticSearch/doc.get': 1,
         'Datastore/operation/ElasticSearch/doc.exists': 1,
         'Datastore/operation/ElasticSearch/search': 1,
-        'Datastore/statement/ElasticSearch/test/doc.create': 1,
-        'Datastore/statement/ElasticSearch/test/doc.search': 1,
+        'Datastore/statement/ElasticSearch/test/doc.update': 1,
+        'Datastore/statement/ElasticSearch/test/doc.get': 1,
         'Datastore/statement/ElasticSearch/test/doc.exists': 1,
         'Datastore/statement/ElasticSearch/test/doc.delete': 1,
         'Datastore/statement/ElasticSearch/any/search': 1