Skip to content

Commit

Permalink
[monitor] Precise Typechecking (Azure#32153)
Browse files Browse the repository at this point in the history
### Packages impacted by this PR
- @azure/monitor-opentelemetry-exporter
- @azure/monitor-ingestion
- @azure/monitor-query

### Issues associated with this PR
N/A

### Describe the problem that is addressed by this PR
Tests are not being typechecked and linting isn't configured correctly.
This PR migrates those libraries to precise typechecking setup
introduced in Azure#31786

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

### Are there test cases added in this PR? _(If not, why?)_
N/A

### Provide a list of related PRs _(if any)_
Azure#31786

### 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)_
- [ ] Added a changelog (if necessary)
  • Loading branch information
deyaaeldeen authored Dec 11, 2024
1 parent bf29f9d commit df390f9
Show file tree
Hide file tree
Showing 44 changed files with 138 additions and 105 deletions.
12 changes: 12 additions & 0 deletions sdk/monitor/monitor-ingestion/eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import azsdkEslint from "@azure/eslint-plugin-azure-sdk";

export default azsdkEslint.config([
{
files: ["**/*.ts", "**/*.cts", "**/*.mts"],
languageOptions: {
parserOptions: {
project: ["./tsconfig.test.json"],
},
},
},
]);
5 changes: 3 additions & 2 deletions sdk/monitor/monitor-ingestion/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"generate:client": "autorest --typescript ./swagger/README.md",
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"integration-test:browser": "npm run clean && dev-tool run build-package && dev-tool run build-test && dev-tool run test:vitest --browser",
"integration-test:node": "dev-tool run test:vitest",
"integration-test:node": "dev-tool run test:vitest --esm",
"lint": "eslint package.json api-extractor.json src test",
"lint:fix": "eslint package.json api-extractor.json src test --fix --fix-type [problem,suggestion]",
"pack": "npm pack 2>&1",
Expand Down Expand Up @@ -125,7 +125,8 @@
"browser",
"react-native"
],
"selfLink": false
"selfLink": false,
"project": "./tsconfig.src.json"
},
"exports": {
"./package.json": "./package.json",
Expand Down
9 changes: 1 addition & 8 deletions sdk/monitor/monitor-ingestion/tsconfig.browser.config.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
{
"extends": "./.tshy/build.json",
"include": ["./src/**/*.ts", "./src/**/*.mts", "./test/**/*.spec.ts", "./test/**/*.mts"],
"exclude": ["./test/**/node/**/*.ts", "./test/snippets.spec.ts"],
"compilerOptions": {
"outDir": "./dist-test/browser",
"rootDir": ".",
"skipLibCheck": true
}
"extends": ["./tsconfig.test.json", "../../../tsconfig.browser.base.json"]
}
16 changes: 6 additions & 10 deletions sdk/monitor/monitor-ingestion/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
{
"extends": "../../../tsconfig",
"compilerOptions": {
"paths": {
"@azure/monitor-ingestion": ["./src/index"]
},
"lib": ["DOM"],
"module": "NodeNext",
"moduleResolution": "NodeNext",
"rootDir": "."
},
"include": ["src/**/*.ts", "src/**/*.mts", "src/**/*.cts", "samples-dev/**/*.ts", "test/**/*.ts"]
"references": [
{ "path": "./tsconfig.src.json" },
{ "path": "./tsconfig.samples.json" },
{ "path": "./tsconfig.test.json" }
],
"files": []
}
8 changes: 8 additions & 0 deletions sdk/monitor/monitor-ingestion/tsconfig.samples.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"extends": "../../../tsconfig.samples.base.json",
"compilerOptions": {
"paths": {
"@azure/monitor-ingestion": ["./dist/esm"]
}
}
}
3 changes: 3 additions & 0 deletions sdk/monitor/monitor-ingestion/tsconfig.src.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": "../../../tsconfig.lib.json"
}
3 changes: 3 additions & 0 deletions sdk/monitor/monitor-ingestion/tsconfig.test.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": ["./tsconfig.src.json", "../../../tsconfig.test.base.json"]
}
5 changes: 5 additions & 0 deletions sdk/monitor/monitor-ingestion/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ export default mergeConfig(
test: {
include: ["test/**/*.spec.ts"],
exclude: ["test/snippets.spec.ts"],
typecheck: {
enabled: true,
tsconfig: "tsconfig.test.json",
include: ["test/**/*.ts", "test/**/*.mts", "test/**/*.cts"],
},
},
}),
);
11 changes: 11 additions & 0 deletions sdk/monitor/monitor-ingestion/vitest.esm.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { mergeConfig } from "vitest/config";
import vitestConfig from "./vitest.config.ts";
import vitestEsmConfig from "../../../vitest.esm.shared.config.ts";

export default mergeConfig(
vitestConfig,
vitestEsmConfig
);
32 changes: 8 additions & 24 deletions sdk/monitor/monitor-opentelemetry-exporter/eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -1,28 +1,12 @@
import azsdkEslint from "@azure/eslint-plugin-azure-sdk";

export default [
{ ignores: ["src/Declarations"] },
...azsdkEslint.configs.recommendedTypeChecked,
export default azsdkEslint.config([
{
"rules": {
"@typescript-eslint/no-unsafe-member-access": "warn",
"@typescript-eslint/no-unsafe-assignment": "warn",
"@typescript-eslint/no-unsafe-call": "warn",
"@typescript-eslint/no-unsafe-argument": "warn",
"@typescript-eslint/restrict-template-expressions": "warn",
"@typescript-eslint/no-floating-promises": "warn",
"no-underscore-dangle": [
"error",
{
"allowAfterThis": true
}
],
"n/no-unsupported-features/es-syntax": [
"error",
{
"ignores": ["modules"]
}
]
}
files: ["**/*.ts", "**/*.cts", "**/*.mts"],
languageOptions: {
parserOptions: {
project: ["./tsconfig.test.json"],
},
},
},
];
]);
3 changes: 2 additions & 1 deletion sdk/monitor/monitor-opentelemetry-exporter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@
"esm",
"commonjs"
],
"selfLink": false
"selfLink": false,
"project": "./tsconfig.src.json"
},
"browser": "./dist/browser/index.js",
"exports": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export class AzureMonitorLogExporter extends AzureMonitorBaseExporter implements
* @param logs - Logs to export.
* @param resultCallback - Result callback.
*/
// eslint-disable-next-line @typescript-eslint/no-misused-promises
public async export(
logs: ReadableLogRecord[],
resultCallback: (result: ExportResult) => void,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export class AzureMonitorMetricExporter
* @param metrics - Resource metrics to export.
* @param resultCallback - Result callback.
*/
// eslint-disable-next-line @typescript-eslint/no-misused-promises
async export(
metrics: ResourceMetrics,
resultCallback: (result: ExportResult) => void,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class LongIntervalStatsbeatMetrics extends StatsbeatMetrics {
};
}

private async initialize() {
private async initialize(): Promise<void> {
try {
await this.getResourceProvider();

Expand All @@ -141,7 +141,7 @@ class LongIntervalStatsbeatMetrics extends StatsbeatMetrics {
}
}

private getEnvironmentStatus(observableResult: BatchObservableResult) {
private getEnvironmentStatus(observableResult: BatchObservableResult): void {
this.setFeatures();
let attributes;
if (this.instrumentation) {
Expand All @@ -163,7 +163,7 @@ class LongIntervalStatsbeatMetrics extends StatsbeatMetrics {
}
}

private setFeatures() {
private setFeatures(): void {
const statsbeatFeatures = process.env.AZURE_MONITOR_STATSBEAT_FEATURES;
if (statsbeatFeatures) {
try {
Expand All @@ -177,7 +177,7 @@ class LongIntervalStatsbeatMetrics extends StatsbeatMetrics {
}
}

private attachCallback(observableResult: ObservableResult) {
private attachCallback(observableResult: ObservableResult): void {
const attributes = { ...this.commonProperties, ...this.attachProperties };
observableResult.observe(1, attributes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export class AzureMonitorStatsbeatExporter
/**
* Export Statsbeat metrics.
*/
// eslint-disable-next-line @typescript-eslint/require-await, @typescript-eslint/no-misused-promises
async export(
metrics: ResourceMetrics,
resultCallback: (result: ExportResult) => void,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export class AzureMonitorTraceExporter extends AzureMonitorBaseExporter implemen
* @param spans - Spans to export.
* @param resultCallback - Result callback.
*/
// eslint-disable-next-line @typescript-eslint/no-misused-promises
async export(
spans: ReadableSpan[],
resultCallback: (result: ExportResult) => void,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ export abstract class BaseSender {
/**
* Disable collection of statsbeat metrics after max failures
*/
private incrementStatsbeatFailure() {
private incrementStatsbeatFailure(): void {
this.statsbeatFailureCount++;
if (this.statsbeatFailureCount > MAX_STATSBEAT_FAILURES) {
this.shutdownStatsbeat();
Expand All @@ -241,7 +241,7 @@ export abstract class BaseSender {
/**
* Shutdown statsbeat metrics
*/
private shutdownStatsbeat() {
private shutdownStatsbeat(): void {
this.networkStatsbeatMetrics?.shutdown();
this.longIntervalStatsbeatMetrics?.shutdown();
this.networkStatsbeatMetrics = undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export class HttpSender extends BaseSender {
* Shutdown sender
* @internal
*/
// eslint-disable-next-line @typescript-eslint/require-await
async shutdown(): Promise<void> {
diag.info("HttpSender shutting down");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ export class FileSystemPersist implements PersistentStorage {
if (files.length === 0) {
return false;
} else {
// eslint-disable-next-line @typescript-eslint/no-misused-promises
files.forEach(async (file) => {
// Check expiration
const fileCreationDate: Date = new Date(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { AzNamespace, MessageBusDestination } from "./constants/span/azAttribute
/**
* Average span.links[].attributes.enqueuedTime
*/
const getTimeSinceEnqueued = (span: ReadableSpan) => {
const getTimeSinceEnqueued = (span: ReadableSpan): number => {
let countEnqueueDiffs = 0;
let sumEnqueueDiffs = 0;
const startTimeMs = hrTimeToMilliseconds(span.startTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export function logToEnvelope(log: ReadableLogRecord, ikey: string): Envelope |
name = ApplicationInsightsExceptionName;
baseType = ApplicationInsightsExceptionBaseType;
const exceptionDetails: TelemetryExceptionDetails = {
// eslint-disable-next-line @typescript-eslint/no-base-to-string
typeName: String(exceptionType),
message: String(exceptionMessage),
hasFullStack: exceptionStacktrace ? true : false,
Expand All @@ -84,7 +83,6 @@ export function logToEnvelope(log: ReadableLogRecord, ikey: string): Envelope |
}
} else {
// If Legacy Application Insights Log
// eslint-disable-next-line @typescript-eslint/no-base-to-string
baseType = String(log.attributes[ApplicationInsightsBaseType]);
name = getLegacyApplicationInsightsName(log);
baseData = getLegacyApplicationInsightsBaseData(log);
Expand All @@ -100,7 +98,6 @@ export function logToEnvelope(log: ReadableLogRecord, ikey: string): Envelope |
}
if (properties) {
for (const key of Object.keys(properties)) {
// eslint-disable-next-line @typescript-eslint/no-base-to-string
properties[key] = String(properties[key]).substring(0, MaxPropertyLengths.THIRTEEN_BIT);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe("ConnectionStringParser", () => {
expectedInstrumentationKey?: string;
expectedBreezeEndpoint: string;
expectedLiveMetricsEndpoint: string;
}) => {
}): void => {
const result = ConnectionStringParser.parse(options.connectionString);

if (options.expectedAuthorization) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ describe("Log Exporter Scenarios", () => {
it("should work", async () => {
await scenario.run();
// promisify doesn't work on this, so use callbacks/done for now
// eslint-disable-next-line promise/always-return
await scenario.flush();
setTimeout(() => {
assertLogExpectation(ingest, scenario.expectation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ describe("Metric Exporter Scenarios", () => {
it("should work", async () => {
await scenario.run();
// promisify doesn't work on this, so use callbacks/done for now
// eslint-disable-next-line promise/always-return
await scenario.flush();
assertMetricExpectation(ingest, scenario.expectation);
assertCount(ingest, scenario.expectation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class TestTokenCredential implements TokenCredential {
this.expiresOn = expiresOn || new Date();
}

// eslint-disable-next-line @typescript-eslint/require-await
async getToken(): Promise<AccessToken | null> {
this.numberOfRefreshs++;
return {
Expand Down Expand Up @@ -71,7 +70,6 @@ describe("HttpSender", () => {
exporterOptions: {},
});
scope.reply(200, JSON.stringify(successfulBreezeResponse(1)));
// eslint-disable-next-line @typescript-eslint/no-misused-promises
setTimeout(async () => {
const { result, statusCode } = await sender.send([envelope]);
assert.strictEqual(statusCode, 200);
Expand Down Expand Up @@ -104,7 +102,6 @@ describe("HttpSender", () => {
exporterOptions: {},
});
scope.reply(206, JSON.stringify(partialBreezeResponse([200, 408, 408])));
// eslint-disable-next-line @typescript-eslint/no-misused-promises
setTimeout(async () => {
const { result, statusCode } = await sender.send([envelope, envelope]);
assert.strictEqual(statusCode, 206);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class TestExporter extends AzureMonitorMetricExporter {
constructor(options: AzureMonitorExporterOptions = {}) {
super(options);
}
// eslint-disable-next-line @typescript-eslint/require-await
async export(metrics: ResourceMetrics): Promise<void> {
testMetrics = metrics;
testMetrics.resource = new Resource({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export class TraceBasicScenario implements Scenario {
provider.register();
}

// eslint-disable-next-line @typescript-eslint/no-misused-promises
async run(): Promise<void> {
const tracer = opentelemetry.trace.getTracer("basic");
const root = tracer.startSpan(`${this.constructor.name}.Root`, {
Expand Down Expand Up @@ -93,7 +92,6 @@ export class TraceBasicScenario implements Scenario {
}

flush(): Promise<void> {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return this._processor.forceFlush();
}

Expand Down Expand Up @@ -290,7 +288,6 @@ export class MetricBasicScenario implements Scenario {
this._provider.addMetricReader(metricReader);
}

// eslint-disable-next-line @typescript-eslint/no-misused-promises
async run(): Promise<void> {
const meter = this._provider.getMeter("basic");
const counter = meter.createCounter("testCounter");
Expand Down Expand Up @@ -479,7 +476,6 @@ export class LogBasicScenario implements Scenario {
this._provider.addLogRecordProcessor(this._processor);
}

// eslint-disable-next-line @typescript-eslint/require-await, @typescript-eslint/no-misused-promises
async run(): Promise<void> {
const logger = this._provider.getLogger("basic");

Expand Down Expand Up @@ -507,7 +503,6 @@ export class LogBasicScenario implements Scenario {
}

flush(): Promise<void> {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return this._processor.forceFlush();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"extends": ["./tsconfig.test.json", "../../../tsconfig.browser.base.json"]
}
Loading

0 comments on commit df390f9

Please sign in to comment.