Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

feat: truncate payloads according to intake API limits #8

Merged
merged 6 commits into from
Aug 9, 2018
Merged

feat: truncate payloads according to intake API limits #8

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
03655bf
feat: truncate payloads according to intake API limits
watson Aug 6, 2018
7021e54
test: add truncation tests
watson Aug 6, 2018
466c553
docs: fix typo
watson Aug 7, 2018
0ad9e5f
fix: truncate events if using client as a writable stream
watson Aug 7, 2018
fa973b1
fix: truncation
watson Aug 7, 2018
ee48136
Merge branch 'api-v2' into truncate
watson Aug 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,23 +102,27 @@ Client.prototype._write = function (obj, enc, cb) {
this._chopper.chop(cb)
}
} else {
if ('transaction' in obj) {
truncate.transaction(obj, this._opts)
} else if ('span' in obj) {
truncate.span(obj, this._opts)
} else if ('error' in obj) {
truncate.error(obj, this._opts)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these not point to the sub-object, not the base object? ie: obj.transaction?

Also, is there a reason to else-if these? Are we unable to send different types at the same time?

Copy link
Contributor Author

@watson watson Aug 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right, it should be truncate.error(obj.error, this._opts) etc. I'll make an update.

The client is an object stream, so each write will be a single object containing either a transaction, a span, or an error. I could use a switch(Object.keys(obj)[0]), but I felt that was less performant than 3 if-statements.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see a case for sending a span and error at the same time. You wrap an async call in a span from call to callback, and an error occurs, so you report the error and span when the callback is reached. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version 2 of the APM Server intake API currently doesn't support that an object contains more than a single root property - either transaction, span or error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's fine then. Might be worth considering for @elastic/apm-server to support in the future though.

this._received++
this._stream.write(obj, cb)
}
}

Client.prototype.sendSpan = function (span, cb) {
truncate.span(span, this._opts)
return this.write({span}, cb)
}

Client.prototype.sendTransaction = function (transaction, cb) {
truncate.transaction(transaction, this._opts)
return this.write({transaction}, cb)
}

Client.prototype.sendError = function (error, cb) {
truncate.error(error, this._opts)
return this.write({error}, cb)
}

Expand Down