Merge aws#769 MetricBase common fields

justinmk3 · web-flow · commit d18e9f11381c · 2024-06-28T09:27:57.000-07:00
diff --git a/telemetry/definitions/commonDefinitions.json b/telemetry/definitions/commonDefinitions.json
@@ -130,7 +130,7 @@
         {
             "name": "awsAccount",
             "type": "string",
-            "description": "AWS account ID associated with a metric. \"n/a\" if credentials are not available. \"not-set\" if credentials are not selected. \"invalid\" if account ID cannot be obtained."
+            "description": "AWS account ID associated with a metric.\n- \"n/a\" if credentials are not available.\n- \"not-set\" if credentials are not selected.\n- \"invalid\" if account ID cannot be obtained."
         },
         {
             "name": "awsFiletype",
@@ -155,7 +155,7 @@
         {
             "name": "awsRegion",
             "type": "string",
-            "description": "AWS Region associated with a metric"
+            "description": "AWS Region associated with a metric\n- \"n/a\" if not associated with a region.\n- \"not-set\" if metric is associated with a region, but region is unknown."
         },
         {
             "name": "causedBy",
@@ -1133,7 +1133,7 @@
         {
             "name": "httpStatusCode",
             "type": "string",
-            "description": "Describes the HTTP status code for request made. The semantics are contextual based off of other fields (e.g. `requestId`)"
+            "description": "HTTP status code for the request (if any) associated with a metric."
         },
         {
             "name": "iamResourceType",
@@ -1303,7 +1303,7 @@
         {
             "name": "requestId",
             "type": "string",
-            "description": "A generic request ID field. The semantics are contextual based off of other fields (e.g. `requestServiceType`). For example, an event with `requestServiceType: s3` means that the request ID is associated with an S3 API call. Events that cover mutliple API calls should use the request ID of the most recent call."
+            "description": "Request ID (if any) associated with a metric. For example, an event with `requestServiceType: s3` means that the request ID is associated with an S3 API call. Events that cover multiple API calls should use the request ID of the most recent call."
         },
         {
             "name": "requestServiceType",
diff --git a/telemetry/telemetryformat.md b/telemetry/telemetryformat.md
@@ -48,24 +48,47 @@ These are then used to generate functions that take arguments pertaining to metr
 
 ### Global Arguments
 
-Global arguments that can be appended to any generated telemetry call.
+Global properties that can annotate any telemetry event.
 
-```
+```javascript
 // The time that the event took place
-createTime?: Date
-// Value based on unit and call type
-value?: number
+createTime: Date
+// AWS account ID associated with a metric.
+// - "n/a" if credentials are not available.
+// - "not-set" if credentials are not selected.
+// - "invalid" if account ID cannot be obtained.
+awsAccount: string
+// AWS Region associated with a metric.
+// - "n/a" if not associated with a region.
+// - "not-set" if metric is associated with a region, but region is unknown.
+awsRegion: string
+// The duration of the operation in milliseconds.
+duration: number
+// HTTP status code for the request (if any) associated with a metric.
+httpStatusCode: string
 // The language-related user preference information. Examples: en-US, en-GB, etc.
-string?: locale
-// The AWS account ID associated with a metric
-// If a metric is not associated with credentials: "n/a"
-// If a metric is associated with credentials, but credentials are not selected: "not-set"
-// If a metric is associated with credentials, but an account ID cannot be obtained: "invalid"
-string? awsAccount
-// The AWS Region associated with a metric
-// If a metric is not associated with a region: "n/a"
-// If a metric is associated with a region, but no region is known: "not-set"
-string? awsRegion
+locale: string
+// Reason code or name for an event (when result=Succeeded) or error (when result=Failed).
+// Unlike the `reasonDesc` field, this should be a stable/predictable name for
+// a class of events or errors (typically the exception name, e.g. FileIOException).
+reason: string
+// Error message detail. May contain arbitrary message details (unlike the
+// `reason` field), but should be truncated (recommendation: 200 chars).
+reasonDesc: string
+// Request ID (if any) associated with a metric.
+// For example, an event with `requestServiceType: s3` means that the request ID
+// is associated with an S3 API call. Events that cover mutliple API calls
+// should use the request ID of the most recent call.
+requestId: string
+// Per-request service identifier. Unlike `serviceType` (which describes the
+// originator of the request), this describes the request itself.
+requestServiceType: string
+// The result of the operation.
+result: Result
+// Indicates that the metric was not caused by an explicit user action.
+passive: boolean
+// Value based on unit and call type
+value: number
 ```
 
 If not specified `createTime` defaults to UTC now, `value` defaults to `1.0`.
diff --git a/telemetry/vscode/src/generate.ts b/telemetry/vscode/src/generate.ts
@@ -121,7 +121,19 @@ function getTypeOrThrow(types: MetadataType[] = [], name: string) {
 }
 
 const baseName = 'MetricBase'
-const commonMetadata = ['result', 'reason', 'reasonDesc', 'duration', 'awsRegion']
+
+const commonMetadata = [
+  'awsAccount',
+  'awsRegion',
+  'duration',
+  'httpStatusCode',
+  'reason',
+  'reasonDesc',
+  'requestId',
+  'requestServiceType',
+  'result',
+]
+
 const passive: PropertySignatureStructure = {
     isReadonly: true,
     hasQuestionToken: true,
diff --git a/telemetry/vscode/src/parser.ts b/telemetry/vscode/src/parser.ts
@@ -49,12 +49,11 @@ export function validateInput(fileText: string, fileName: string): MetricDefinit
         const input = JSON.parse(fileText)
         const valid = jsonValidator(input)
         if (!valid) {
-            console.error('validating schema failed!')
             throw jsonValidator.errors
         }
         return input as MetricDefinitionRoot
     } catch (errors) {
-        console.error(`Error while trying to parse the definitions file ${fileName}: ${JSON.stringify(errors)}`)
-        throw new Error('Failed to parse')
+        const msg = `Failed to parse definitions file ${fileName}: ${JSON.stringify(errors)}`
+        throw new Error(msg)
     }
 }
diff --git a/telemetry/vscode/test/resources/generatorOutput.ts b/telemetry/vscode/test/resources/generatorOutput.ts
@@ -3,16 +3,33 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 export interface MetricBase {
-    /** The result of the operation */
-    readonly result?: Result
+    /**
+     * AWS account ID associated with a metric.
+     * - "n/a" if credentials are not available.
+     * - "not-set" if credentials are not selected.
+     * - "invalid" if account ID cannot be obtained.
+     */
+    readonly awsAccount?: string
+    /**
+     * AWS Region associated with a metric
+     * - "n/a" if not associated with a region.
+     * - "not-set" if metric is associated with a region, but region is unknown.
+     */
+    readonly awsRegion?: string
+    /** The duration of the operation in milliseconds */
+    readonly duration?: number
+    /** HTTP status code for the request (if any) associated with a metric. */
+    readonly httpStatusCode?: string
     /** Reason code or name for an event (when result=Succeeded) or error (when result=Failed). Unlike the `reasonDesc` field, this should be a stable/predictable name for a class of events or errors (typically the exception name, e.g. FileIOException). */
     readonly reason?: string
     /** Error message detail. May contain arbitrary message details (unlike the `reason` field), but should be truncated (recommendation: 200 chars). */
     readonly reasonDesc?: string
-    /** The duration of the operation in milliseconds */
-    readonly duration?: number
-    /** AWS Region associated with a metric */
-    readonly awsRegion?: string
+    /** Request ID (if any) associated with a metric. For example, an event with `requestServiceType: s3` means that the request ID is associated with an S3 API call. Events that cover multiple API calls should use the request ID of the most recent call. */
+    readonly requestId?: string
+    /** Per-request service identifier. Unlike `serviceType` (which describes the originator of the request), this describes the request itself. */
+    readonly requestServiceType?: string
+    /** The result of the operation */
+    readonly result?: Result
     /** A flag indicating that the metric was not caused by the user. */
     readonly passive?: boolean
     /** @deprecated Arbitrary "value" of the metric. */
diff --git a/telemetry/vscode/test/resources/generatorOverrideOutput.ts b/telemetry/vscode/test/resources/generatorOverrideOutput.ts
@@ -3,16 +3,33 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 export interface MetricBase {
-    /** The result of the operation */
-    readonly result?: Result
+    /**
+     * AWS account ID associated with a metric.
+     * - "n/a" if credentials are not available.
+     * - "not-set" if credentials are not selected.
+     * - "invalid" if account ID cannot be obtained.
+     */
+    readonly awsAccount?: string
+    /**
+     * AWS Region associated with a metric
+     * - "n/a" if not associated with a region.
+     * - "not-set" if metric is associated with a region, but region is unknown.
+     */
+    readonly awsRegion?: string
+    /** The duration of the operation in milliseconds */
+    readonly duration?: number
+    /** HTTP status code for the request (if any) associated with a metric. */
+    readonly httpStatusCode?: string
     /** Reason code or name for an event (when result=Succeeded) or error (when result=Failed). Unlike the `reasonDesc` field, this should be a stable/predictable name for a class of events or errors (typically the exception name, e.g. FileIOException). */
     readonly reason?: string
     /** Error message detail. May contain arbitrary message details (unlike the `reason` field), but should be truncated (recommendation: 200 chars). */
     readonly reasonDesc?: string
-    /** The duration of the operation in milliseconds */
-    readonly duration?: number
-    /** AWS Region associated with a metric */
-    readonly awsRegion?: string
+    /** Request ID (if any) associated with a metric. For example, an event with `requestServiceType: s3` means that the request ID is associated with an S3 API call. Events that cover multiple API calls should use the request ID of the most recent call. */
+    readonly requestId?: string
+    /** Per-request service identifier. Unlike `serviceType` (which describes the originator of the request), this describes the request itself. */
+    readonly requestServiceType?: string
+    /** The result of the operation */
+    readonly result?: Result
     /** A flag indicating that the metric was not caused by the user. */
     readonly passive?: boolean
     /** @deprecated Arbitrary "value" of the metric. */

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,7 @@`
`130`	`130`	`{`
`131`	`131`	`"name": "awsAccount",`
`132`	`132`	`"type": "string",`
`133`		`- "description": "AWS account ID associated with a metric. \"n/a\" if credentials are not available. \"not-set\" if credentials are not selected. \"invalid\" if account ID cannot be obtained."`
	`133`	`+ "description": "AWS account ID associated with a metric.\n- \"n/a\" if credentials are not available.\n- \"not-set\" if credentials are not selected.\n- \"invalid\" if account ID cannot be obtained."`
`134`	`134`	`},`
`135`	`135`	`{`
`136`	`136`	`"name": "awsFiletype",`
`@@ -155,7 +155,7 @@`
`155`	`155`	`{`
`156`	`156`	`"name": "awsRegion",`
`157`	`157`	`"type": "string",`
`158`		`- "description": "AWS Region associated with a metric"`
	`158`	`+ "description": "AWS Region associated with a metric\n- \"n/a\" if not associated with a region.\n- \"not-set\" if metric is associated with a region, but region is unknown."`
`159`	`159`	`},`
`160`	`160`	`{`
`161`	`161`	`"name": "causedBy",`
`@@ -1133,7 +1133,7 @@`
`1133`	`1133`	`{`
`1134`	`1134`	`"name": "httpStatusCode",`
`1135`	`1135`	`"type": "string",`
`1136`		- "description": "Describes the HTTP status code for request made. The semantics are contextual based off of other fields (e.g. `requestId`)"
	`1136`	`+ "description": "HTTP status code for the request (if any) associated with a metric."`
`1137`	`1137`	`},`
`1138`	`1138`	`{`
`1139`	`1139`	`"name": "iamResourceType",`
`@@ -1303,7 +1303,7 @@`
`1303`	`1303`	`{`
`1304`	`1304`	`"name": "requestId",`
`1305`	`1305`	`"type": "string",`
`1306`		- "description": "A generic request ID field. The semantics are contextual based off of other fields (e.g. `requestServiceType`). For example, an event with `requestServiceType: s3` means that the request ID is associated with an S3 API call. Events that cover mutliple API calls should use the request ID of the most recent call."
	`1306`	+ "description": "Request ID (if any) associated with a metric. For example, an event with `requestServiceType: s3` means that the request ID is associated with an S3 API call. Events that cover multiple API calls should use the request ID of the most recent call."
`1307`	`1307`	`},`
`1308`	`1308`	`{`
`1309`	`1309`	`"name": "requestServiceType",`
Original file line number	Diff line number	Diff line change
`@@ -49,12 +49,11 @@ export function validateInput(fileText: string, fileName: string): MetricDefinit`
`49`	`49`	`const input = JSON.parse(fileText)`
`50`	`50`	`const valid = jsonValidator(input)`
`51`	`51`	`if (!valid) {`
`52`		`- console.error('validating schema failed!')`
`53`	`52`	`throw jsonValidator.errors`
`54`	`53`	`}`
`55`	`54`	`return input as MetricDefinitionRoot`
`56`	`55`	`} catch (errors) {`
`57`		- console.error(`Error while trying to parse the definitions file ${fileName}: ${JSON.stringify(errors)}`)
`58`		`- throw new Error('Failed to parse')`
	`56`	+ const msg = `Failed to parse definitions file ${fileName}: ${JSON.stringify(errors)}`
	`57`	`+ throw new Error(msg)`
`59`	`58`	`}`
`60`	`59`	`}`