Skip to content

refactor: updated sdk to use setErrorDetails fn #2222

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.

Sign up for GitHub

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

Merged
abhilash-sivan merged 3 commits into from
Dec 10, 2025
Merged

refactor: updated sdk to use setErrorDetails fn #2222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77cbf81
refactor: updated treacingutil to allow custom key path for sdk
abhilash-sivan Dec 9, 2025
27cf79a
refactor: updated sdk to use setErrorDetails function
abhilash-sivan Dec 10, 2025
b62f51a
test: removed unused fn getErrorDetails
abhilash-sivan Dec 10, 2025
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
24 changes: 19 additions & 5 deletions packages/collector/test/tracing/sdk/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,8 +710,7 @@ mochaSuiteFn('tracing/sdk', function () {
if (error) {
expectations = expectations.concat([
span => expect(span.ec).to.equal(1),
span => expect(span.data.sdk.custom.tags.message).to.contain('Error: Boom!\n'),
span => expect(span.data.sdk.custom.tags.message).to.contain('packages/collector/test/tracing/sdk/app.js')
Copy link
Contributor Author

@abhilash-sivan abhilash-sivan Dec 10, 2025

Choose a reason for hiding this comment

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

Removed this check because earlier it was coming from getErrorDetails and this was substring of error.stack. Now this is only error.message and thus the app path is not available

span => expect(span.data.sdk.custom.tags.message).to.contain('Error: Boom!')
]);
} else {
expectations.push(span => expect(span.ec).to.equal(0));
Expand All @@ -721,8 +720,17 @@ mochaSuiteFn('tracing/sdk', function () {
span => expect(span.data.sdk).to.exist,
span => expect(span.data.sdk.name).to.equal('custom-entry'),
span => expect(span.data.sdk.type).to.equal(constants.SDK.ENTRY),
span => expect(span.stack[0].c).to.match(/test\/tracing\/sdk\/app.js$/),
span => expect(span.stack[0].m).to.match(functionName)
span => {
// TODO: Remove this !error condition when the span.stack to array conversion is implemented
if (functionName && !error) {
// When there's an error, span.stack is overwritten with error stack string, not stack frames array
expect(span.stack[0].c).to.match(/test\/tracing\/sdk\/app.js$/);
expect(span.stack[0].m).to.match(functionName);
} else if (error) {
// For errors, span.stack is a string containing the error stack trace
expect(span.stack).to.be.a('string');
}
}
]);

tagsAt = tagsAt || 'none';
Expand Down Expand Up @@ -828,9 +836,15 @@ mochaSuiteFn('tracing/sdk', function () {
: expect(span.data.sdk.name).to.equal(kind === 'INTERMEDIATE' ? 'intermediate-file-access' : 'file-access'),
span => expect(span.data.sdk.type).to.equal(constants.SDK[kind]),
span => {
if (functionName) {
// TODO: Remove this !error condition when the span.stack to array conversion is implemented
if (functionName && !error) {
// When there's an error, span.stack is overwritten with error stack string, not stack frames array
expect(span.stack[0].c).to.match(/test\/tracing\/sdk\/app.js$/);
expect(span.stack[0].m).to.match(functionName);
} else if (error) {
// For errors, span.stack is a string containing the error stack trace
expect(span.stack).to.be.a('string');
expect(span.stack).to.contain('Error: ENOENT');
}
},
span => expect(span.data.sdk.custom).to.be.an('object'),
Expand Down
16 changes: 3 additions & 13 deletions packages/core/src/tracing/sdk/sdk.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,19 +321,9 @@ exports.generate = function (isCallbackApi) {

if (error) {
span.ec = 1;
if (!span.data.sdk.custom) {
span.data.sdk.custom = {
tags: {}
};
}
if (!span.data.sdk.custom.tags) {
span.data.sdk.custom.tags = {};
}
if (span.data.sdk.custom.tags.message == null) {
// TODO: fix sdk error capture and handling
// This fn used getErrorDetails for now and will need to change
span.data.sdk.custom.tags.message = tracingUtil.getErrorDetails(error);
}
// since the key for error message is different in sdk, we pass the complete path as dot(.) separated string
// also it can be passed as an array like ["sdk", "custom", "tags", "message"]
tracingUtil.setErrorDetails(span, error, 'sdk.custom.tags.message');
}

if (span.data.sdk.custom && tags) {
Expand Down
72 changes: 47 additions & 25 deletions packages/core/src/tracing/tracingUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,6 @@ exports.renderTraceContextToBuffer = function renderTraceContextToBuffer(span) {
return exports.unsignedHexStringsToBuffer(span.t, span.s);
};

/**
* @param {Error} err
* @returns {string}
*/
exports.getErrorDetails = function getErrorDetails(err) {
if (err == null) {
return undefined;
}
return String(err.stack || err.message || err).substring(0, 500);
};

/**
* @param {string} stmt
* @returns {string}
Expand Down Expand Up @@ -278,9 +267,18 @@ exports.findCallback = (/** @type {string | any[]} */ originalArgs) => {
};

/**
* Sets error details on a span for a specific technology.
* Handles different error formats: strings, objects with details/message/code properties.
* Supports nested paths for SDK spans via dot-separated strings or arrays.
*
* Examples:
* - setErrorDetails(span, error, 'nats') // flat key
* - setErrorDetails(span, error, 'sdk.custom.tags.message') // dot-separated string
* - setErrorDetails(span, error, ['sdk', 'custom', 'tags', 'message']) // array path
*
* @param {import('../core').InstanaBaseSpan} span - The span to update
* @param {Error | string} error
* @param {string} technology - The technology name (e.g., 'mysql', 'pg', 'http')
* @param {Error | string | Object} error - The error object, error string, or object with error properties
* @param {string | Array<string>} technology - The technology name or nested path
*/
exports.setErrorDetails = function setErrorDetails(span, error, technology) {
if (!error) {
Expand All @@ -297,20 +295,44 @@ exports.setErrorDetails = function setErrorDetails(span, error, technology) {
normalizedError = error;
}

if (technology && span.data?.[technology]) {
// This extra check allows instrumentations to override the error property (not recommended)
if (!span.data[technology].error) {
let combinedMessage;

if (normalizedError?.details) {
combinedMessage = `${normalizedError.name || 'Error'}: ${normalizedError.details}`;
} else if (normalizedError?.message) {
combinedMessage = `${normalizedError.name || 'Error'}: ${normalizedError.message}`;
} else {
combinedMessage = normalizedError?.code || 'No error message found.';
const extractErrorMessage = () => {
if (normalizedError?.details) {
return `${normalizedError.name || 'Error'}: ${normalizedError.details}`;
} else if (normalizedError?.message) {
return `${normalizedError.name || 'Error'}: ${normalizedError.message}`;
} else {
return normalizedError?.code || 'No error message found.';
}
};

let errorPath = null;
if (Array.isArray(technology)) {
errorPath = technology;
} else if (typeof technology === 'string' && technology.includes('.')) {
errorPath = technology.split('.');
}

if (errorPath) {
let target = span.data;

// Traverse the object path and create missing nested objects along the way
// Without this, deeper properties would fail to assign if their parent objects don't exist
for (let i = 0; i < errorPath.length - 1; i++) {
const key = errorPath[i];
if (!target[key]) {
target[key] = {};
}
target = target[key];
}

const errorKey = errorPath[errorPath.length - 1];

span.data[technology].error = combinedMessage.substring(0, 200);
if (!target[errorKey]) {
target[errorKey] = extractErrorMessage().substring(0, 200);
}
} else if (typeof technology === 'string' && technology && span.data?.[technology]) {
if (!span.data[technology].error) {
span.data[technology].error = extractErrorMessage().substring(0, 200);
}
}

Expand Down
112 changes: 92 additions & 20 deletions packages/core/test/tracing/tracingUtil_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const {
generateRandomId,
generateRandomSpanId,
generateRandomTraceId,
getErrorDetails,
readTraceContextFromBuffer,
sanitizeConnectionStr,
unsignedHexStringToBuffer,
Expand Down Expand Up @@ -357,25 +356,6 @@ describe('tracing/tracingUtil', () => {
}
});

describe('getErrorDetails', () => {
it('must not fail on null/undefined', () => {
expect(getErrorDetails(null)).to.equal(undefined);
expect(getErrorDetails(undefined)).to.equal(undefined);
});

it('must use error stack when available', () => {
expect(getErrorDetails(new Error('Whhoooopppppss'))).to.match(/Whhoooopppppss/);
});

it('must use error message when available', () => {
expect(getErrorDetails({ message: 'Whhoooopppppss' })).to.match(/Whhoooopppppss/);
});

it('must use the whole provided error when all else fails', () => {
expect(getErrorDetails('Whhoooopppppss')).to.match(/Whhoooopppppss/);
});
});

describe('sanitizeConnectionStr', () => {
it('should redact password at the start of the connection string', () => {
expect(
Expand Down Expand Up @@ -567,5 +547,97 @@ describe('tracing/tracingUtil', () => {

expect(span.data.rpc.error).to.contain('UNAVAILABLE: Connection refused');
});

describe('SDK spans with nested error paths', () => {
it('should not overwrite existing SDK error', () => {
const span = {
data: {
sdk: {
custom: {
tags: {
message: 'Existing SDK error'
}
}
}
}
};
const error = 'New SDK error';
setErrorDetails(span, error, ['sdk', 'custom', 'tags', 'message']);

expect(span.data.sdk.custom.tags.message).to.equal('Existing SDK error');
});

it('should truncate SDK error messages to 200 characters', () => {
const span = {
data: {}
};
const longError = 'b'.repeat(600);
setErrorDetails(span, longError, ['sdk', 'custom', 'tags', 'message']);

expect(span.data.sdk.custom.tags.message).to.have.lengthOf(200);
expect(span.data.sdk.custom.tags.message).to.equal(`Error: ${'b'.repeat(193)}`);
});

it('should create nested structure if it does not exist', () => {
const span = {
data: {}
};
const error = 'Test error';
setErrorDetails(span, error, ['sdk', 'custom', 'tags', 'error']);

expect(span.data.sdk).to.be.an('object');
expect(span.data.sdk.custom).to.be.an('object');
expect(span.data.sdk.custom.tags).to.be.an('object');
expect(span.data.sdk.custom.tags.error).to.equal('Error: Test error');
});

it('should handle different nested paths', () => {
const span = {
data: {}
};
const error = 'Custom path error';
setErrorDetails(span, error, ['custom', 'nested', 'path', 'errorField']);

expect(span.data.custom.nested.path.errorField).to.equal('Error: Custom path error');
});

it('should handle Error object with name and message for SDK spans', () => {
const span = {
data: {}
};
const error = new TypeError('Type mismatch');
setErrorDetails(span, error, ['sdk', 'custom', 'tags', 'message']);

expect(span.data.sdk.custom.tags.message).to.match(/TypeError: Type mismatch/);
expect(span.stack).to.match(/Type mismatch/);
});

it('should handle SDK error with dot-separated string path', () => {
const span = {
data: {}
};
const error = new Error('SDK error via string path');
setErrorDetails(span, error, 'sdk.custom.tags.message');

expect(span.data.sdk).to.exist;
expect(span.data.sdk.custom).to.exist;
expect(span.data.sdk.custom.tags).to.exist;
expect(span.data.sdk.custom.tags.message).to.match(/Error: SDK error via string path/);
expect(span.stack).to.be.a('string');
expect(span.stack).to.match(/SDK error via string path/);
});

it('should handle both array and string paths equivalently', () => {
const span1 = { data: {} };
const span2 = { data: {} };
const error = new Error('Test error');

setErrorDetails(span1, error, ['sdk', 'custom', 'tags', 'message']);
setErrorDetails(span2, error, 'sdk.custom.tags.message');

expect(span1.data.sdk.custom.tags.message).to.equal(span2.data.sdk.custom.tags.message);
expect(span1.data.sdk.custom.tags.message).to.match(/Error: Test error/);
});
});
});
});