add optional pino logger and collection of internal stats #143
Conversation
github-actions
bot
added
the
agent-nodejs
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
| @@ -54,7 +54,8 @@ Server. | |||
|
|
|||
There was a problem hiding this comment.
👍 to the README.md changes. I see we've clarified some documentation about the the options array, added documentation for the new logger format, and made explicit note of the face payloadLogFile has a (negative) impact on perf, and clarified the behavior of http.send w/r/t errors.
README.md
Outdated
| An integer indicating the number of events (spans, transactions, errors, or | ||
| metricsets) sent by the client. An event is considered sent when the HTTP | ||
| request used to transmit it has ended. Note that errors in requests to APM | ||
| server may main this value is not the same as the number of events *accepted* |
There was a problem hiding this comment.
may main this value should be may mean this value?
| @@ -15,6 +15,7 @@ const streamToBuffer = require('fast-stream-to-buffer') | |||
| const StreamChopper = require('stream-chopper') | |||
There was a problem hiding this comment.
👍 to the changes in this file. I see we're assigning our new logger or a NoopLogger, and have added some internal properties for tracking metrics related to the client with a _getStats helper function. The simpler approach makes sense to me for tracking agent perf at this point (vs. a formal metrics system like Measured, which can be heavy). I also see we've renamed the _received property to the more precise _numEventsEnqueued and renamed _writevCleaned to _writeBatch.
loggeroption to the client, which expects a pino logger. If not given, a "no-op" logger will be used to stay silent. This logger is documented to be for trace and debug-level logging only, i.e. nothing about logging for the user of this apmclient or the apm agent using this lib._getStats()is added that can be used by dev/debugging scripts to dump info under load tests.The matching APM agent PR is: elastic/apm-agent-nodejs#2015
This is pre-support for a coming PR to fix #136
I'm trying to separate out some changes to make that change more focused.