Skip to content

Commit

Permalink
chore!: remove deprecated span.toString(), transaction.toString()
Browse files Browse the repository at this point in the history
Closes: #2348
  • Loading branch information
trentm committed Aug 2, 2023
1 parent 05257bc commit cd90bde
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 100 deletions.
5 changes: 2 additions & 3 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1214,9 +1214,8 @@ added to multiple events.
of (re)implementing Lambda support ({pull}2363[#2363]). Some fixes for better
flushing of data at the end of a Lambda invocation.
* <<span-to-string,`span.toString()`>> and <<transaction-to-string,`transaction.toString()`>>
have been *deprecated*. The exact string output may change in v4 of the
agent.
* `span.toString()` and `transaction.toString()` have been *deprecated*. The
exact string output may change in v4 of the agent.
* Add `Span.ids` and `Transaction.ids` to TypeScript types. ({pull}2347[#2347])
Expand Down
26 changes: 0 additions & 26 deletions docs/span-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -185,32 +185,6 @@ This enables log correlation to APM traces with structured loggers.
----


[[span-to-string]]
==== `span.toString()` deprecated:[v3.23.0]

[small]#Added in: v2.17.0# +
[small]#Deprecated in: v3.23.0#

Produces a string representation of the span to inject in log messages.
This enables log correlation to APM traces with text-only loggers.

[source,js]
----
"trace.id=abc123 span.id=abc123"
----

Relying on the format of `span.toString()` has been **deprecated** and may
change in v4 of the agent. Prefer the use of <<span-ids,`span.ids`>> or
<<apm-current-trace-ids,`apm.currentTraceIds`>>. The v3 format may be reproduced
via:

[source,js]
----
const { stringify } = require('querystring')
console.log( stringify(span.ids, ' ', '=')) )
----


[[span-end]]
==== `span.end([endTime])`

Expand Down
26 changes: 0 additions & 26 deletions docs/transaction-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -262,32 +262,6 @@ This enables log correlation to APM traces with structured loggers.
----


[[transaction-to-string]]
==== `transaction.toString()` deprecated:[v3.23.0]

[small]#Added in: v2.17.0# +
[small]#Deprecated in: v3.23.0#

Produces a string representation of the transaction to inject in log messages.
This enables log correlation to APM traces with text-only loggers.

[source,js]
----
"trace.id=abc123 transaction.id=abc123"
----

Relying on the format of `transaction.toString()` has been **deprecated** and may
change in v4 of the agent. Prefer the use of <<transaction-ids,`transaction.ids`>> or
<<apm-current-trace-ids,`apm.currentTraceIds`>>. The v3 format may be reproduced
via:

[source,js]
----
const { stringify } = require('querystring')
console.log( stringify(transaction.ids, ' ', '=')) )
----


[[transaction-end]]
==== `transaction.end([result][, endTime])`

Expand Down
36 changes: 32 additions & 4 deletions docs/upgrade-to-v4.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,48 @@ Version 4.0.0 of the Node.js APM agent supports Node.js v14.5.0 and later.
[[v4-config-options]]
==== Config options

===== `ELASTIC_APM_KUBERNETES_*`

Support for the following Kubernetes environment variables have been removed:
`ELASTIC_APM_KUBERNETES_NAMESPACE`, `ELASTIC_APM_KUBERNETES_NODE_NAME`,
`ELASTIC_APM_KUBERNETES_POD_NAME`, and `ELASTIC_APM_KUBERNETES_POD_UID`. The
correct environment variables for these config vars are **without** the
correct environment variables for these config vars are _without_ the
`ELASTIC_APM_` prefix -- for example
<<kubernetes-pod-name,`KUBERNETES_POD_NAME`>> -- and has been documented that
way since v2.11.0.

===== `filterHttpHeaders`

Support for `filterHttpHeaders` config option has been removed. Redaction of
HTTP headers and also request cookies is controlled by the existing config option
<<sanitize-field-names, `sanitizeFieldNames`>>.
HTTP headers and also request cookies is controlled by the existing config
option <<sanitize-field-names, `sanitizeFieldNames`>>.


[[v4-api-changes]]
==== API changes

XXX
[[v4-api-toString]]
===== `span.toString()`, `transaction.toString()`

The `span.toString()` and `transaction.toString()` methods have been removed as
documented APIs. They were never in the "index.d.ts" types and were deprecated
in v3.23.0.

Since v2.17.0 they would return a string of the form `trace.id=<hex id the
trace> span.id=<hex id of the span>`, with the intent that this could be used in
text-only loggers for log correlation. Using `.toString()` for this was
deprecated in v3.23.0, and has now been removed in v4. In v4 the output of
`.toString()` is not defined.

Instead, prefer the use of <<span-ids,`span.ids`>>,
<<transaction-ids,`transaction.ids`>>, or
<<apm-current-trace-ids,`apm.currentTraceIds`>>. The v3 format may be reproduced
via:

[source,js]
----
const {stringify} = require('querystring');
console.log( stringify(span.ids, ' ', '=')) );
----

For log correlation with _structured_ logs, see <<log-correlation-ids>>.
18 changes: 5 additions & 13 deletions lib/instrumentation/run-context/RunContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
// anything for other code bound to the original RunContext (e.g. via
// `ins.bindFunction` or `ins.bindEmitter`).
//
// Warning: Agent code should never using the `RunContext` class directly
// Warning: Agent code should never use the `RunContext` class directly
// because a subclass can be provided for the `Instrumentation` to use.
// Instead new instances should be built from an existing one, typically the
// active one (`_runCtxMgr.active()`) or the root one (`_runCtxMgr.root()`).
Expand Down Expand Up @@ -112,26 +112,18 @@ class RunContext {
// For example:
// RunContext(Transaction(abc123, 'trans name'), [Span(def456, 'span name', ended)])
// ^^^^^^^-- if the span has ended
// ^^^^^^ ^^^^^^-- 6-char prefix of .id
// ^^^^^^^^^^^-- Transaction class name
// ^^^^^^ ^^^^^^-- id
// ^^^^^^^^^^-- the class name, typically "RunContext", but can be overriden
toString() {
const bits = [];
if (this._trans) {
bits.push(
`${this._trans.constructor.name}(${this._trans.id.slice(0, 6)}, '${
this._trans.name
}'${this._trans.ended ? ', ended' : ''})`,
);
bits.push(this._trans.toString());
}
if (this._spans.length > 0) {
const spanStrs = this._spans.map(
(s) =>
`Span(${s.id.slice(0, 6)}, '${s.name}'${s.ended ? ', ended' : ''})`,
);
const spanStrs = this._spans.map((s) => s.toString());
bits.push('[' + spanStrs + ']');
}
return `${this.constructor.name}<${bits.join(', ')}>`;
return `${this.constructor.name}(${bits.join(', ')})`;
}

// ---- The following implements the OTel Context interface.
Expand Down
6 changes: 5 additions & 1 deletion lib/instrumentation/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,12 @@ Object.defineProperty(Span.prototype, 'ids', {
},
});

/*
* A string representation of the span to help with internal debugging. This
* is not a promised interface.
*/
Span.prototype.toString = function () {
return this.ids.toString();
return `Span(${this.id}, '${this.name}'${this.ended ? ', ended' : ''})`;
};

Span.prototype.customStackTrace = function (stackObj) {
Expand Down
8 changes: 7 additions & 1 deletion lib/instrumentation/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,14 @@ Object.defineProperty(Transaction.prototype, 'ids', {
},
});

/*
* A string representation of the transaction to help with internal debugging.
* This is not a promised interface.
*/
Transaction.prototype.toString = function () {
return this.ids.toString();
return `Transaction(${this.id}, '${this.name}'${
this.ended ? ', ended' : ''
})`;
};

Transaction.prototype.setUserContext = function (context) {
Expand Down
12 changes: 0 additions & 12 deletions test/instrumentation/span.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,13 +481,6 @@ test('#ids', function (t) {
t.end();
});

test('#toString()', function (t) {
var trans = new Transaction(agent);
var span = new Span(trans);
t.strictEqual(span.toString(), `trace.id=${span.traceId} span.id=${span.id}`);
t.end();
});

test('Span API on ended span', function (t) {
const trans = agent.startTransaction('theTransName');
const span = trans.startSpan(
Expand Down Expand Up @@ -524,11 +517,6 @@ test('Span API on ended span', function (t) {
{ 'trace.id': traceId, 'span.id': spanId },
'span.ids',
);
t.equal(
span.toString(), // deprecated
`trace.id=${traceId} span.id=${spanId}`,
span.toString(),
);

// We just want to ensure that the Span API methods don't throw. Whether
// they make span field changes after the span has ended isn't tested.
Expand Down
14 changes: 0 additions & 14 deletions test/instrumentation/transaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -681,15 +681,6 @@ test('#ids', function (t) {
t.end();
});

test('#toString()', function (t) {
var trans = new Transaction(agent);
t.strictEqual(
trans.toString(),
`trace.id=${trans.traceId} transaction.id=${trans.id}`,
);
t.end();
});

test('Transaction API on ended transaction', function (t) {
// Enable breakdown metrics to test it below.
agent._config({ breakdownMetrics: true, metricsInterval: '30s' });
Expand Down Expand Up @@ -719,11 +710,6 @@ test('Transaction API on ended transaction', function (t) {
{ 'trace.id': traceId, 'transaction.id': transId },
'trans.ids',
);
t.equal(
trans.toString(), // deprecated
`trace.id=${traceId} transaction.id=${transId}`,
trans.toString(),
);

// We just want to ensure that these Transaction API methods don't throw.
// Whether they make field changes after the transaction has ended isn't
Expand Down

0 comments on commit cd90bde

Please sign in to comment.