Error build TypeScript (tsc)
Clement-Stauner opened this issue · 4 comments
Clement-Stauner commented
What happened?
Steps to Reproduce
npm run buildExpected Result
Builds all dependencies
Actual Result
Fail to build module @opentelemetry/sdk-logs
Additional Details
OpenTelemetry Setup Code
// Import downloaded modules: winston
import Transport from "winston-transport";
import type TransportStream from "winston-transport";
// Import downloaded modules: Open Telemetry
import { Logger } from "@opentelemetry/api-logs";
import { OTLPLogExporter } from "@opentelemetry/exporter-logs-otlp-grpc";
import { Resource } from "@opentelemetry/resources";
import {
LoggerProvider,
SimpleLogRecordProcessor,
} from "@opentelemetry/sdk-logs";
import { SemanticResourceAttributes } from "@opentelemetry/semantic-conventions";
const OpenTelemetryLogLevel = {
FATAL: 21,
ERROR: 17,
WARN: 13,
INFO: 9,
DEBUG: 5,
TRACE: 1,
} as const;
// we use default winston level https://github.com/winstonjs/winston#logging-levels
// OTL have different level, we need to do a mapping, so customer have final log in OTLP
const mapLogLevelFromNpmToOpenTelemetry = (
level: string,
): keyof typeof OpenTelemetryLogLevel => {
switch (level) {
case "error":
return "ERROR";
case "warn":
return "WARN";
case "info":
return "INFO";
case "verbose":
case "debug":
case "silly":
return "DEBUG";
default:
return "INFO";
}
};
const mapNpmLogLevelToOpenTelemetrySeverityNumber = (
level: string | undefined,
): (typeof OpenTelemetryLogLevel)[keyof typeof OpenTelemetryLogLevel] => {
switch (level) {
case "trace":
return 1;
case "verbose":
case "debug":
return 5;
case "info":
return 9;
case "warn":
return 13;
case "error":
return 17;
case "fatal":
return 21;
default:
return 9;
}
};
export class OpenTelemetryWinstonTransport extends Transport {
loggerOTL: Logger;
constructor(opts?: TransportStream.TransportStreamOptions) {
super(opts);
const logExporter = new OTLPLogExporter();
const loggerProvider = new LoggerProvider({
resource: new Resource({
[SemanticResourceAttributes.SERVICE_NAME]:
process.env["OTEL_SERVICE_NAME"],
}),
});
loggerProvider.addLogRecordProcessor(
new SimpleLogRecordProcessor(logExporter),
);
this.loggerOTL = loggerProvider.getLogger("default");
}
/* eslint-disable @typescript-eslint/no-explicit-any */
override log(info: any, next: () => void) {
/* eslint-disable @typescript-eslint/no-explicit-any */
setImmediate(() => {
const logRecord = {
severityNumber:
info?.severityNumber ||
mapNpmLogLevelToOpenTelemetrySeverityNumber(info?.level),
severityText:
mapLogLevelFromNpmToOpenTelemetry(info?.level || "") || "",
body: info?.message || "",
};
this.loggerOTL.emit(logRecord);
next();
});
}
}package.json
{
"scripts": {
"clean": "rimraf dist",
"build": "npm run clean && npx tsc",
},
"dependencies": {
"@opentelemetry/api-logs": "^0.48.0",
"@opentelemetry/exporter-logs-otlp-grpc": "^0.48.0",
"@opentelemetry/resources": "^1.21.0",
"@opentelemetry/sdk-logs": "^0.48.0",
"@opentelemetry/semantic-conventions": "^1.21.0",
}
}Relevant log output
npm run build
> project@1.0.0 build
> npm run clean && npx tsc
> project@1.0.0 clean
> rimraf dist
node_modules/@opentelemetry/sdk-logs/build/src/export/NoopLogRecordProcessor.d.ts:5:5 - error TS2416: Property 'onEmit' in type 'NoopLogRecordProcessor' is not assignable to the same property in base type 'LogRecordProcessor'.
Type '(_logRecord: ReadableLogRecord) => void' is not assignable to type '(logRecord: LogRecord, context?: Context | undefined) => void'.
Types of parameters '_logRecord' and 'logRecord' are incompatible.
Type 'LogRecord' is not assignable to type 'ReadableLogRecord' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
Types of property 'severityText' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.
5 onEmit(_logRecord: ReadableLogRecord): void;
~~~~~~
node_modules/@opentelemetry/sdk-logs/build/src/LogRecord.d.ts:9:22 - error TS2420: Class 'LogRecord' incorrectly implements interface 'ReadableLogRecord'.
Types of property 'severityText' are incompatible.
Type 'string | undefined' is not assignable to type 'string'.
Type 'undefined' is not assignable to type 'string'.
9 export declare class LogRecord implements ReadableLogRecord {
~~~~~~~~~
Found 2 errors in 2 files.
Errors Files
1 node_modules/@opentelemetry/sdk-logs/build/src/export/NoopLogRecordProcessor.d.ts:5
1 node_modules/@opentelemetry/sdk-logs/build/src/LogRecord.d.ts:9pichlermarc commented
@Clement-Stauner thanks for reaching out. I've tried reproducing this (https://github.com/pichlermarc/repro-4489) but I'm not sure I'm missing something. Would you mind trying to remove node_modules and package-lock.json and trying again with a clean install of dependencies?
Clement-Stauner commented
I managed to solve the issue by setting the TS compiler skipLibCheck option to true.
{
"compilerOptions": {
"skipLibCheck": true
}
}But there are still two native type safety bugs in the library that the TS compiler caught while trying to compile, it you set strong type checking in the tsconfig.json file, you will get the error.
Here here my tsconfig.json file that caught the two bugs with skipLibCheck option to false.
{
"compilerOptions": {
"baseUrl": "./", // Add this line
"paths": {
// Add or adjust this section
"*": ["node_modules/*", "src/*"]
},
"module": "CommonJS", // Ensure this is set for Node.js compatibility
"rootDirs": ["src", "dist"], // Add this line if you want to include multiple root directories
"outDir": "dist",
"noEmitOnError": true,
"forceConsistentCasingInFileNames": true,
"esModuleInterop": true,
"isolatedModules": true,
"moduleResolution": "node",
/* Type Checking */
"strict": true /* Enable all strict type-checking options. */,
"noImplicitAny": true /* Enable error reporting for expressions and declarations with an implied 'any' type. */,
"strictNullChecks": true /* When type checking, take into account 'null' and 'undefined'. */,
"strictFunctionTypes": true /* When assigning functions, check to ensure parameters and the return values are subtype-compatible. */,
"strictBindCallApply": true /* Check that the arguments for 'bind', 'call', and 'apply' methods match the original function. */,
"strictPropertyInitialization": true /* Check for class properties that are declared but not set in the constructor. */,
"noImplicitThis": true /* Enable error reporting when 'this' is given the type 'any'. */,
"useUnknownInCatchVariables": true /* Default catch clause variables as 'unknown' instead of 'any'. */,
"alwaysStrict": true /* Ensure 'use strict' is always emitted. */,
"noUnusedLocals": true /* Enable error reporting when local variables aren't read. */,
"noUnusedParameters": true /* Raise an error when a function parameter isn't read. */,
"exactOptionalPropertyTypes": true /* Interpret optional property types as written, rather than adding 'undefined'. */,
"noImplicitReturns": true /* Enable error reporting for codepaths that do not explicitly return in a function. */,
"noFallthroughCasesInSwitch": true /* Enable error reporting for fallthrough cases in switch statements. */,
"noUncheckedIndexedAccess": true /* Add 'undefined' to a type when accessed using an index. */,
"noImplicitOverride": true /* Ensure overriding members in derived classes are marked with an override modifier. */,
"noPropertyAccessFromIndexSignature": true /* Enforces using indexed accessors for keys declared using an indexed type. */,
"allowUnusedLabels": true /* Disable error reporting for unused labels. */,
"allowUnreachableCode": true /* Disable error reporting for unreachable code. */,
/* Completeness */
"skipDefaultLibCheck": true /* Skip type checking .d.ts files that are included with TypeScript. */,
"skipLibCheck": true
},
"include": ["src/**/*"]
}
pichlermarc commented
Ah I see. This is actually reproducible with "exactOptionalPropertyTypes": true, which is not included in strict by default (because turning it on does cause a lot of churn for existing code bases to adhere to it).
My guess is that adding that will actually cause a huge diff though because we're not using optional properties like that. Not sure what the implications for this would be (if that's a breaking change for some consumers of the library, my guess is "no" but I would have to try that out). It might be a good idea to do this in SDK 2.0 at least to maximize compatibility.
I'll bring it up in the SIG meeting this week, looks like this issue is a duplicate of #3713
pichlermarc commented