[monitor-query] Addressing last bits of feedback for 1.0.0-beta.1 (Azure#15578)

Addressing more of the API review and feature crew feedback:

- Rename CommonDurations to Durations, Logs and Metrics clients are now LogsQueryClient and MetricsQueryClient, respectively.
- Make timespan a required parameter (request from the service team)
- Fixing some cosmetic naming in the samples. We have three parameters now and they're all strings...
- Deserialize `dynamic` and `Date` properties into their native types, when possible. Also, - fixed type definition for 'Table.rows' so it accomodates the wider variety of primitives it deserializes (Date, number (int, long, real), boolean and dynamic).
- LocalizableStrings are now just normal `string`'s.
- Rename the return objects from the operations to be `*Result` rather than `*Response`.

And some other general improvements:
- Add in a reliable test for server timeout using a query that Pavel cooked up.
- Fixed any outstanding lint issues and made lint a required build step so they don't sneak back in.

And making some things a bit less specified:
- Replace statistics with 'any' until we figure out how we want to map the schema.
- Leaving MetadataValue in place until I find the right replacement for it.

Fixing most of Azure#15296

Author: richardpark-msft

sdk/monitor/monitor-query/README.md

@@ -37,7 +37,7 @@ Instructions:
 
 ### Authenticate the client
 
-[LogsClient]<!--[msdocs_logs_client]--> and [MetricsClient]<!--[msdocs_metrics_client]--> authenticate using a [service principal](#authenticating-with-a-service-principal).
+[LogsQueryClient]<!--[msdocs_logs_client]--> and [MetricsQueryClient]<!--[msdocs_metrics_client]--> authenticate using a [service principal](#authenticating-with-a-service-principal).
 
 #### Authenticating with a service principal
 
@@ -51,40 +51,40 @@ Using [DefaultAzureCredential](https://github.com/Azure/azure-sdk-for-js/blob/ma
 
 ```javascript
 const { DefaultAzureCredential } = require("@azure/identity");
-const { LogsClient, MetricsClient } = require("@azure/monitor-query");
+const { LogsQueryClient, MetricsQueryClient } = require("@azure/monitor-query");
 
 const credential = new DefaultAzureCredential();
 
-const logsClient = new LogsClient(credential);
+const logsQueryClient = new LogsQueryClient(credential);
 // or
-const metricsClient = new MetricsClient(credential);
+const metricsQueryClient = new MetricsQueryClient(credential);
 ```
 
 More information about `@azure/identity` can be found [here](https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/identity/identity/README.md)
 
 ## Key concepts
 
-The [`LogsClient`]<!--[msdocs_logs_client]--> allows you to query logs, using the [Kusto query language][kusto_query_language]. This data can be queried in the
+The [`LogsQueryClient`]<!--[msdocs_logs_client]--> allows you to query logs, using the [Kusto query language][kusto_query_language]. This data can be queried in the
 portal using tables like `AppEvents`, `AppDependencies` and others.
 
-The [`MetricsClient`]<!--[msdocs_metrics_client]--> allows you to query metrics.
+The [`MetricsQueryClient`]<!--[msdocs_metrics_client]--> allows you to query metrics.
 
 ## Examples
 
 #### Querying logs
 
-The LogsClient can be used to query a Monitor workspace using the Kusto Query language.
+The `LogsQueryClient` can be used to query a Monitor workspace using the Kusto Query language.
 
 ```javascript
-const { LogsClient } = require("@azure/monitor-query");
+const { LogsQueryClient } = require("@azure/monitor-query");
 const { DefaultAzureCredential } = require("@azure/identity");
 
 const azureLogAnalyticsWorkspaceId = "<the Workspace Id for your Azure Log Analytics resource>";
-const logsClient = new LogsClient(new DefaultAzureCredential());
+const logsQueryClient = new LogsQueryClient(new DefaultAzureCredential());
 
 async function run() {
   const kustoQuery = "AppEvents | limit 1";
-  const result = logsClient.queryLogs(azureLogAnalyticsWorkspaceId, kustoQuery);
+  const result = await logsQueryClient.queryLogs(azureLogAnalyticsWorkspaceId, kustoQuery);
   const tablesFromResult = result.tables;
 
   if (tablesFromResult == null) {
@@ -166,8 +166,8 @@ folder for more details.
 [azure_monitor_create_using_cli]: https://docs.microsoft.com/azure/azure-monitor/logs/quick-create-workspace-cli
 [azure_monitor_roles]: https://docs.microsoft.com/azure/azure-monitor/roles-permissions-security
 [kusto_query_language]: https://docs.microsoft.com/azure/data-explorer/kusto/query/
-[msdocs_metrics_client]: https://docs.microsoft.com/javascript/api/@azure/monitor-query/metricsclient
-[msdocs_logs_client]: https://docs.microsoft.com/javascript/api/@azure/monitor-query/logsclient
+[msdocs_metrics_client]: https://docs.microsoft.com/javascript/api/@azure/monitor-query/metricsqueryclient
+[msdocs_logs_client]: https://docs.microsoft.com/javascript/api/@azure/monitor-query/logsqueryclient
 [samples]: https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/monitor/monitor-query/samples
 [samples_logsquery_ts]: https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/monitor/monitor-query/samples/v1/typescript/src/logQuery.ts
 [samples_logquerybatch_ts]: https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/monitor/monitor-query/samples/v1/typescript/src/logQueryBatch.ts

sdk/monitor/monitor-query/package.json

@@ -62,7 +62,7 @@
     "integration-test:node": "nyc mocha -r esm --require source-map-support/register --slow 5000 --reporter ../../../common/tools/mocha-multi-reporter.js --timeout 5000000 --full-trace \"dist-esm/test/{,!(browser)/**/}/*.spec.js\"",
     "integration-test": "npm run integration-test:node && npm run integration-test:browser",
     "lint:fix": "eslint package.json api-extractor.json src test --ext .ts --fix --fix-type [problem,suggestion]",
-    "lint": "eslint package.json api-extractor.json src test --format unix --ext .ts",
+    "lint": "eslint package.json api-extractor.json src test --format unix --ext .ts --max-warnings 0",
     "pack": "npm pack 2>&1",
     "prebuild": "npm run clean",
     "test:browser": "npm run build:test && npm run integration-test:browser",

sdk/monitor/monitor-query/review/monitor-query.api.md

@@ -18,7 +18,7 @@ export interface BatchQuery {
     qualifiedNames?: string[];
     query: string;
     serverTimeoutInSeconds?: number;
-    timespan?: string;
+    timespan: string;
     workspace: string;
     workspaceIds?: string[];
     workspaces?: string[];
@@ -28,7 +28,7 @@ export interface BatchQuery {
 export type ColumnDataType = string;
 
 // @public
-export const CommonDurations: {
+export const Durations: {
     readonly last7Days: "P7D";
     readonly last3Days: "P3D";
     readonly last2Days: "P2D";
@@ -66,7 +66,7 @@ export interface GetMetricDefinitionsOptions extends OperationOptions {
 }
 
 // @public
-export interface GetMetricDefinitionsResponse {
+export interface GetMetricDefinitionsResult {
     definitions: MetricDefinition[];
 }
 
@@ -76,33 +76,38 @@ export interface GetMetricNamespacesOptions {
 }
 
 // @public
-export interface GetMetricNamespacesResponse {
+export interface GetMetricNamespacesResult {
     namespaces: MetricNamespace[];
 }
 
 // @public
-export interface LocalizableString {
-    localizedValue?: string;
-    value: string;
+export interface LogsColumn {
+    name?: string;
+    type?: ColumnDataType;
 }
 
 // @public
-export class LogsClient {
-    constructor(tokenCredential: TokenCredential, options?: LogsClientOptions);
-    // (undocumented)
-    queryLogs(workspaceId: string, query: string, options?: QueryLogsOptions): Promise<QueryLogsResult>;
-    // (undocumented)
-    queryLogsBatch(batch: QueryLogsBatch, options?: QueryLogsBatchOptions): Promise<QueryLogsBatchResponse>;
+export class LogsQueryClient {
+    constructor(tokenCredential: TokenCredential, options?: LogsQueryClientOptions);
+    queryLogs(workspaceId: string, query: string, timespan: string, options?: QueryLogsOptions): Promise<QueryLogsResult>;
+    queryLogsBatch(batch: QueryLogsBatch, options?: QueryLogsBatchOptions): Promise<QueryLogsBatchResult>;
 }
 
-// @public (undocumented)
-export interface LogsClientOptions extends PipelineOptions {
+// @public
+export interface LogsQueryClientOptions extends PipelineOptions {
     endpoint?: string;
 }
 
+// @public
+export interface LogsTable {
+    columns: LogsColumn[];
+    name: string;
+    rows: (Date | string | number | Record<string, unknown> | boolean)[][];
+}
+
 // @public
 export interface MetadataValue {
-    name?: LocalizableString;
+    name?: string;
     value?: string;
 }
 
@@ -111,7 +116,7 @@ export interface Metric {
     displayDescription: string;
     errorCode?: string;
     id: string;
-    name: LocalizableString;
+    name: string;
     timeseries: TimeSeriesElement[];
     type: string;
     unit: MetricUnit;
@@ -123,26 +128,18 @@ export interface MetricAvailability {
     timeGrain?: string;
 }
 
-// @public
-export interface MetricColumn {
-    name?: string;
-    type?: ColumnDataType;
-}
-
 // @public
 export interface MetricDefinition {
     category?: string;
-    dimensions?: LocalizableString_2[];
+    dimensions?: string[];
     displayDescription?: string;
     id?: string;
     isDimensionRequired?: boolean;
     metricAvailabilities?: MetricAvailability[];
-    // Warning: (ae-forgotten-export) The symbol "LocalizableString" needs to be exported by the entry point index.d.ts
-    name?: LocalizableString_2;
+    name?: string;
     primaryAggregationType?: AggregationType;
     resourceId?: string;
-    // Warning: (ae-forgotten-export) The symbol "MetricUnit" needs to be exported by the entry point index.d.ts
-    unit?: MetricUnit_2;
+    unit?: MetricUnit;
 }
 
 // @public
@@ -159,22 +156,19 @@ export interface MetricNamespaceName {
     metricNamespaceName?: string;
 }
 
-// @public (undocumented)
-export class MetricsClient {
-    constructor(tokenCredential: TokenCredential, options?: MetricsClientOptions);
-    // (undocumented)
-    getMetricDefinitions(resourceUri: string, options?: GetMetricDefinitionsOptions): Promise<GetMetricDefinitionsResponse>;
-    // (undocumented)
-    getMetricNamespaces(resourceUri: string, options?: GetMetricNamespacesOptions): Promise<GetMetricNamespacesResponse>;
-    // (undocumented)
-    queryMetrics(resourceUri: string, options?: QueryMetricsOptions): Promise<QueryMetricsResponse>;
-}
-
-// @public (undocumented)
+// @public
 export interface MetricsClientOptions extends PipelineOptions {
     endpoint?: string;
 }
 
+// @public
+export class MetricsQueryClient {
+    constructor(tokenCredential: TokenCredential, options?: MetricsClientOptions);
+    getMetricDefinitions(resourceUri: string, options?: GetMetricDefinitionsOptions): Promise<GetMetricDefinitionsResult>;
+    getMetricNamespaces(resourceUri: string, options?: GetMetricNamespacesOptions): Promise<GetMetricNamespacesResult>;
+    queryMetrics(resourceUri: string, timespan: string, options?: QueryMetricsOptions): Promise<QueryMetricsResult>;
+}
+
 // @public
 export type MetricUnit = string;
 
@@ -196,30 +190,30 @@ export interface QueryLogsBatch {
     queries: BatchQuery[];
 }
 
-// @public (undocumented)
+// @public
 export type QueryLogsBatchOptions = OperationOptions;
 
 // @public
-export interface QueryLogsBatchResponse {
+export interface QueryLogsBatchResult {
     results?: {
         id?: string;
         status?: number;
-        tables?: Table[];
+        tables?: LogsTable[];
         error?: ErrorInfo;
     }[];
 }
 
-// @public (undocumented)
+// @public
 export interface QueryLogsOptions extends OperationOptions {
     includeQueryStatistics?: boolean;
     serverTimeoutInSeconds?: number;
-    timespan?: string;
 }
 
-// @public (undocumented)
-export type QueryLogsResult = QueryResults & {
-    statistics?: QueryStatistics;
-};
+// @public
+export interface QueryLogsResult {
+    statistics?: any;
+    tables: LogsTable[];
+}
 
 // @public
 export interface QueryMetricsOptions extends OperationOptions {
@@ -230,12 +224,11 @@ export interface QueryMetricsOptions extends OperationOptions {
     metricNamespace?: string;
     orderBy?: string;
     resultType?: ResultType;
-    timespan?: string;
     top?: number;
 }
 
 // @public
-export interface QueryMetricsResponse {
+export interface QueryMetricsResult {
     cost?: number;
     interval?: string;
     metrics: Metric[];
@@ -244,33 +237,9 @@ export interface QueryMetricsResponse {
     timespan: string;
 }
 
-// @public
-export interface QueryResults {
-    statistics?: any;
-    tables: Table[];
-}
-
-// @public (undocumented)
-export interface QueryStatistics {
-    // (undocumented)
-    [key: string]: unknown;
-    // (undocumented)
-    query?: {
-        executionTime?: number;
-        [key: string]: unknown;
-    };
-}
-
 // @public
 export type ResultType = "Data" | "Metadata";
 
-// @public
-export interface Table {
-    columns: MetricColumn[];
-    name: string;
-    rows: string[][];
-}
-
 // @public
 export interface TimeSeriesElement {
     data?: MetricValue[];

sdk/monitor/monitor-query/samples-dev/logsQuery.ts

@@ -6,47 +6,50 @@
  */
 
 import { DefaultAzureCredential } from "@azure/identity";
-import { CommonDurations, LogsClient, Table } from "@azure/monitor-query";
+import { Durations, LogsQueryClient, LogsTable } from "@azure/monitor-query";
 import * as dotenv from "dotenv";
 dotenv.config();
 
 const monitorWorkspaceId = process.env.MONITOR_WORKSPACE_ID;
 
 export async function main() {
   const tokenCredential = new DefaultAzureCredential();
-  const logsClient = new LogsClient(tokenCredential);
+  const logsQueryClient = new LogsQueryClient(tokenCredential);
 
   if (!monitorWorkspaceId) {
     throw new Error("MONITOR_WORKSPACE_ID must be set in the environment for this sample");
   }
 
-  const kqlQuery =
+  const kustoQuery =
     "AppEvents | project TimeGenerated, Name, AppRoleInstance | order by TimeGenerated asc | limit 10";
 
-  console.log(`Running '${kqlQuery}' for the last 5 minutes`);
-  const result = await logsClient.queryLogs(monitorWorkspaceId, kqlQuery, {
+  console.log(`Running '${kustoQuery}' over the last 5 minutes`);
+  const result = await logsQueryClient.queryLogs(
+    monitorWorkspaceId,
+    kustoQuery,
     // The timespan is an ISO8601 formatted time (or interval). Some common aliases
     // are available (like lastDay, lastHour, last48Hours, etc..) but any properly formatted ISO8601
     // value is valid.
-    timespan: CommonDurations.last5Minutes,
+    Durations.last5Minutes,
+    {
+      // optionally enable returning additional statistics about the query's execution.
+      // (by default this is off)
+      includeQueryStatistics: true,
 
-    // optionally enable returning additional statistics about the query's execution.
-    // (by default this is off)
-    includeQueryStatistics: true,
-
-    // explicitly control the amount of time the server can spend processing the query.
-    serverTimeoutInSeconds: 60
-  });
+      // explicitly control the amount of time the server can spend processing the query.
+      serverTimeoutInSeconds: 60
+    }
+  );
 
-  const tablesFromResult: Table[] | undefined = result.tables;
+  const tablesFromResult: LogsTable[] | undefined = result.tables;
 
   if (tablesFromResult == null) {
-    console.log(`No results for query '${kqlQuery}'`);
+    console.log(`No results for query '${kustoQuery}'`);
     return;
   }
 
   console.log(
-    `Results for query '${kqlQuery}', execution time: ${result.statistics?.query?.executionTime}`
+    `Results for query '${kustoQuery}', execution time: ${result.statistics?.query?.executionTime}`
   );
 
   for (const table of tablesFromResult) {