[@azure/monitor-query] Code Changes to add scope to Metrics Query Cli…

…ent and modify MetricsBatchOptionalParams (#26868)

### Packages impacted by this PR
@azure/monitor-query

### Issues associated with this PR
N/A

### Describe the problem that is addressed by this PR
1. In this release, In the `MetricsBatchOptionalParams`object, the names
of the properties `endtime`, `orderby`, `starttime` are changed to
`endTime`, `orderBy`, `startTime` respectively.
2. The scope value is not set correctly in the previous release. In this
release, I am setting the scope value correctly.
3. The previous release did not have integration tests. With this PR, I
have added the integration tests.

### 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?
There are no special design changes in this PR, other than setting the
credential scope.

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

One point to note is that the new tests are marked as skip. Because, the
recordings will have subscription id in them which we do not want to
expose. So, for CI, I have marked them as Skip. I am working on possible
solutions to bypass this issue in the future. But, the tests have been
run locally and the snapshots are provided below.


![image](https://github.com/Azure/azure-sdk-for-js/assets/602456/43e9149b-a882-4707-b43b-68b3f4df7211)


![image](https://github.com/Azure/azure-sdk-for-js/assets/602456/f89e32a2-2af9-45b6-a90e-1e1ddc504166)

### Provide a list of related PRs _(if any)_
N/A

### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_
N/A

### 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)

---------

Co-authored-by: Scott Addie <10702007+scottaddie@users.noreply.github.com>

sdk/monitor/monitor-query/CHANGELOG.md

@@ -1,14 +1,15 @@
 # Release History
 
-## 1.2.0-beta.2 (Unreleased)
+## 1.2.0-beta.2 (2023-08-21)
 
-### Features Added
+- Added `batchMetricsAuthScope` property to `MetricsBatchQueryClientOptions` object.
+- Added integration test cases for `MetricsBatchQueryClient`.
 
 ### Breaking Changes
 
-### Bugs Fixed
-
-### Other Changes
+- In the `MetricsBatchOptionalParams` object, the casing of property names `endtime`, `orderby`, and `starttime` changed to `endTime`, `orderBy`, and `startTime` respectively.
+- In the `MetricsBatchQueryClientOptions` object, the `batchEndPoint` has been removed as optional property. Instead, this has been changed to mandatory parameter in the `MetricsBatchQueryClient` class.
+- In the `MetricsBatchOptionalParams` object, the data type properties of `startTime` & `endTime` have been changed from `string` to `Date`.
 
 ## 1.2.0-beta.1 (2023-08-11)
 

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

@@ -205,23 +205,23 @@ export interface MetricResultsResponseValuesItem {
 // @public
 export interface MetricsBatchOptionalParams extends coreClient.OperationOptions {
     aggregation?: string;
-    endtime?: string;
+    endTime?: Date;
     filter?: string;
     interval?: string;
-    orderby?: string;
-    starttime?: string;
+    orderBy?: string;
+    startTime?: Date;
     top?: number;
 }
 
 // @public
 export class MetricsBatchQueryClient {
-    constructor(tokenCredential: TokenCredential, options?: MetricsBatchQueryClientOptions);
+    constructor(batchEndPoint: string, tokenCredential: TokenCredential, options?: MetricsBatchQueryClientOptions);
     queryBatch(resourceIds: string[], metricNamespace: string, metricNames: string[], options?: MetricsBatchOptionalParams): Promise<MetricResultsResponseValuesItem[]>;
 }
 
 // @public
 export interface MetricsBatchQueryClientOptions extends CommonClientOptions {
-    batchEndPoint?: string;
+    batchMetricsAuthScope?: string;
 }
 
 // @public

sdk/monitor/monitor-query/src/index.ts

@@ -62,9 +62,10 @@ export {
 export { AggregationType, MetricClass } from "./generated/metricsdefinitions/src";
 export { NamespaceClassification } from "./generated/metricsnamespaces/src";
 
-export { MetricsBatchOptionalParams, LocalizableString } from "./generated/metricBatch/src";
+export { LocalizableString } from "./generated/metricBatch/src";
 export {
   MetricResultsResponseValuesItem,
   Metric as BatchQueryMetric,
+  MetricsBatchOptionalParams,
 } from "./models/publicBatchModels";
 export { MetricsBatchQueryClient, MetricsBatchQueryClientOptions } from "./metricsBatchQueryClient";

sdk/monitor/monitor-query/src/internal/modelConverters.ts

@@ -55,8 +55,14 @@ import {
   LogsQueryResultStatus,
   LogsQuerySuccessfulResult,
 } from "../models/publicLogsModels";
-import { MetricsBatchResponse as GeneratedMetricsBatchResponse } from "../generated/metricBatch/src";
-import { MetricResultsResponseValuesItem } from "../models/publicBatchModels";
+import {
+  MetricsBatchResponse as GeneratedMetricsBatchResponse,
+  MetricsBatchOptionalParams as GeneratedMetricsBatchOptionalParams,
+} from "../generated/metricBatch/src";
+import {
+  MetricResultsResponseValuesItem,
+  MetricsBatchOptionalParams,
+} from "../models/publicBatchModels";
 
 /**
  * @internal
@@ -183,6 +189,23 @@ export function fixInvalidBatchQueryResponse(
   return hadToFix;
 }
 
+/**
+ * @internal
+ */
+export function convertRequestForMetricsBatchQuery(
+  metricsBatchQueryOptions: MetricsBatchOptionalParams | undefined
+): GeneratedMetricsBatchOptionalParams {
+  if (!metricsBatchQueryOptions) {
+    return {};
+  }
+
+  return {
+    starttime: metricsBatchQueryOptions.startTime?.toISOString(),
+    endtime: metricsBatchQueryOptions.endTime?.toISOString(),
+    ...metricsBatchQueryOptions,
+  };
+}
+
 /**
  * @internal
  */

sdk/monitor/monitor-query/src/metricsBatchQueryClient.ts

@@ -6,20 +6,25 @@ import { tracingClient } from "./tracing";
 import {
   AzureMonitorMetricBatch as GeneratedMonitorMetricBatchClient,
   KnownApiVersion20230501Preview as MonitorMetricBatchApiVersion,
-  MetricsBatchOptionalParams,
 } from "./generated/metricBatch/src";
-import { convertResponseForMetricBatch } from "./internal/modelConverters";
+import {
+  convertResponseForMetricBatch,
+  convertRequestForMetricsBatchQuery,
+} from "./internal/modelConverters";
 import { SDK_VERSION } from "./constants";
-import { MetricResultsResponseValuesItem } from "./models/publicBatchModels";
+import {
+  MetricResultsResponseValuesItem,
+  MetricsBatchOptionalParams,
+} from "./models/publicBatchModels";
 
 const defaultMetricsScope = "https://management.azure.com/.default";
 
 /**
  * Options for the MetricsQueryClient.
  */
 export interface MetricsBatchQueryClientOptions extends CommonClientOptions {
-  /** Overrides batch client endpoint. */
-  batchEndPoint?: string;
+  /** Metrics scope */
+  batchMetricsAuthScope?: string;
 }
 
 export const getSubscriptionFromResourceId = function (resourceId: string): string {
@@ -35,10 +40,14 @@ export class MetricsBatchQueryClient {
   private _metricBatchClient: GeneratedMonitorMetricBatchClient;
   private _baseUrl: string;
 
-  constructor(tokenCredential: TokenCredential, options?: MetricsBatchQueryClientOptions) {
+  constructor(
+    batchEndPoint: string,
+    tokenCredential: TokenCredential,
+    options?: MetricsBatchQueryClientOptions
+  ) {
     let scope;
-    if (options?.batchEndPoint) {
-      scope = `${options?.batchEndPoint}/.default`;
+    if (options?.batchMetricsAuthScope) {
+      scope = `${options?.batchMetricsAuthScope}/.default`;
     }
     const credentialOptions = {
       credentialScopes: scope,
@@ -50,16 +59,16 @@ export class MetricsBatchQueryClient {
         : `${packageDetails}`;
     const serviceClientOptions = {
       ...options,
-      $host: options?.batchEndPoint,
-      endpoint: options?.batchEndPoint,
+      $host: batchEndPoint,
+      endpoint: batchEndPoint,
       credentialScopes: credentialOptions?.credentialScopes ?? defaultMetricsScope,
       credential: tokenCredential,
       userAgentOptions: {
         userAgentPrefix,
       },
     };
 
-    this._baseUrl = serviceClientOptions.batchEndPoint ?? "";
+    this._baseUrl = batchEndPoint;
 
     this._metricBatchClient = new GeneratedMonitorMetricBatchClient(
       MonitorMetricBatchApiVersion.TwoThousandTwentyThree0501Preview,
@@ -80,21 +89,25 @@ export class MetricsBatchQueryClient {
       throw new Error("Resource IDs can not be empty");
     }
 
-    return tracingClient.withSpan("MetricsBatchClient.batch", options, async (updatedOptions) => {
-      const subscriptionId = getSubscriptionFromResourceId(resourceIds[0]);
+    return tracingClient.withSpan(
+      "MetricsBatchQueryClient.batch",
+      options,
+      async (updatedOptions) => {
+        const subscriptionId = getSubscriptionFromResourceId(resourceIds[0]);
 
-      const response = await this._metricBatchClient.metrics.batch(
-        this._baseUrl,
-        subscriptionId,
-        metricNamespace,
-        metricNames,
-        {
-          resourceids: resourceIds,
-        },
-        updatedOptions
-      );
+        const response = await this._metricBatchClient.metrics.batch(
+          this._baseUrl,
+          subscriptionId,
+          metricNamespace,
+          metricNames,
+          {
+            resourceids: resourceIds,
+          },
+          convertRequestForMetricsBatchQuery(updatedOptions)
+        );
 
-      return convertResponseForMetricBatch(response);
-    });
+        return convertResponseForMetricBatch(response);
+      }
+    );
   }
 }

sdk/monitor/monitor-query/src/models/publicBatchModels.ts

@@ -4,6 +4,7 @@
 import { TimeSeriesElement } from "./publicMetricsModels";
 import { MetricUnit } from "../generated/metrics/src";
 import { LocalizableString } from "../generated/metricBatch/src";
+import * as coreClient from "@azure/core-client";
 
 /**
  * Metric Results Response Values Item
@@ -44,3 +45,39 @@ export interface Metric {
   /** Error message encountered querying this specific metric. */
   errorMessage?: string;
 }
+
+/** Optional parameters. */
+export interface MetricsBatchOptionalParams extends coreClient.OperationOptions {
+  /**
+   * The start time of the query. It is a string in the format 'yyyy-MM-ddTHH:mm:ss.fffZ'. If you have specified the endTime parameter, then this parameter is required.
+   * If only startTime is specified, then endTime defaults to the current time.
+   * If no time interval is specified, the default is 1 hour.
+   */
+  startTime?: Date;
+  /** The end time of the query. It is a string in the format 'yyyy-MM-ddTHH:mm:ss.fffZ'. */
+  endTime?: Date;
+  /**
+   * The interval (i.e. timegrain) of the query.
+   * *Examples: PT15M, PT1H, P1D*
+   */
+  interval?: string;
+  /**
+   * The list of aggregation types (comma separated) to retrieve.
+   * *Examples: average, minimum, maximum*
+   */
+  aggregation?: string;
+  /**
+   * The maximum number of records to retrieve per resource ID in the request.
+   * Valid only if filter is specified.
+   * Defaults to 10.
+   */
+  top?: number;
+  /**
+   * The aggregation to use for sorting results and the direction of the sort.
+   * Only one order can be specified.
+   * *Examples: sum asc*
+   */
+  orderBy?: string;
+  /** The filter is used to reduce the set of metric data returned.<br>Example:<br>Metric contains metadata A, B and C.<br>- Return all time series of C where A = a1 and B = b1 or b2<br>**filter=A eq ‘a1’ and B eq ‘b1’ or B eq ‘b2’ and C eq ‘*’**<br>- Invalid variant:<br>**filter=A eq ‘a1’ and B eq ‘b1’ and C eq ‘*’ or B = ‘b2’**<br>This is invalid because the logical or operator cannot separate two different metadata names.<br>- Return all time series where A = a1, B = b1 and C = c1:<br>**filter=A eq ‘a1’ and B eq ‘b1’ and C eq ‘c1’**<br>- Return all time series where A = a1<br>**filter=A eq ‘a1’ and B eq ‘*’ and C eq ‘*’**. */
+  filter?: string;
+}

sdk/monitor/monitor-query/swagger/metric-Batch.md

@@ -8,7 +8,7 @@
 input-file: https://raw.githubusercontent.com/Azure/azure-rest-api-specs/5f700acd3d094d8eedca381932f2e7916afd2e55/specification/monitor/data-plane/Microsoft.Insights/preview/2023-05-01-preview/metricBatch.json
 output-folder: ../src/generated/metricBatch
 package-name: "monitor-metric-batch"
-package-version: "1.2.0-beta.1"
+package-version: "1.2.0-beta.2"
 clear-output-folder: true
 generate-metadata: false
 add-credentials: false

sdk/monitor/monitor-query/test/public/metricsBatchClient.spec.ts

@@ -0,0 +1,61 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { assert } from "chai";
+import { Context } from "mocha";
+import { MetricsBatchQueryClient, MetricResultsResponseValuesItem } from "../../src";
+import {
+  RecorderAndMetricsBatchQueryClient,
+  createRecorderAndMetricsBatchQueryClient,
+  getMetricsBatchResourceIds,
+  getMetricsBatchNamespace,
+  getMetricsBatchNames,
+} from "./shared/testShared";
+
+describe.skip("MetricsBatchClient live tests", function () {
+  let resourceIds: string[];
+  let metricsNamespace: string;
+  let metricNames: string[];
+  let metricsBatchQueryClient: MetricsBatchQueryClient;
+
+  beforeEach(async function (this: Context) {
+    const recordedClient: RecorderAndMetricsBatchQueryClient =
+      await createRecorderAndMetricsBatchQueryClient();
+    resourceIds = getMetricsBatchResourceIds();
+    metricsNamespace = getMetricsBatchNamespace();
+    metricNames = getMetricsBatchNames();
+    metricsBatchQueryClient = recordedClient.client;
+  });
+
+  // afterEach(async function () {
+  //   loggerForTest.verbose("Recorder: stopping");
+  //   await recorder.stop();
+  // });
+
+  it("batch query with no resource ids", async () => {
+    try {
+      await metricsBatchQueryClient.queryBatch([], metricsNamespace, metricNames);
+      assert.fail("Code should not reach here.");
+    } catch (e) {
+      assert.equal(1, 1);
+    }
+  });
+
+  it("batch query for 2 resource ids", async () => {
+    const result: MetricResultsResponseValuesItem[] = await metricsBatchQueryClient.queryBatch(
+      resourceIds,
+      metricsNamespace,
+      metricNames
+    );
+    assert.equal(result.length, 2);
+  });
+
+  it("batch query for 1 resource id", async () => {
+    const result: MetricResultsResponseValuesItem[] = await metricsBatchQueryClient.queryBatch(
+      [resourceIds[0]],
+      metricsNamespace,
+      metricNames
+    );
+    assert.equal(result.length, 1);
+  });
+});

sdk/monitor/monitor-query/test/public/shared/testShared.ts

@@ -9,7 +9,12 @@ import {
 } from "@azure-tools/test-recorder";
 import * as assert from "assert";
 import { createClientLogger } from "@azure/logger";
-import { LogsQueryClient, LogsTable, MetricsQueryClient } from "../../../src";
+import {
+  LogsQueryClient,
+  LogsTable,
+  MetricsQueryClient,
+  MetricsBatchQueryClient,
+} from "../../../src";
 import { ExponentialRetryPolicyOptions } from "@azure/core-rest-pipeline";
 export const loggerForTest = createClientLogger("test");
 const replacementForLogsResourceId = env["LOGS_RESOURCE_ID"]?.startsWith("/")
@@ -39,6 +44,41 @@ export interface RecorderAndMetricsClient {
   recorder: Recorder;
 }
 
+export interface RecorderAndMetricsBatchQueryClient {
+  client: MetricsBatchQueryClient;
+  // recorder: Recorder;
+}
+
+export async function createRecorderAndMetricsBatchQueryClient(): Promise<RecorderAndMetricsBatchQueryClient> {
+  // await recorder.start(recorderOptions);
+  const testCredential = createTestCredential();
+  const batchEndPoint =
+    env["AZURE_MONITOR_BATCH_ENDPOINT"] ?? "https://eastus.metrics.monitor.azure.com/";
+  const client = new MetricsBatchQueryClient(batchEndPoint, testCredential);
+
+  return {
+    client: client,
+    // recorder: recorder,
+  };
+}
+
+export function getMetricsBatchResourceIds(): string[] {
+  const resourceId: string = assertEnvironmentVariable("LOGS_RESOURCE_ID");
+  return [resourceId, `${resourceId}2`];
+}
+
+export function getMetricsBatchNamespace(): string {
+  return env["AZURE_MONITOR_BATCH_NAMESPACE"] ?? "requests/count";
+}
+
+export function getMetricsBatchNames(): string[] {
+  const metricNamesString = env["AZURE_MONITOR_BATCH_METRICNAMES"];
+  if (!metricNamesString) {
+    return ["requests", "count"];
+  }
+  return metricNamesString.split(" ");
+}
+
 export const testEnv = new Proxy(envSetupForPlayback, {
   get: (target, key: string) => {
     return env[key] || target[key];