@elastic/ecs-pino-format - npm Package Compare versions

Comparing version 1.0.0 to 1.1.0

CHANGELOG.md

 # @elastic/ecs-pino-format Changelog
 
+## v1.1.0
+
+- Fix a "TypeError: Cannot read property 'host' of undefined" crash when using
+  `convertReqRes: true` and logging a `req` field that is not an HTTP request
+  object.
+
+- Set the "message" to the empty string for logger calls that provide no
+  message, e.g. `log.info({foo: 'bar'})`. In this case pino will not add a
+  message field, which breaks ecs-logging spec.
+
+- Fix handling when the [`base`](https://getpino.io/#/docs/api?id=base-object)
+  option is used to the pino constructor.
+
+  Before this change, using, for example:
+        const log = pino({base: {foo: "bar"}, ...ecsFormat()})
+  would result in two issues:
+  1. The log records would not include the "foo" field.
+  2. The log records would include `"process": {}, "host": {}` for the
+     expected process.pid and os.hostname.
+
+  Further, if the following is used:
+        const log = pino({base: null, ...ecsFormat()})
+  pino does not call `formatters.bindings()` at all, resulting in log
+  records that were missing "ecs.version" (making them invalid ecs-logging
+  records) and part of the APM integration.
+
+- Add `apmIntegration: false` option to all ecs-logging formatters to
+  enable explicitly disabling Elastic APM integration.
+  ([#62](https://github.com/elastic/ecs-logging-nodejs/pull/62))
+
+- Fix "elasticApm.isStarted is not a function" crash on startup.
+  ([#60](https://github.com/elastic/ecs-logging-nodejs/issues/60))
+
 ## v1.0.0
@@ -4,0 +37,0 @@

index.js

@@ -35,12 +35,24 @@ // Licensed to Elasticsearch B.V. under one or more contributor
 
+// Create options for `pino(...)` that configure it for ecs-logging output.
+//
+// @param {Object} opts - Optional.
+//    - {Boolean} opts.convertErr - Whether to convert a logged `err` field
+//      to ECS error fields. Default true, to match Pino's default of having
+//      an `err` serializer.
+//    - {Boolean} opts.convertReqRes - Whether to convert logged `req` and `res`
+//      HTTP request and response fields to ECS HTTP, User agent, and URL
+//      fields. Default false.
+//    - {Boolean} opts.apmIntegration - Whether to automatically integrate with
+//      Elastic APM (https://github.com/elastic/apm-agent-nodejs). If a started
+//      APM agent is detected, then log records will include the following
+//      fields:
+//        - "service.name" - the configured serviceName in the agent
+//        - "event.dataset" - set to "$serviceName.log" for correlation in Kibana
+//        - "trace.id", "transaction.id", and "span.id" - if there is a current
+//          active trace when the log call is made
+//      Default true.
 function createEcsPinoOptions (opts) {
-  // Boolean options for whether to specially handle some logged field names:
-  //  - `err` to ECS Error fields
-  //  - `req` and `res` to ECS HTTP, User agent, etc. fields
-  // These intentionally match the common serializers
-  // (https://getpino.io/#/docs/api?id=serializers-object). If enabled,
-  // this ECS conversion will take precedence over a serializer for the same
-  // field name.
   let convertErr = true
   let convertReqRes = false
+  let apmIntegration = true
   if (opts) {
@@ -53,8 +65,24 @@ if (hasOwnProperty.call(opts, 'convertErr')) {
     }
+    if (hasOwnProperty.call(opts, 'apmIntegration')) {
+      apmIntegration = opts.apmIntegration
+    }
   }
 
-  // If there is a *started* APM agent, then use it.
-  const apm = elasticApm && elasticApm.isStarted() ? elasticApm : null
+  let apm = null
+  let apmServiceName = null
+  if (apmIntegration && elasticApm && elasticApm.isStarted && elasticApm.isStarted()) {
+    apm = elasticApm
+    // Elastic APM v3.11.0 added getServiceName(). Fallback to private `apm._conf`.
+    // istanbul ignore next
+    apmServiceName = apm.getServiceName
+      ? apm.getServiceName()
+      : apm._conf.serviceName
+  }
 
+  let isServiceNameInBindings = false
+  let isEventDatasetInBindings = false
+
   const ecsPinoOptions = {
+    messageKey: 'message',
+    timestamp: () => `,"@timestamp":"${new Date().toISOString()}"`,
     formatters: {
@@ -65,112 +93,135 @@ level (label, number) {
 
-      // Add the following ECS fields:
-      // - https://www.elastic.co/guide/en/ecs/current/ecs-process.html#field-process-pid
-      // - https://www.elastic.co/guide/en/ecs/current/ecs-host.html#field-host-hostname
-      // - https://www.elastic.co/guide/en/ecs/current/ecs-log.html#field-log-logger
-      //
-      // This is called once at logger creation, and for each child logger creation.
       bindings (bindings) {
         const {
-          // We assume the default `pid` and `hostname` bindings
-          // (https://getpino.io/#/docs/api?id=bindings) will be always be
-          // defined because currently one cannot use this package *and*
-          // pass a custom `formatters` to a pino logger.
+          // `pid` and `hostname` are default bindings, unless overriden by
+          // a `base: {...}` passed to logger creation.
           pid,
           hostname,
           // name is defined if `log = pino({name: 'my name', ...})`
-          name
+          name,
+          // Warning: silently drop any "ecs" value from `base`. See
+          // "ecs.version" comment below.
+          ecs,
+          ...ecsBindings
         } = bindings
 
-        const ecsBindings = {
-          ecs: {
-            version
-          },
-          process: {
-            pid: pid
-          },
-          host: {
-            hostname: hostname
-          }
+        if (pid !== undefined) {
+          // https://www.elastic.co/guide/en/ecs/current/ecs-process.html#field-process-pid
+          ecsBindings.process = { pid: pid }
         }
+        if (hostname !== undefined) {
+          // https://www.elastic.co/guide/en/ecs/current/ecs-host.html#field-host-hostname
+          ecsBindings.host = { hostname: hostname }
+        }
         if (name !== undefined) {
+          // https://www.elastic.co/guide/en/ecs/current/ecs-log.html#field-log-logger
           ecsBindings.log = { logger: name }
         }
 
-        if (apm) {
-          // https://github.com/elastic/apm-agent-nodejs/pull/1949 is adding
-          // getServiceName() in v3.11.0. Fallback to private `apm._conf`.
-          // istanbul ignore next
-          const serviceName = apm.getServiceName
-            ? apm.getServiceName()
-            : apm._conf.serviceName
-          // A mis-configured APM Agent can be "started" but not have a
-          // "serviceName".
-          if (serviceName) {
-            ecsBindings.service = { name: serviceName }
-            ecsBindings.event = { dataset: serviceName + '.log' }
-          }
+        // Note if service.name & event.dataset are set, to not do so again below.
+        if (bindings.service && bindings.service.name) {
+          isServiceNameInBindings = true
         }
+        if (bindings.event && bindings.event.dataset) {
+          isEventDatasetInBindings = true
+        }
 
         return ecsBindings
-      }
-    },
-    messageKey: 'message',
-    timestamp: () => `,"@timestamp":"${new Date().toISOString()}"`
-  }
+      },
 
-  // For performance, avoid adding the `formatters.log` pino option unless we
-  // know we'll do some processing in it.
-  if (convertErr || convertReqRes || apm) {
-    ecsPinoOptions.formatters.log = function (obj) {
-      const {
-        req,
-        res,
-        err,
-        ...ecsObj
-      } = obj
+      log (obj) {
+        const {
+          req,
+          res,
+          err,
+          ...ecsObj
+        } = obj
 
-      // istanbul ignore else
-      if (apm) {
-        // https://www.elastic.co/guide/en/ecs/current/ecs-tracing.html
-        const tx = apm.currentTransaction
-        if (tx) {
-          ecsObj.trace = ecsObj.trace || {}
-          ecsObj.trace.id = tx.traceId
-          ecsObj.transaction = ecsObj.transaction || {}
-          ecsObj.transaction.id = tx.id
-          const span = apm.currentSpan
-          // istanbul ignore else
-          if (span) {
-            ecsObj.span = ecsObj.span || {}
-            ecsObj.span.id = span.id
+        // https://www.elastic.co/guide/en/ecs/current/ecs-ecs.html
+        // For "ecs.version" we take a heavier-handed approach, because it is
+        // a require ecs-logging field: overwrite any possible "ecs" value from
+        // the log statement. This means we don't need to spend the time
+        // guarding against "ecs" being null, Array, Buffer, Date, etc.
+        ecsObj.ecs = { version }
+
+        if (apm) {
+          // A mis-configured APM Agent can be "started" but not have a
+          // "serviceName".
+          if (apmServiceName) {
+            // Per https://github.com/elastic/ecs-logging/blob/master/spec/spec.json
+            // "service.name" and "event.dataset" should be automatically set
+            // if not already by the user.
+            if (!isServiceNameInBindings) {
+              const service = ecsObj.service
+              if (service === undefined) {
+                ecsObj.service = { name: apmServiceName }
+              } else if (!isVanillaObject(service)) {
+                // Warning: "service" type conflicts with ECS spec. Overwriting.
+                ecsObj.service = { name: apmServiceName }
+              } else if (typeof service.name !== 'string') {
+                ecsObj.service.name = apmServiceName
+              }
+            }
+            if (!isEventDatasetInBindings) {
+              const event = ecsObj.event
+              if (event === undefined) {
+                ecsObj.event = { dataset: apmServiceName + '.log' }
+              } else if (!isVanillaObject(event)) {
+                // Warning: "event" type conflicts with ECS spec. Overwriting.
+                ecsObj.event = { dataset: apmServiceName + '.log' }
+              } else if (typeof event.dataset !== 'string') {
+                ecsObj.event.dataset = apmServiceName + '.log'
+              }
+            }
           }
+
+          // https://www.elastic.co/guide/en/ecs/current/ecs-tracing.html
+          const tx = apm.currentTransaction
+          if (tx) {
+            ecsObj.trace = ecsObj.trace || {}
+            ecsObj.trace.id = tx.traceId
+            ecsObj.transaction = ecsObj.transaction || {}
+            ecsObj.transaction.id = tx.id
+            const span = apm.currentSpan
+            // istanbul ignore else
+            if (span) {
+              ecsObj.span = ecsObj.span || {}
+              ecsObj.span.id = span.id
+            }
+          }
         }
-      }
 
-      // https://www.elastic.co/guide/en/ecs/current/ecs-http.html
-      if (err !== undefined) {
-        if (!convertErr) {
-          ecsObj.err = err
-        } else {
-          formatError(ecsObj, err)
+        // https://www.elastic.co/guide/en/ecs/current/ecs-http.html
+        if (err !== undefined) {
+          if (!convertErr) {
+            ecsObj.err = err
+          } else {
+            formatError(ecsObj, err)
+          }
         }
-      }
 
-      // https://www.elastic.co/guide/en/ecs/current/ecs-http.html
-      if (req !== undefined) {
-        if (!convertReqRes) {
-          ecsObj.req = req
-        } else {
-          formatHttpRequest(ecsObj, req)
+        // https://www.elastic.co/guide/en/ecs/current/ecs-http.html
+        if (req !== undefined) {
+          if (!convertReqRes) {
+            ecsObj.req = req
+          } else {
+            formatHttpRequest(ecsObj, req)
+          }
         }
-      }
-      if (res !== undefined) {
-        if (!convertReqRes) {
-          ecsObj.res = res
-        } else {
-          formatHttpResponse(ecsObj, res)
+        if (res !== undefined) {
+          if (!convertReqRes) {
+            ecsObj.res = res
+          } else {
+            formatHttpResponse(ecsObj, res)
+          }
         }
+
+        // If no message (https://getpino.io/#/docs/api?id=message-string) is
+        // given in the log statement, then pino will not emit a message field.
+        // However, the ecs-logging spec requires a message field, so we set
+        // a fallback empty string.
+        ecsObj.message = ''
+
+        return ecsObj
       }
-
-      return ecsObj
     }
@@ -182,2 +233,17 @@ }
 
+// Return true if the given arg is a "vanilla" object. Roughly the intent is
+// whether this is basic mapping of string keys to values that will serialize
+// as a JSON object.
+//
+// Currently, it excludes Map. The uses above don't really expect a user to:
+//     service = new Map([["foo", "bar"]])
+//     log.info({ service }, '...')
+//
+// There are many ways tackle this. See some attempts and benchmarks at:
+// https://gist.github.com/trentm/34131a92eede80fd2109f8febaa56f5a
+function isVanillaObject (o) {
+  return (typeof o === 'object' &&
+    (!o.constructor || o.constructor.name === 'Object'))
+}
+
 module.exports = createEcsPinoOptions

package.json

 {
   "name": "@elastic/ecs-pino-format",
-  "version": "1.0.0",
+  "version": "1.1.0",
   "description": "A formatter for the pino logger compatible with Elastic Common Schema.",
@@ -37,3 +37,3 @@ "main": "index.js",
   "dependencies": {
-    "@elastic/ecs-helpers": "^1.0.0"
+    "@elastic/ecs-helpers": "^1.1.0"
   },
@@ -40,0 +40,0 @@ "devDependencies": {

Fixed alerts

Major refactor

Supply chain risk

Package has recently undergone a major refactor. It may be unstable or indicate significant internal changes. Use caution when updating to versions that include significant changes.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

27682

increased by20.33%

Lines of code

223

increased by37.65%

Number of medium supply chain risk alerts

0

decreased by-100%

Dependency changes

• Updated@elastic/ecs-helpers@^1.1.0