From 39e385472b5c30f17d3d9af95b95c7bdf3d47a08 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Wed, 15 Sep 2021 13:47:11 -0700 Subject: [PATCH] fix: drop 'transaction.sync' field, it was never used by apm-server Also add a test for span.sync=false for ES and HTTP spans from @elastic/elasticsearch instrumentation for #1996. Move the span.sync test from async-hooks.test.js over to span.test.js because its impl no longer deps on lib/instrumentation/async-hooks.js. Fixes: #2292 --- lib/instrumentation/generic-span.js | 1 - lib/instrumentation/span.js | 1 + lib/instrumentation/transaction.js | 6 ----- test/instrumentation/async-hooks.test.js | 24 ------------------- .../modules/@elastic/elasticsearch.test.js | 2 ++ test/instrumentation/span.test.js | 23 +++++++++++++----- test/instrumentation/transaction.test.js | 10 -------- 7 files changed, 20 insertions(+), 47 deletions(-) diff --git a/lib/instrumentation/generic-span.js b/lib/instrumentation/generic-span.js index 1cf2ba130b4..cf26d16dc8c 100644 --- a/lib/instrumentation/generic-span.js +++ b/lib/instrumentation/generic-span.js @@ -23,7 +23,6 @@ function GenericSpan (agent, ...args) { this.timestamp = this._timer.start this.ended = false - this.sync = true this.outcome = constants.OUTCOME_UNKNOWN diff --git a/lib/instrumentation/span.js b/lib/instrumentation/span.js index b10e5888365..1d5db4161a5 100644 --- a/lib/instrumentation/span.js +++ b/lib/instrumentation/span.js @@ -35,6 +35,7 @@ function Span (transaction, name, ...args) { this._message = null this._stackObj = null this._capturedStackTrace = null + this.sync = true this._startXid = executionAsyncId() this.transaction = transaction diff --git a/lib/instrumentation/transaction.js b/lib/instrumentation/transaction.js index 89d6ed28c4a..68520a75bd3 100644 --- a/lib/instrumentation/transaction.js +++ b/lib/instrumentation/transaction.js @@ -1,6 +1,5 @@ 'use strict' -const { executionAsyncId } = require('async_hooks') var util = require('util') var ObjectIdentityMap = require('object-identity-map') @@ -36,7 +35,6 @@ function Transaction (agent, name, ...args) { this._contextLost = false // TODO: Send this up to the server some how this._abortTime = 0 this._breakdownTimings = new ObjectIdentityMap() - this._startXid = executionAsyncId() this.outcome = constants.OUTCOME_UNKNOWN } @@ -131,7 +129,6 @@ Transaction.prototype.toJSON = function () { result: String(this.result), sampled: this.sampled, context: undefined, - sync: this.sync, span_count: { started: this._builtSpans }, @@ -234,9 +231,6 @@ Transaction.prototype.end = function (result, endTime) { this._timer.end(endTime) this._captureBreakdown(this) this.ended = true - if (executionAsyncId() !== this._startXid) { - this.sync = false - } var trans = this._agent._instrumentation.currentTransaction diff --git a/test/instrumentation/async-hooks.test.js b/test/instrumentation/async-hooks.test.js index 17fecd33287..a77397e091d 100644 --- a/test/instrumentation/async-hooks.test.js +++ b/test/instrumentation/async-hooks.test.js @@ -121,30 +121,6 @@ test('post-defined, post-resolved promise', function (t) { }) }) -test('sync.sync', function (t) { - var trans = agent.startTransaction() - t.strictEqual(trans.sync, true) - - var span1 = agent.startSpan('span1') - t.strictEqual(span1.sync, true) - - // This span will be *ended* synchronously. It should stay `span.sync=true`. - var span2 = agent.startSpan('span2') - t.strictEqual(span2.sync, true, 'span2.sync=true immediately after creation') - span2.end() - t.strictEqual(span2.sync, true, 'span2.sync=true immediately after end') - - setImmediate(() => { - span1.end() - t.strictEqual(span1.sync, false) - trans.end() - t.strictEqual(trans.sync, false) - t.strictEqual(span2.sync, true, - 'span2.sync=true later after having ended sync') - t.end() - }) -}) - function twice (fn) { setImmediate(fn) setImmediate(fn) diff --git a/test/instrumentation/modules/@elastic/elasticsearch.test.js b/test/instrumentation/modules/@elastic/elasticsearch.test.js index 802dfd9aa38..9b339e17f4a 100644 --- a/test/instrumentation/modules/@elastic/elasticsearch.test.js +++ b/test/instrumentation/modules/@elastic/elasticsearch.test.js @@ -586,12 +586,14 @@ function checkDataAndEnd (t, method, path, dbStatement) { t.strictEqual(esSpan.type, 'db') t.strictEqual(esSpan.subtype, 'elasticsearch') t.strictEqual(esSpan.action, 'request') + t.strictEqual(esSpan.sync, false, 'span.sync=false') const httpSpan = findObjInArray(data.spans, 'subtype', 'http') t.ok(httpSpan, 'have an http span') t.strictEqual(httpSpan.type, 'external') t.strictEqual(httpSpan.subtype, 'http') t.strictEqual(httpSpan.action, method) + t.strictEqual(httpSpan.sync, false, 'span.sync=false') t.equal(httpSpan.name, method + ' ' + host, 'http span should have expected name') t.equal(esSpan.name, 'Elasticsearch: ' + method + ' ' + path, 'elasticsearch span should have expected name') diff --git a/test/instrumentation/span.test.js b/test/instrumentation/span.test.js index 43afa8486e0..682523919b0 100644 --- a/test/instrumentation/span.test.js +++ b/test/instrumentation/span.test.js @@ -161,13 +161,24 @@ test('#addLabels', function (t) { t.end() }) -test('sync/async tracking', function (t) { - var trans = new Transaction(agent) - var span = new Span(trans) - t.strictEqual(span.sync, true) +test('span.sync', function (t) { + var trans = agent.startTransaction() + + var span1 = agent.startSpan('span1') + t.strictEqual(span1.sync, true) + + // This span will be *ended* synchronously. It should stay `span.sync=true`. + var span2 = agent.startSpan('span2') + t.strictEqual(span2.sync, true, 'span2.sync=true immediately after creation') + span2.end() + t.strictEqual(span2.sync, true, 'span2.sync=true immediately after end') + setImmediate(() => { - span.end() - t.strictEqual(span.sync, false) + span1.end() + t.strictEqual(span1.sync, false, 'span1.sync=false immediately after end') + trans.end() + t.strictEqual(span2.sync, true, + 'span2.sync=true later after having ended sync') t.end() }) }) diff --git a/test/instrumentation/transaction.test.js b/test/instrumentation/transaction.test.js index a87cb63bbe9..f79eb28e655 100644 --- a/test/instrumentation/transaction.test.js +++ b/test/instrumentation/transaction.test.js @@ -291,16 +291,6 @@ test('parallel transactions', function (t) { }, 25) }) -test('sync/async tracking', function (t) { - var trans = new Transaction(agent) - t.strictEqual(trans.sync, true) - setImmediate(() => { - trans.end() - t.strictEqual(trans.sync, false) - t.end() - }) -}) - test('#_encode() - un-ended', function (t) { var trans = new Transaction(agent) t.strictEqual(trans._encode(), null, 'cannot encode un-ended transaction')