Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: drop trans.subtype and trans.action #3557

Merged
merged 5 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG4.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
* Change `apm.startTransaction()` api to return a noop transaction instead of
null. ({issues}2429[#2429])

* Remove `transaction.subtype` and `transaction.action` properties from API.
This also impacts <<apm-start-transaction>> and `transaction.setType(...)`,
both of which now no longer accept `subtype` and `action` parameters.
These two properties were deprecated in v3.25.0.

[float]
===== Features

Expand Down
15 changes: 3 additions & 12 deletions docs/agent-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -519,10 +519,11 @@ app.listen(3000)
NOTE: `apm.middleware.connect` _must_ be added to the middleware stack _before_ any other error handling middleware functions or there's a chance that the error will never get to the agent.

[[apm-start-transaction]]
==== `apm.startTransaction([name][, type][, subtype][, action][, options])`
==== `apm.startTransaction([name][, type][, options])`

[small]#Added in: v0.1.0# +
[small]#Transaction `subtype` and `action` deprecated in: v3.25.0#
[small]#Transaction `subtype` and `action` deprecated in: v3.25.0# +
[small]#Transaction `subtype` and `action` removed in: v4.0.0#

* `name` +{type-string}+ The name of the transaction.
You can always set this later via <<transaction-name,`transaction.name`>> or <<apm-set-transaction-name,`apm.setTransactionName()`>>.
Expand All @@ -531,16 +532,6 @@ You can always set this later via <<transaction-name,`transaction.name`>> or <<a
* `type` +{type-string}+ The type of the transaction.
You can always set this later via <<transaction-type,`transaction.type`>>.

* `subtype` +{type-string}+ The subtype of the transaction.
You can alternatively set this via <<transaction-subtype,`transaction.subtype`>>.
The transaction `subtype` field is deprecated: it is not used and will be
removed in the next major version.

* `action` +{type-string}+ The action of the transaction.
You can alternatively set this via <<transaction-action,`transaction.action`>>.
The transaction `action` field is deprecated: it is not used and will be removed
in the next major version.

* `options` +{type-object}+ The following options are supported:

** `startTime` +{type-number}+ The time when the transaction started.
Expand Down
10 changes: 9 additions & 1 deletion docs/upgrade-to-v4.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,22 @@ start options.
==== API changes

[[v4-api-start-transaction]]
===== `apm.startTransaction()`
===== `agent.startTransaction(...)`

The `apm.startTransaction()` method has been changed to return a do-nothing
no-op Transaction, if the agent is not yet started. The return type has changed to
no longer include `| null`. The intent of these changes is to allow the user to use
`.startTransaction()` without having to worry if the agent is yet started, nor to
have to handle a possible `null` return value.

[[v4-api-transaction-subtype-action]]
===== `transaction.subtype` and `transaction.action`

The `subtype` and `action` properties have been removed from `Transaction`.
This also impacts <<apm-start-transaction>> and `transaction.setType(...)`,
both of which now no longer accept `subtype` and `action` parameters.
These two properties were deprecated in v3.25.0.

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

Expand Down
29 changes: 1 addition & 28 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,25 +67,6 @@ declare namespace apm {
type: string | null,
options?: TransactionOptions
): Transaction | null;
/**
* @deprecated Transaction 'subtype' is not used.
*/
startTransaction(
name: string | null,
type: string | null,
subtype: string | null,
options?: TransactionOptions
): Transaction | null;
/**
* @deprecated Transaction 'subtype' and 'action' are not used.
*/
startTransaction(
name: string | null,
type: string | null,
subtype: string | null,
action: string | null,
options?: TransactionOptions
): Transaction;
setTransactionName (name: string): void;
endTransaction (result?: string | number, endTime?: number): void;
currentTransaction: Transaction | null;
Expand Down Expand Up @@ -154,14 +135,6 @@ declare namespace apm {

name: string;
type: string | null;
/**
* @deprecated Transaction 'subtype' is not used.
*/
subtype: string | null;
/**
* @deprecated Transaction 'action' is not used.
*/
action: string | null;
traceparent: string;
outcome: Outcome;
result: string | number;
Expand All @@ -170,7 +143,7 @@ declare namespace apm {
'transaction.id': string;
}

setType (type?: string | null, subtype?: string | null, action?: string | null): void;
setType (type?: string | null): void;
setLabel (name: string, value: LabelValue, stringify?: boolean): boolean;
addLabels (labels: Labels, stringify?: boolean): boolean;
setOutcome(outcome: Outcome): void;
Expand Down
2 changes: 0 additions & 2 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,6 @@ Agent.prototype.clearPatches = function (modules) {
Agent.prototype.startTransaction = function (
name,
type,
subtype,
action,
{ startTime, childOf } = {},
) {
return this._instrumentation.startTransaction.apply(
Expand Down
27 changes: 8 additions & 19 deletions lib/instrumentation/generic-span.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@ const { TraceParent } = require('../tracecontext/traceparent');

module.exports = GenericSpan;

function GenericSpan(agent, ...args) {
const opts =
typeof args[args.length - 1] === 'object' ? args.pop() || {} : {};

/**
* GenericSpan is an internal base class for Span and Transaction.
*
* @param {Agent} agent
* @param {Object} opts
* TODO: document supported opts
*/
function GenericSpan(agent, opts) {
this._timer = new Timer(opts.timer, opts.startTime);

this._context = TraceContext.startOrResume(
Expand Down Expand Up @@ -64,11 +68,6 @@ function GenericSpan(agent, ...args) {
// from the API and allows a span to keep its unknown status
// even if it succesfully ends.
this._isOutcomeFrozen = false;

this.type = null;
this.subtype = null;
this.action = null;
this.setType(...args);
}

Object.defineProperty(GenericSpan.prototype, 'id', {
Expand Down Expand Up @@ -193,16 +192,6 @@ GenericSpan.prototype._addLinks = function (links) {
}
};

GenericSpan.prototype.setType = function (
type = null,
subtype = null,
action = null,
) {
this.type = type || constants.DEFAULT_SPAN_TYPE;
this.subtype = subtype;
this.action = action;
};

GenericSpan.prototype._freezeOutcome = function () {
this._isOutcomeFrozen = true;
};
Expand Down
2 changes: 0 additions & 2 deletions lib/instrumentation/noop-transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ class NoopTransaction {
constructor() {
this.name = 'unnamed';
this.type = 'noop';
this.subtype = undefined;
this.action = undefined;
this.traceparent = NOOP_TRACEPARENT;
this.result = constants.RESULT_SUCCESS;
this.outcome = constants.OUTCOME_UNKNOWN;
Expand Down
13 changes: 12 additions & 1 deletion lib/instrumentation/span.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ function Span(transaction, ...args) {

delete opts.exitSpan;

GenericSpan.call(this, transaction._agent, ...tsaArgs, opts);
this.type = null;
this.subtype = null;
this.action = null;
this.setType(...tsaArgs);

GenericSpan.call(this, transaction._agent, opts);

this._db = null;
this._http = null;
Expand Down Expand Up @@ -88,6 +93,12 @@ Object.defineProperty(Span.prototype, 'ids', {
},
});

Span.prototype.setType = function (type = null, subtype = null, action = null) {
this.type = type || constants.DEFAULT_SPAN_TYPE;
this.subtype = subtype;
this.action = action;
};

/*
* A string representation of the span to help with internal debugging. This
* is not a promised interface.
Expand Down
17 changes: 9 additions & 8 deletions lib/instrumentation/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,11 @@ util.inherits(Transaction, GenericSpan);
// new Transaction(agent, opts?)
// new Transaction(agent, name, opts?)
// new Transaction(agent, name, type?, opts?)
// new Transaction(agent, name, type?, subtype?, opts?)
// new Transaction(agent, name, type?, subtype?, action?, opts?)
//
// @param {Agent} agent
// @param {string} [name]
// @param {string} [type] - Defaults to 'custom' when serialized.
// @param {string} [subtype] - Deprecated. Unused.
// @param {string} [action] - Deprecated. Unused.
// @param {string} [opts]
// @param {Object} [opts]
// - opts.childOf - Used to determine the W3C trace-context trace id, parent
// id, and sampling information for this new transaction. This currently
// accepts a Transaction instance, Span instance, TraceParent instance, or
Expand Down Expand Up @@ -82,7 +78,10 @@ function Transaction(agent, name, ...args) {
}
}

GenericSpan.call(this, agent, ...args, opts);
this.type = null;
this.setType(...args);

GenericSpan.call(this, agent, opts);

const verb = this.parentId ? 'continue' : 'start';
agent.logger.debug('%s trace %o', verb, {
Expand All @@ -91,8 +90,6 @@ function Transaction(agent, name, ...args) {
trace: this.traceId,
name: this.name,
type: this.type,
subtype: this.subtype,
action: this.action,
});

this._defaultName = name || '';
Expand Down Expand Up @@ -173,6 +170,10 @@ Object.defineProperty(Transaction.prototype, 'ids', {
},
});

Transaction.prototype.setType = function (type = null) {
this.type = type || constants.DEFAULT_SPAN_TYPE;
};

/*
* A string representation of the transaction to help with internal debugging.
* This is not a promised interface.
Expand Down
6 changes: 2 additions & 4 deletions test/agent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,17 +208,15 @@ test('#startTransaction()', function (t) {
},
);

t.test('name, type, subtype and action', function (t) {
t.test('name, type', function (t) {
const agent = new Agent().start(agentOptsNoopTransport);
var trans = agent.startTransaction('foo', 'type', 'subtype', 'action');
var trans = agent.startTransaction('foo', 'type');
t.ok(
!(trans instanceof NoopTransaction),
'agent retuns a real transaction',
);
t.strictEqual(trans.name, 'foo');
t.strictEqual(trans.type, 'type');
t.strictEqual(trans.subtype, 'subtype');
t.strictEqual(trans.action, 'action');
agent.destroy();
t.end();
});
Expand Down
6 changes: 2 additions & 4 deletions test/instrumentation/transaction.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ test('#duration() - un-ended transaction', function (t) {

test('custom start time', function (t) {
var startTime = Date.now() - 1000;
var trans = new Transaction(agent, null, null, { startTime });
var trans = new Transaction(agent, { startTime });
trans.end();

var duration = trans.duration();
Expand All @@ -325,7 +325,7 @@ test('custom start time', function (t) {
test('#end(time)', function (t) {
var startTime = Date.now() - 1000;
var endTime = startTime + 2000.123;
var trans = new Transaction(agent, null, null, { startTime });
var trans = new Transaction(agent, { startTime });
trans.end(null, endTime);

t.strictEqual(trans.duration(), 2000.123);
Expand Down Expand Up @@ -696,8 +696,6 @@ test('Transaction API on ended transaction', function (t) {
// behaves as expected on an ended transaction.
t.equal(trans.name, 'theTransName', 'trans.name');
t.equal(trans.type, 'theTransType', 'trans.type');
t.equal(trans.subtype, null, 'trans.subtype');
t.equal(trans.action, null, 'trans.action');
t.equal(
trans.traceparent,
traceparentBefore,
Expand Down
4 changes: 0 additions & 4 deletions test/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,8 @@ apm.captureError(new Error('an error with explicitly no parent'), { parent: null
apm.startTransaction()
apm.startTransaction('foo')
apm.startTransaction('foo', 'type')
apm.startTransaction('foo', 'type', 'subtype')
apm.startTransaction('foo', 'type', 'subtype', 'action')
apm.startTransaction('foo', { startTime: 1 })
apm.startTransaction('foo', 'type', { startTime: 1 })
apm.startTransaction('foo', 'type', 'subtype', { startTime: 1 })
apm.startTransaction('foo', 'type', 'subtype', 'action', { startTime: 1 })
apm.startTransaction('foo', { childOf: '00-12345678901234567890123456789012-1234567890123456-01' })
apm.startTransaction('foo', { tracestate: 'foo=42,bar=43' })
apm.startTransaction('foo', { links: [{ context: '00-12345678901234567890123456789012-1234567890123456-01' }] })
Expand Down
Loading