From 0886249af363b5ed1c51954eff3e9263b9117dc3 Mon Sep 17 00:00:00 2001 From: Jackson Weber <47067795+JacksonWeber@users.noreply.github.com> Date: Fri, 2 Feb 2024 15:02:43 -0800 Subject: [PATCH] [Monitor OpenTelemetry Exporter] Add Exception Handling to File Name for Telemetry Caching (#28399) ### Packages impacted by this PR @azure/monitor-opentelemetry-exporter ### Issues associated with this PR https://github.com/microsoft/ApplicationInsights-node.js/issues/1230 ### Describe the problem that is addressed by this PR Append the process ID to the file name created for holding disk cached telemetry. This should resolve the issue with multiple Azure Functions cores attempting to read/write/delete the same file when functions are scaled to use multiple cores. Extended this logic outside of Azure Functions so that in any case where the SDK could be run concurrently we create distinct cache files. ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [x] Added a changelog (if necessary) --- .../CHANGELOG.md | 10 ++++++++ .../nodejs/persist/fileSystemHelpers.ts | 25 +++++++++++-------- .../test/internal/fileSystemPersist.test.ts | 4 ++- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/sdk/monitor/monitor-opentelemetry-exporter/CHANGELOG.md b/sdk/monitor/monitor-opentelemetry-exporter/CHANGELOG.md index 5f916b240c1d..74d6975b2e33 100644 --- a/sdk/monitor/monitor-opentelemetry-exporter/CHANGELOG.md +++ b/sdk/monitor/monitor-opentelemetry-exporter/CHANGELOG.md @@ -1,5 +1,15 @@ # Release History +## 1.0.0-beta.20 () + +### Features Added + +### Bugs Fixed + +- Added exception handling for reading files to avoid concurrency errors. + +### Other Changes + ## 1.0.0-beta.19 (2024-01-23) ### Features Added diff --git a/sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileSystemHelpers.ts b/sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileSystemHelpers.ts index 25420d347669..73ac7e6b0e23 100644 --- a/sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileSystemHelpers.ts +++ b/sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileSystemHelpers.ts @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import { diag } from "@opentelemetry/api"; import * as fs from "fs"; import * as path from "path"; import { promisify } from "util"; @@ -15,20 +16,24 @@ const mkdirAsync = promisify(fs.mkdir); * @internal */ export const getShallowDirectorySize = async (directory: string): Promise => { - // Get the directory listing - const files = await readdirAsync(directory); - let totalSize = 0; + try { + // Get the directory listing + const files = await readdirAsync(directory); - // Query all file sizes - for (const file of files) { - const fileStats = await statAsync(path.join(directory, file)); - if (fileStats.isFile()) { - totalSize += fileStats.size; + // Query all file sizes + for (const file of files) { + const fileStats = await statAsync(path.join(directory, file)); + if (fileStats.isFile()) { + totalSize += fileStats.size; + } } - } - return totalSize; + return totalSize; + } catch (err) { + diag.error(`Error getting directory size: ${err}`); + return 0; + } }; /** diff --git a/sdk/monitor/monitor-opentelemetry-exporter/test/internal/fileSystemPersist.test.ts b/sdk/monitor/monitor-opentelemetry-exporter/test/internal/fileSystemPersist.test.ts index 6f82d4183bee..f8c20a9e2564 100644 --- a/sdk/monitor/monitor-opentelemetry-exporter/test/internal/fileSystemPersist.test.ts +++ b/sdk/monitor/monitor-opentelemetry-exporter/test/internal/fileSystemPersist.test.ts @@ -46,7 +46,9 @@ const assertFirstFile = async (tempDir: string, expectation: unknown): Promise path.basename(f).includes(".ai.json")); + const files = origFiles.filter((f) => + path.basename(f).includes(FileSystemPersist.FILENAME_SUFFIX), + ); assert.ok(files.length > 0); // Assert file matches expectation