-
Notifications
You must be signed in to change notification settings - Fork 36
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
Detect and warn if duplicate lambda libraries are detected #585
base: main
Are you sure you want to change the base?
Changes from all commits
7d54dcc
97937ec
e8c85e9
5c57143
fed16d1
a2939bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -21,11 +21,14 @@ import { | |||||||||||||||||||||||||||||||||||
setSandboxInit, | ||||||||||||||||||||||||||||||||||||
setLogger, | ||||||||||||||||||||||||||||||||||||
setLogLevel, | ||||||||||||||||||||||||||||||||||||
logWarning, | ||||||||||||||||||||||||||||||||||||
} from "./utils"; | ||||||||||||||||||||||||||||||||||||
import { getEnhancedMetricTags } from "./metrics/enhanced-metrics"; | ||||||||||||||||||||||||||||||||||||
import { DatadogTraceHeaders } from "./trace/context/extractor"; | ||||||||||||||||||||||||||||||||||||
import { SpanWrapper } from "./trace/span-wrapper"; | ||||||||||||||||||||||||||||||||||||
import { SpanOptions, TracerWrapper } from "./trace/tracer-wrapper"; | ||||||||||||||||||||||||||||||||||||
import path from "path"; | ||||||||||||||||||||||||||||||||||||
import fs from "fs"; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// Backwards-compatible export, TODO deprecate in next major | ||||||||||||||||||||||||||||||||||||
export { DatadogTraceHeaders as TraceHeaders } from "./trace/context/extractor"; | ||||||||||||||||||||||||||||||||||||
|
@@ -104,6 +107,9 @@ export const _metricsQueue: MetricsQueue = new MetricsQueue(); | |||||||||||||||||||||||||||||||||||
let currentMetricsListener: MetricsListener | undefined; | ||||||||||||||||||||||||||||||||||||
let currentTraceListener: TraceListener | undefined; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const LAMBDA_LAYER_PATH = "/opt/nodejs/node_modules/datadog-lambda-js"; | ||||||||||||||||||||||||||||||||||||
const LAMBDA_LIBRARY_PATH = path.join(process.cwd(), "node_modules/datadog-lambda-js"); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (getEnvValue(coldStartTracingEnvVar, "true").toLowerCase() === "true") { | ||||||||||||||||||||||||||||||||||||
subscribeToDC(); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
@@ -131,6 +137,19 @@ export function datadog<TEvent, TResult>( | |||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const traceListener = new TraceListener(finalConfig); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// Check for duplicate installations of the Lambda library | ||||||||||||||||||||||||||||||||||||
detectDuplicateInstallations() | ||||||||||||||||||||||||||||||||||||
.then((duplicateFound) => { | ||||||||||||||||||||||||||||||||||||
if (duplicateFound) { | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, this code might be relevant, so wanted to share for reviewers: datadog-lambda-js/src/trace/tracer-wrapper.ts Lines 28 to 44 in f05c600
Note that this code only loads in one version of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wonder if there's a way for us to use similar logic like this one 🤔 |
||||||||||||||||||||||||||||||||||||
logWarning( | ||||||||||||||||||||||||||||||||||||
`Detected duplicate installations of datadog-lambda-js. This can cause: (1) increased cold start times, (2) broken metrics, and (3) other unexpected behavior. Please use either the Lambda layer version or the package in node_modules, but not both. See: https://docs.datadoghq.com/serverless/aws_lambda/installation/nodejs/?tab=custom`, | ||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||
.catch(() => { | ||||||||||||||||||||||||||||||||||||
logDebug("Failed to check for duplicate installations."); | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// Only wrap the handler once unless forced | ||||||||||||||||||||||||||||||||||||
const _ddWrappedKey = "_ddWrapped"; | ||||||||||||||||||||||||||||||||||||
if ((handler as any)[_ddWrappedKey] !== undefined && !finalConfig.forceWrap) { | ||||||||||||||||||||||||||||||||||||
|
@@ -478,3 +497,26 @@ export async function emitTelemetryOnErrorOutsideHandler( | |||||||||||||||||||||||||||||||||||
await metricsListener.onCompleteInvocation(); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
async function detectDuplicateInstallations() { | ||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||
const checkPathExistsAsync = async (libraryPath: string): Promise<boolean> => { | ||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||
await fs.promises.access(libraryPath); | ||||||||||||||||||||||||||||||||||||
return true; | ||||||||||||||||||||||||||||||||||||
} catch { | ||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const [layerExists, localExists] = await Promise.all([ | ||||||||||||||||||||||||||||||||||||
checkPathExistsAsync(LAMBDA_LAYER_PATH), | ||||||||||||||||||||||||||||||||||||
checkPathExistsAsync(LAMBDA_LIBRARY_PATH), | ||||||||||||||||||||||||||||||||||||
]); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
return layerExists && localExists; | ||||||||||||||||||||||||||||||||||||
} catch (err) { | ||||||||||||||||||||||||||||||||||||
logDebug("Failed to check for duplicate installations."); | ||||||||||||||||||||||||||||||||||||
return false; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this under the
datadog()
wrapper, but I'm not sure if this is the best place to make this call. If there's somewhere that's better, I'm open to suggestions!