Skip to content

Commit

Permalink
Api Consistency Review feedback (#17873)
Browse files Browse the repository at this point in the history
  • Loading branch information
KarishmaGhiya authored Sep 29, 2021
1 parent 49f8480 commit 632629c
Show file tree
Hide file tree
Showing 17 changed files with 175 additions and 190 deletions.
83 changes: 31 additions & 52 deletions sdk/monitor/monitor-query/review/monitor-query.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,57 +11,33 @@ import { TokenCredential } from '@azure/core-auth';

// @public
export class AggregateBatchError extends Error {
constructor(errors: ErrorInfo[]);
// (undocumented)
constructor(errors: LogsErrorInfo[]);
errors: BatchError[];
}

// @public
export type AggregationType = "None" | "Average" | "Count" | "Minimum" | "Maximum" | "Total";

// @public
export class BatchError extends Error implements ErrorInfo {
constructor(errorInfo: ErrorInfo);
additionalProperties?: Record<string, unknown>;
export class BatchError extends Error implements LogsErrorInfo {
constructor(errorInfo: LogsErrorInfo);
code: string;
details?: ErrorDetail[];
innerError?: ErrorInfo;
message: string;
}

// @public
export const Durations: {
readonly sevenDays: "P7D";
readonly threeDays: "P3D";
readonly twoDays: "P2D";
readonly OneDay: "P1D";
readonly OneHour: "PT1H";
readonly FourHours: "PT4H";
readonly TwentyFourHours: "P1D";
readonly FourtyEightHours: "P2D";
readonly ThirtyMinutes: "PT30M";
readonly FiveMinutes: "PT5M";
readonly oneDay: "P1D";
readonly oneHour: "PT1H";
readonly fourHours: "PT4H";
readonly twentyFourHours: "P1D";
readonly fourtyEightHours: "P2D";
readonly thirtyMinutes: "PT30M";
readonly fiveMinutes: "PT5M";
};

// @public
export interface ErrorDetail {
additionalProperties?: Record<string, unknown>;
code: string;
message: string;
resources?: string[];
target?: string;
value?: string;
}

// @public
export interface ErrorInfo extends Error {
additionalProperties?: Record<string, unknown>;
code: string;
details?: ErrorDetail[];
innerError?: ErrorInfo;
message: string;
}

// @public
export interface ListMetricDefinitionsOptions extends OperationOptions {
metricNamespace?: string;
Expand All @@ -81,6 +57,11 @@ export interface LogsColumn {
// @public
export type LogsColumnType = string;

// @public
export interface LogsErrorInfo extends Error {
code: string;
}

// @public
export interface LogsQueryBatchOptions extends OperationOptions {
throwOnAnyFailure?: boolean;
Expand All @@ -90,7 +71,7 @@ export interface LogsQueryBatchOptions extends OperationOptions {
export interface LogsQueryBatchResult {
results: {
tables?: LogsTable[];
error?: ErrorInfo;
error?: LogsErrorInfo;
status?: LogsQueryResultStatus;
statistics?: Record<string, unknown>;
visualization?: Record<string, unknown>;
Expand All @@ -100,15 +81,13 @@ export interface LogsQueryBatchResult {
// @public
export class LogsQueryClient {
constructor(tokenCredential: TokenCredential, options?: LogsQueryClientOptions);
query(workspaceId: string, query: string, timespan: TimeInterval, options?: LogsQueryOptions): Promise<LogsQueryResult>;
queryBatch(batch: QueryBatch[], options?: LogsQueryBatchOptions): Promise<LogsQueryBatchResult>;
queryWorkspace(workspaceId: string, query: string, timespan: QueryTimeInterval, options?: LogsQueryOptions): Promise<LogsQueryResult>;
}

// @public
export interface LogsQueryClientOptions extends CommonClientOptions {
credentialOptions?: {
credentialScopes?: string | string[];
};
audience?: string;
endpoint?: string;
}

Expand All @@ -123,15 +102,15 @@ export interface LogsQueryOptions extends OperationOptions {

// @public
export interface LogsQueryResult {
error?: ErrorInfo;
error?: LogsErrorInfo;
statistics?: Record<string, unknown>;
status: LogsQueryResultStatus;
tables: LogsTable[];
visualization?: Record<string, unknown>;
}

// @public
export type LogsQueryResultStatus = "Partial" | "Success" | "Failed";
export type LogsQueryResultStatus = "PartialFailure" | "Success" | "Failure";

// @public
export interface LogsTable {
Expand Down Expand Up @@ -160,8 +139,8 @@ export interface Metric {

// @public
export interface MetricAvailability {
granularity?: string;
retention?: string;
timeGrain?: string;
}

// @public
Expand Down Expand Up @@ -195,16 +174,16 @@ export interface MetricNamespace {

// @public
export interface MetricsClientOptions extends CommonClientOptions {
audience?: string;
endpoint?: string;
}

// @public
export class MetricsQueryClient {
constructor(tokenCredential: TokenCredential, options?: MetricsClientOptions);
listMetricDefinitions(resourceUri: string, options?: ListMetricDefinitionsOptions): PagedAsyncIterableIterator<MetricDefinition>;
// Warning: (ae-forgotten-export) The symbol "MetricNamespace" needs to be exported by the entry point index.d.ts
listMetricNamespaces(resourceUri: string, options?: ListMetricNamespacesOptions): PagedAsyncIterableIterator<MetricNamespace_2>;
query(resourceUri: string, metricNames: string[], options?: MetricsQueryOptions): Promise<MetricsQueryResult>;
listMetricNamespaces(resourceUri: string, options?: ListMetricNamespacesOptions): PagedAsyncIterableIterator<MetricNamespace>;
queryResource(resourceUri: string, metricNames: string[], options?: MetricsQueryOptions): Promise<MetricsQueryResult>;
}

// @public
Expand All @@ -215,7 +194,7 @@ export interface MetricsQueryOptions extends OperationOptions {
metricNamespace?: string;
orderBy?: string;
resultType?: ResultType;
timespan?: TimeInterval;
timespan?: QueryTimeInterval;
top?: number;
}

Expand All @@ -227,7 +206,7 @@ export interface MetricsQueryResult {
metrics: Metric[];
namespace?: string;
resourceRegion?: string;
timespan: TimeInterval;
timespan: QueryTimeInterval;
}

// @public
Expand All @@ -253,15 +232,12 @@ export interface QueryBatch {
includeVisualization?: boolean;
query: string;
serverTimeoutInSeconds?: number;
timespan: TimeInterval;
timespan: QueryTimeInterval;
workspaceId: string;
}

// @public
export type ResultType = "Data" | "Metadata";

// @public
export type TimeInterval = {
export type QueryTimeInterval = {
startTime: Date;
endTime: Date;
} | {
Expand All @@ -274,6 +250,9 @@ export type TimeInterval = {
duration: string;
};

// @public
export type ResultType = "Data" | "Metadata";

// @public
export interface TimeSeriesElement {
data?: MetricValue[];
Expand Down
4 changes: 2 additions & 2 deletions sdk/monitor/monitor-query/samples-dev/logsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ export async function main() {
includeQueryStatistics: true
};

const result = await logsQueryClient.query(
const result = await logsQueryClient.queryWorkspace(
monitorWorkspaceId,
kustoQuery,
// The timespan is an ISO8601 formatted time (or interval). Some common aliases
// are available (like OneDay, OneHour, FoutyEightHours, etc..) but any properly formatted ISO8601
// value is valid.
{ duration: Durations.OneHour },
{ duration: Durations.oneHour },
queryLogsOptions
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ export async function main() {
additionalWorkspaces: [additionalWorkspaces1, additionalWorkspaces2]
};

const result = await logsQueryClient.query(
const result = await logsQueryClient.queryWorkspace(
monitorWorkspaceId,
kustoQuery,
// The timespan is an ISO8601 formatted time (or interval). Some common aliases
// are available (like durationOf1Day, durationOf1Hour, durationOf48Hours, etc..) but any properly formatted ISO8601
// value is valid.
{ duration: Durations.OneHour },
{ duration: Durations.oneHour },
queryLogsOptions
);

Expand Down
4 changes: 2 additions & 2 deletions sdk/monitor/monitor-query/samples-dev/metricsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ export async function main() {

if (metricNames.length > 0) {
console.log(`Picking an example list of metrics to query: ${metricNames}`);
const metricsResponse = await metricsQueryClient.query(metricsResourceId, metricNames, {
const metricsResponse = await metricsQueryClient.queryResource(metricsResourceId, metricNames, {
granularity: "PT1M",
timespan: { duration: Durations.FiveMinutes }
timespan: { duration: Durations.fiveMinutes }
});

console.log(
Expand Down
16 changes: 6 additions & 10 deletions sdk/monitor/monitor-query/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export {
LogsTable,
LogsColumn,
LogsQueryResultStatus,
ErrorInfo,
LogsErrorInfo,
BatchError,
AggregateBatchError
} from "./models/publicLogsModels";
Expand All @@ -33,20 +33,20 @@ export {
MetricsQueryOptions,
MetricsQueryResult,
TimeSeriesElement,
MetricNamespace
MetricNamespace,
MetricAvailability
} from "./models/publicMetricsModels";

export { Durations } from "./models/constants";
export { TimeInterval } from "./models/timeInterval";
export { QueryTimeInterval } from "./models/timeInterval";
//
// LogsClient: generated exports
//

export {
// TODO: these are the generated model names. We probably want to run them
// through a manual review to make them consistent with style.
LogsColumnType,
ErrorDetail
LogsColumnType
} from "./generated/logquery/src";

//
Expand All @@ -60,9 +60,5 @@ export {
MetricUnit
} from "./generated/metrics/src";

export {
AggregationType,
MetricAvailability,
MetricClass
} from "./generated/metricsdefinitions/src";
export { AggregationType, MetricClass } from "./generated/metricsdefinitions/src";
export { NamespaceClassification } from "./generated/metricsnamespaces/src";
34 changes: 26 additions & 8 deletions sdk/monitor/monitor-query/src/internal/modelConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,15 @@ import {
Metric,
MetricDefinition,
TimeSeriesElement,
createMetricsQueryResult
createMetricsQueryResult,
MetricAvailability
} from "../models/publicMetricsModels";
import { FullOperationResponse } from "@azure/core-client";
import {
convertIntervalToTimeIntervalObject,
convertTimespanToInterval
} from "../timespanConversion";
import { ErrorInfo, LogsQueryResult } from "../models/publicLogsModels";
import { LogsErrorInfo, LogsQueryResult } from "../models/publicLogsModels";

/**
* @internal
Expand Down Expand Up @@ -137,11 +138,11 @@ export function convertResponseForQueryBatch(
for (let i = 0; i < resultsCount; i++) {
const result = newResponse.results[i];
if (result.error && result.tables) {
result.status = "Partial";
result.status = "PartialFailure";
} else if (result.tables) {
result.status = "Success";
} else {
result.status = "Failed";
result.status = "Failure";
}
}
(newResponse as any)["__fixApplied"] = fixApplied;
Expand Down Expand Up @@ -309,7 +310,7 @@ export function convertResponseForMetricsDefinitions(
generatedResponse: Array<GeneratedMetricDefinition>
): Array<MetricDefinition> {
const definitions: Array<MetricDefinition> = generatedResponse?.map((genDef) => {
const { name, dimensions, displayDescription, ...rest } = genDef;
const { name, dimensions, displayDescription, metricAvailabilities, ...rest } = genDef;

const response: MetricDefinition = {
...rest
Expand All @@ -322,6 +323,18 @@ export function convertResponseForMetricsDefinitions(
response.name = name.value;
}

const mappedMetricAvailabilities:
| Array<MetricAvailability>
| undefined = metricAvailabilities?.map((genMetricAvail) => {
return {
granularity: genMetricAvail.timeGrain,
retention: genMetricAvail.retention
};
});

if (mappedMetricAvailabilities) {
response.metricAvailabilities = mappedMetricAvailabilities;
}
const mappedDimensions = dimensions?.map((dim) => dim.value);

if (mappedDimensions) {
Expand Down Expand Up @@ -422,12 +435,17 @@ export function convertBatchQueryResponseHelper(
}
}

export function mapError(error?: GeneratedErrorInfo): ErrorInfo | undefined {
export function mapError(error?: GeneratedErrorInfo): LogsErrorInfo | undefined {
if (error) {
let innermostError = error;
while (innermostError.innerError) {
innermostError = innermostError.innerError;
}

return {
...error,
name: "Error",
innerError: mapError(error.innerError)
code: error.code,
message: `${error.message}. ${innermostError.message}`
};
}
return undefined;
Expand Down
Loading

0 comments on commit 632629c

Please sign in to comment.