Skip to content

Commit

Permalink
fix(core): resolved flooding log file when agent connection is not es…
Browse files Browse the repository at this point in the history
…tablished (#850)

refs #849
  • Loading branch information
kirrg001 authored Aug 24, 2023
1 parent 7444a90 commit c80eca6
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 61 deletions.
6 changes: 5 additions & 1 deletion packages/core/src/tracing/cls.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,11 @@ function skipExitTracing(options) {
}

if (!opts.skipParentSpanCheck && (!parentSpan || isExitSpanResult)) {
if (opts.log) {
// NOTE: We need to check for `isActive` otherwise we flood this warning in case the collector is not
// yet connected to the agent, but the application receives traffic already
// The underlying problem is that all instrumentations are already "working" before the collector
// is successfully connected with the agent, but they are skipped with the `isActive` flag.
if (opts.log && opts.isActive) {
logger.warn(
// eslint-disable-next-line max-len
`Cannot start an exit span as this requires an active entry (or intermediate) span as parent. ${
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ const operations = Object.keys(operationsInfo);
const SPAN_NAME = 'dynamodb';

class InstanaAWSDynamoDB extends InstanaAWSProduct {
instrumentedMakeRequest(ctx, originalMakeRequest, originalArgs) {
// NOTE: `shimMakeRequest` in index.js is already checking the result of `isActive`
if (cls.skipExitTracing()) {
instrumentedMakeRequest(ctx, isActive, originalMakeRequest, originalArgs) {
if (cls.skipExitTracing({ isActive })) {
return originalMakeRequest.apply(ctx, originalArgs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,17 @@ function instrumentAWS(AWS) {

function shimMakeRequest(originalMakeRequest) {
return function () {
if (isActive) {
const originalArgs = new Array(arguments.length);
for (let i = 0; i < originalArgs.length; i++) {
originalArgs[i] = arguments[i];
}
const originalArgs = new Array(arguments.length);
for (let i = 0; i < originalArgs.length; i++) {
originalArgs[i] = arguments[i];
}

const awsProduct = operationMap[originalArgs[0]];
const awsProduct = operationMap[originalArgs[0]];

// to match operation + reference (S3, Dynamo, etc)
if (awsProduct) {
return awsProduct.instrumentedMakeRequest(this, originalMakeRequest, originalArgs);
}
return originalMakeRequest.apply(this, originalArgs);
// to match operation + reference (S3, Dynamo, etc)
if (awsProduct) {
return awsProduct.instrumentedMakeRequest(this, isActive, originalMakeRequest, originalArgs);
}

return originalMakeRequest.apply(this, arguments);
return originalMakeRequest.apply(this, originalArgs);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ const operationsInfo = {
const operations = Object.keys(operationsInfo);

class InstanaAWSKinesis extends InstanaAWSProduct {
instrumentedMakeRequest(ctx, originalMakeRequest, originalArgs) {
instrumentedMakeRequest(ctx, isActive, originalMakeRequest, originalArgs) {
const self = this;

// NOTE: `shimMakeRequest` in index.js is already checking the result of `isActive`
if (cls.skipExitTracing()) {
if (cls.skipExitTracing({ isActive })) {
return originalMakeRequest.apply(ctx, originalArgs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,10 @@ class InstanaAWSLambda extends InstanaAWSProduct {
}
}

instrumentedMakeRequest(ctx, originalMakeRequest, originalArgs) {
instrumentedMakeRequest(ctx, isActive, originalMakeRequest, originalArgs) {
const self = this;
const skipTracingResult = cls.skipExitTracing({ extendedResponse: true });
const skipTracingResult = cls.skipExitTracing({ isActive, extendedResponse: true });

// NOTE: `shimMakeRequest` in index.js is already checking the result of `isActive`
if (skipTracingResult.skip) {
if (skipTracingResult.suppressed) {
this.propagateInstanaHeaders(originalArgs, null, skipTracingResult.suppressed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,10 @@ class InstanaAWSS3 extends InstanaAWSProduct {
/**
* Original args for ALL AWS SDK Requets: [method, params, callback]
*/
instrumentedMakeRequest(ctx, originalMakeRequest, originalArgs) {
instrumentedMakeRequest(ctx, isActive, originalMakeRequest, originalArgs) {
const self = this;

// NOTE: `shimMakeRequest` in index.js is already checking the result of `isActive`
if (cls.skipExitTracing()) {
if (cls.skipExitTracing({ isActive })) {
return originalMakeRequest.apply(ctx, originalArgs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ const operations = Object.keys(operationsInfo);
const SPAN_NAME = 'sns';

class InstanaAWSSNS extends InstanaAWSProduct {
instrumentedMakeRequest(ctx, originalMakeRequest, originalArgs) {
instrumentedMakeRequest(ctx, isActive, originalMakeRequest, originalArgs) {
const messageBody = originalArgs[1];

if (!messageBody.MessageAttributes) {
messageBody.MessageAttributes = {};
}

const skipTracingResult = cls.skipExitTracing({ isActive: true, extendedResponse: true });
const skipTracingResult = cls.skipExitTracing({ isActive, extendedResponse: true });
if (skipTracingResult.skip) {
if (skipTracingResult.suppressed) {
this.propagateSuppression(messageBody.MessageAttributes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ const operations = Object.keys(operationsInfo);
const SPAN_NAME = 'dynamodb';

class InstanaAWSDynamoDB extends InstanaAWSProduct {
instrumentedSmithySend(ctx, originalSend, smithySendArgs) {
// NOTE: `shimSmithySend` in index.js is already checking the result of `isActive`
if (cls.skipExitTracing()) {
instrumentedSmithySend(ctx, isActive, originalSend, smithySendArgs) {
if (cls.skipExitTracing({ isActive })) {
return originalSend.apply(ctx, smithySendArgs);
}

Expand Down
38 changes: 18 additions & 20 deletions packages/core/src/tracing/instrumentation/cloud/aws-sdk/v3/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ exports.isActive = function () {
return isActive;
};

// NOTE: you currently can only disable the whole AWS SDK v3
exports.activate = function activate() {
isActive = true;
};
Expand All @@ -74,27 +75,24 @@ function shimSmithySend(originalSend) {
const smithySendArgs = getFunctionArguments(arguments);
const command = smithySendArgs[0];

if (isActive) {
const serviceId = self.config && self.config.serviceId;
let awsProduct = serviceId && awsProducts.find(aws => aws.spanName === serviceId.toLowerCase());

if (awsProduct && awsProduct.supportsOperation(command.constructor.name)) {
return awsProduct.instrumentedSmithySend(self, originalSend, smithySendArgs);
} else {
// This code can be removed once all AWS SDK v3 instrumentations have been refactored to use the new approach
// introduced in https://github.com/instana/nodejs/pull/838 for kinesis. That is: Do not use an explicit
// operationsInfo/operationsMap map that restricts the traced operations to a subset of possible operations, but
// instead allow _all_ operations to be traced, using the operation name from `command.constructor.name` for
// span.data.$spanName.op. We plan to finish this refactoring before or with the next major release (3.x) of the
// @instana packages.
awsProduct = operationMap[smithySendArgs[0].constructor.name];
if (awsProduct) {
return awsProduct.instrumentedSmithySend(this, originalSend, smithySendArgs);
}

return originalSend.apply(this, smithySendArgs);
const serviceId = self.config && self.config.serviceId;
let awsProduct = serviceId && awsProducts.find(aws => aws.spanName === serviceId.toLowerCase());

if (awsProduct && awsProduct.supportsOperation(command.constructor.name)) {
return awsProduct.instrumentedSmithySend(self, isActive, originalSend, smithySendArgs);
} else {
// This code can be removed once all AWS SDK v3 instrumentations have been refactored to use the new approach
// introduced in https://github.com/instana/nodejs/pull/838 for kinesis. That is: Do not use an explicit
// operationsInfo/operationsMap map that restricts the traced operations to a subset of possible operations, but
// instead allow _all_ operations to be traced, using the operation name from `command.constructor.name` for
// span.data.$spanName.op. We plan to finish this refactoring before or with the next major release (3.x) of the
// @instana packages.
awsProduct = operationMap[smithySendArgs[0].constructor.name];
if (awsProduct) {
return awsProduct.instrumentedSmithySend(self, isActive, originalSend, smithySendArgs);
}

return originalSend.apply(self, smithySendArgs);
}
return originalSend.apply(self, smithySendArgs);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ const { InstanaAWSProduct } = require('./instana_aws_product');
const SPAN_NAME = 'kinesis';

class InstanaAWSKinesis extends InstanaAWSProduct {
instrumentedSmithySend(ctx, originalSend, smithySendArgs) {
if (cls.skipExitTracing()) {
instrumentedSmithySend(ctx, isActive, originalSend, smithySendArgs) {
if (cls.skipExitTracing({ isActive })) {
return originalSend.apply(ctx, smithySendArgs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ const operations = Object.keys(operationsInfo);
const SPAN_NAME = 's3';

class InstanaAWSS3 extends InstanaAWSProduct {
instrumentedSmithySend(ctx, originalSend, smithySendArgs) {
// NOTE: `shimSmithySend` in index.js is already checking the result of `isActive`
if (cls.skipExitTracing()) {
instrumentedSmithySend(ctx, isActive, originalSend, smithySendArgs) {
if (cls.skipExitTracing({ isActive })) {
return originalSend.apply(ctx, smithySendArgs);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,20 @@ class InstanaAWSSQS extends InstanaAWSProduct {
});
}

instrumentedSmithySend(ctx, originalSend, smithySendArgs) {
instrumentedSmithySend(ctx, isActive, originalSend, smithySendArgs) {
const commandName = smithySendArgs[0].constructor.name;
const operation = operationsInfo[commandName];

if (operation && operation.sort === 'exit') {
return this.instrumentExit(ctx, originalSend, smithySendArgs, operation);
return this.instrumentExit(ctx, isActive, originalSend, smithySendArgs, operation);
} else if (operation && operation.sort === 'entry') {
return this.instrumentEntry(ctx, originalSend, smithySendArgs, operation);
}

return originalSend.apply(ctx, smithySendArgs);
}

instrumentExit(ctx, originalSend, smithySendArgs, operation) {
instrumentExit(ctx, isActive, originalSend, smithySendArgs, operation) {
const command = smithySendArgs[0];
const sendMessageInput = command.input;

Expand Down Expand Up @@ -109,8 +109,7 @@ class InstanaAWSSQS extends InstanaAWSProduct {
attributes = sendMessageInput.MessageAttributes = {};
}

// NOTE: `shimSmithySend` in index.js is already checking the result of `isActive`
const skipTracingResult = cls.skipExitTracing({ extendedResponse: true });
const skipTracingResult = cls.skipExitTracing({ extendedResponse: true, isActive });

if (skipTracingResult.skip) {
if (skipTracingResult.suppressed) {
Expand Down

0 comments on commit c80eca6

Please sign in to comment.