[OTel-SDK] Merge Core and Common shared packages into otel-core-js#2691
[OTel-SDK] Merge Core and Common shared packages into otel-core-js#2691
Conversation
MSNev
commented
Jan 23, 2026
MSNev
commented
- Also extracts core Noop operations to a new otel-noop-js package
- Required to merge phases 4 and 5 together because it caused a circular dependency issue with the tests of Common as they required Core, but Core required otel-core-js
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| import { cfgDfMerge } from "../Config/ConfigDefaultHelpers"; | ||
| import { createDynamicConfig, onConfigChange } from "../Config/DynamicConfig"; | ||
| import { DiagnosticLogger, _InternalLogMessage, _throwInternal, _warnToConsole } from "../Diagnostics/DiagnosticLogger"; | ||
| import { cfgDfMerge } from "../config/ConfigDefaultHelpers"; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 days ago
In general, to fix an unused import issue, either remove the import or start using the imported symbol in active code. To avoid changing functionality, we prefer removal only when the symbol is not referenced anywhere in live code.
Here, cfgDfMerge is only used in a commented-out configuration (defaultStatsCfg). Since commented code has no effect, the safest and cleanest fix is to remove the cfgDfMerge import from AppInsightsCore.ts. No other code in the shown snippet depends on it, and no additional methods, imports, or definitions are needed.
Concretely, in shared/otel-core/src/core/AppInsightsCore.ts, delete the import line:
import { cfgDfMerge } from "../config/ConfigDefaultHelpers";Leave the commented-out defaultStatsCfg block unchanged, as it will simply refer to a symbol that is not currently in scope but also not executed.
| @@ -8,7 +8,6 @@ | ||
| isFunction, isNullOrUndefined, isPlainObject, isPromiseLike, objDeepFreeze, objDefine, objForEachKey, objFreeze, objHasOwn, | ||
| scheduleTimeout, throwError | ||
| } from "@nevware21/ts-utils"; | ||
| import { cfgDfMerge } from "../config/ConfigDefaultHelpers"; | ||
| import { createDynamicConfig, onConfigChange } from "../config/DynamicConfig"; | ||
| import { ChannelControllerPriority } from "../constants/Constants"; | ||
| import { |
| const hasProcessors = registeredLogRecordProcessors.length > 0; | ||
|
|
||
| if (!hasProcessors) { | ||
| // This was previously creating a noop processor, but to reduce bundle size we are no longer doing that automatically. |
There was a problem hiding this comment.
This is the only functional change in this PR.
| const hasProcessors = registeredLogRecordProcessors.length > 0; | ||
| const activeProcessor = hasProcessors | ||
| ? createMultiLogRecordProcessor(registeredLogRecordProcessors, forceFlushTimeoutMillis) | ||
| : createNoopLogRecordProcessor(); |
There was a problem hiding this comment.
This is the functionality that was removed
- No longer uses
createNoopLogRecordProcessoras this is not in this package and the underlying createMultiLogRecordProcessor already "supported" passing in an empty array.
Fix: Issue for dts-gen because of renaming entrypoint to index.ts
MSNev
force-pushed
the
MSNev/OTelPkgMerge
branch
from
cb1e1eb to
1f2969b
Compare
3 participants
