Merge pull request #1511 from ajantrania/starling/merge

Merge `feature/awsq` into `feature/starling`

Author: Apurva

.changes/next-release/Feature-b704a316-60d8-4cf3-bd5e-0729a22acbac.json

@@ -0,0 +1,4 @@
+{
+	"type": "Feature",
+	"description": "Auth: Delete SSO connections from quickpick with button on hover"
+}

.changes/next-release/Feature-efde9b06-c26d-4d9b-9e92-f11f4870f05e.json

@@ -0,0 +1,4 @@
+{
+	"type": "Feature",
+	"description": "Support for Amazon Q, your generative AI–powered assistant designed for work that can be tailored to your business, code, data, and operations."
+}

CONTRIBUTING.md

@@ -81,9 +81,28 @@ You can also use these NPM tasks (see `npm run` for the full list):
 
 -   Project patterns and practices: [CODE_GUIDELINES.md](./docs/CODE_GUIDELINES.md)
 -   [VS Code Extension Guidelines](https://code.visualstudio.com/api/references/extension-guidelines)
+    -   [Webview guidance](https://code.visualstudio.com/api/ux-guidelines/webviews)
 -   [VS Code API Documentation](https://code.visualstudio.com/api/references/vscode-api)
 -   [VS Code Extension Capabilities](https://code.visualstudio.com/api/extension-capabilities/common-capabilities)
 
+### Prerelease artifacts
+
+-   CI automatically publishes GitHub [prereleases](https://github.com/aws/aws-toolkit-vscode/releases)
+    for `master` and `feature/x` branches, including `.vsix` artifacts which can
+    be used to test the latest build for that branch. Each prerelease and its
+    artifact are continually updated from the HEAD of its branch.
+-   PR artifacts: each pull request is processed by an AWS CodeBuild job which
+    runs all tests and provides the build result via the _Details_ link as shown
+    below.
+    -   <img src="./docs/images/ci-artifact.png" alt="CI artifact" width="512"/>
+
+### Debug failing integration tests
+
+-   Check for recent changes in each of these projects:
+    -   https://github.com/microsoft/vscode-python (releases)
+    -   https://github.com/aws/aws-sam-cli/releases
+    -   https://github.com/aws/aws-sam-cli-app-templates/ (`master` branch, not releases!)
+
 ### Technical notes
 
 -   VSCode extensions have a [100MB](https://github.com/Microsoft/vscode-vsce/issues/179) file size limit.
@@ -170,13 +189,6 @@ To run a single test in VSCode, do any one of:
     rootTestsPath: __dirname + '/shared/sam/debugger/'
     ```
 
-### Debug failing integration tests
-
--   Check for recent changes in each of these projects:
-    -   https://github.com/microsoft/vscode-python (releases)
-    -   https://github.com/aws/aws-sam-cli/releases
-    -   https://github.com/aws/aws-sam-cli-app-templates/ (`master` branch, not releases!)
-
 ### Browser Support
 
 Running the extension in the browser (eg: [vscode.dev](https://vscode.dev/)).
@@ -192,17 +204,6 @@ You can find the coverage report at `./coverage/index.html` after running the te
 -   Exercise the code (`Extension Tests`, `Integration Tests`, etc.)
 -   Generate a report with `npm run report`
 
-### Prerelease artifacts
-
--   CI automatically publishes GitHub [prereleases](https://github.com/aws/aws-toolkit-vscode/releases)
-    for `master` and `feature/x` branches, including `.vsix` artifacts which can
-    be used to test the latest build for that branch. Each prerelease and its
-    artifact are continually updated from the HEAD of its branch.
--   PR artifacts: each pull request is processed by an AWS CodeBuild job which
-    runs all tests and provides the build result via the _Details_ link as shown
-    below.
-    -   <img src="./docs/images/ci-artifact.png" alt="CI artifact" width="512"/>
-
 ### CodeCatalyst Blueprints
 
 You can find documentation to create VSCode IDE settings for CodeCatalyst blueprints at [docs/vscode-config.md](./docs/vscode-config.md).
@@ -221,11 +222,28 @@ To send a pull request:
 2. Modify the source; focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
     - Read the [project guidelines](#guidelines), this is very important for non-trivial changes.
 3. Commit to your fork [using clear commit messages](#commit-messages).
-4. Update the changelog by running `npm run newChange`.
-    - Note: the main purpose of the `newChange` task is to avoid merge conflicts.
+4. Update the [changelog](#changelog).
 5. [Create a pull request](https://help.github.com/articles/creating-a-pull-request/).
 6. Pay attention to any CI failures reported in the pull request.
 
+### Changelog
+
+Pull requests that change customer-facing behavior (in any _describable_ way) should include
+a changelog item(s). Update the changelog via:
+
+    npm run newChange
+
+Guidelines:
+
+-   Describe the change in a way that is meaningful to the _customer_.
+    -   ❌ `Remove the cache when the connection wizard is re-launched`
+    -   ✅ `Connection wizard sometimes shows the old (stale) connection`
+-   If there are multiple unrelated changes, run `npm run newChange` for each change.
+-   **Bug Fix** changes should describe the _problem being fixed_. This tends to produce simpler,
+    more-readable descriptions. It's redundant to mention "Fixed" in the description. Example:
+    -   ❌ `Fixed S3 bug which caused filenames to be uppercase`
+    -   ✅ `S3 filenames are always uppercase`
+
 ### Commit messages
 
 Generally your PR description should be a copy-paste of your commit message(s).
@@ -238,7 +256,6 @@ Follow these [commit message guidelines](https://cbea.ms/git-commit/):
     -   Imperative voice ("Fix bug", not "Fixed"/"Fixes"/"Fixing").
 -   Body: for non-trivial or uncommon changes, explain your motivation for the
     change and contrast your implementation with previous behavior.
-
     -   Often you can save a _lot_ of words by using this simple template:
         ```
         Problem: …
@@ -306,9 +323,9 @@ Example:
 ```json
 "aws.dev.codecatalystService": {
     "region": "us-west-2",
-    "endpoint": "https://public.codecatalyst-gamma.global.api.aws",
-    "hostname": "integ.stage.REMOVE.codes",
-    "gitHostname": "git.gamma.source.caws.REMOVE",
+    "endpoint": "https://codecatalyst-gamma.example.com",
+    "hostname": "integ.stage.example.com",
+    "gitHostname": "git.gamma.source.example.com",
 }
 ```
 

docs/ARCHITECTURE.md

@@ -86,6 +86,8 @@ Complex programs often require more than just simple functions to act as entry-p
 
 ## Exceptions
 
+_See also [CODE_GUIDELINES.md](./CODE_GUIDELINES.md#exceptions)._
+
 Large applications often have a correspondingly large number of failure points. For feature-level logic, these failures are often non-recoverable. The best we can do is show the user that something went wrong and maybe offer guidance on how to fix it.
 
 Because this is such a common pattern, shared error handling logic is defined by `ToolkitError` found [here](../src/shared/errors.ts). This class provides the basic structure for errors throughout the Toolkit.

docs/CODE_GUIDELINES.md

@@ -11,6 +11,27 @@ that cannot be enforced by a `lint` build-task.
     instead of inventing new conventions.
 -   Convention: provide global editor commands as an alternative to browsing items in the Explorer.
     -   Instead of needing to visit service _Foo_ in the Explorer to _View_ its items, consider also providing a `AWS: Foo: View Item` command.
+-   [Webview guidance](https://code.visualstudio.com/api/ux-guidelines/webviews)
+-   Webview costs:
+    -   Webviews very easily lead to the [inner-platform effect](https://en.wikipedia.org/wiki/Inner-platform_effect).
+        Because they are fully isolated from vscode and its extensions, they must include any code
+        and frameworks.
+    -   Leads to extra dependencies.
+    -   Webviews inherit none of vscode's standard features. This means features like keyboard
+        shortcuts, syntax highlighting, and editor navigation, are not available to users. Instead
+        users must learn the custom _web application_ embedded in the webview.
+
+## Dependencies
+
+Dependencies can be very high-leverage if they solve a difficult problem.
+Dependencies are also a [maintenance burden](https://github.com/aws/aws-toolkit-vscode/pulls?q=is%3Apr+author%3Aapp%2Fdependabot+is%3Aclosed) and security risk
+Copy-pasting or "inlining" a dependency doesn't solve that, of course--it only hides the problem (another cost).
+So before taking on a new dependency, ask:
+
+-   is this solving a problem that is worth the cost?
+-   could the problem be solved in some other way that involves a smaller cost? For example, using
+    an isolated function with good test coverage, or a native vscode feature such as a TreeView or
+    quickpick menu.
 
 ## Naming
 
@@ -91,6 +112,13 @@ that is a net cost.
     -   Localize UI messages. Do _not_ localize log and exception messages.
     -   Use `extensionUtilities.getIdeProperties()` to automatically match IDE
         terminology (e.g. VS Code : CodeLens :: AWS Cloud9 : Inline Action)
+-   Refactoring tools allow us to avoid "premature abstraction". Avoid wrappers
+    or other abstractions until the need is clear and obvious.
+
+### Exceptions
+
+_See also [ARCHITECTURE.md](./ARCHITECTURE.md#exceptions)._
+
 -   Bubble-up error conditions, do not sink them to random hidden places (such as
     logs only), expecting callers to figure out the failure mode. If a caller
     spawns a process that fails, the caller should get an exception, callback, or
@@ -101,10 +129,22 @@ that is a net cost.
     attaching metadata to the error, or retrying the failed action. Do not just
     log and rethrow the error without additional context as to where the error occured.
 -   Do not log a stack trace unless it's truly a fatal exception. Stack traces are
-    noisy and mostly useless in production because the extension is 'bundled', removing
-    anything useful from the trace.
--   Refactoring tools allow us to avoid "premature abstraction". Avoid wrappers
-    until the need is clear and obvious.
+    noisy and mostly useless in production because the extension is bundled
+    (webpacked), removing anything useful from the trace.
+    -   When _debugging_ the extension, you can tell the debugger to break on
+        exceptions, so logging the stacktrace is unnecessary there.
+-   Do not use multiple logger calls to log what is semantically a single
+    message. Use a string template or printf-style syntax (`%s` ) to format the
+    message:
+    -   GOOD:
+        ```
+        getLogger().error(`Failed to create %s: %s`, foo, (err as Error).message)
+        ```
+    -   BAD:
+        ```
+        getLogger().error(`Failed to create %s`, foo)
+        getLogger().error(err)
+        ```
 
 ## Test guidelines
 

docs/develop.md

@@ -5,8 +5,22 @@ Notes about the codebase, its utilities, special globals, etc.
 ## VSCode context keys
 
 VScode extensions can use vscode 'setContext' command to set special context keys which are
-available in `package.json`. This extension sets the following keys:
+available in `package.json`.
 
+### Defining a new setContext key
+
+If you must define a new key,
+
+-   Prefix it with `aws.` as recommended by the [vscode docs](https://code.visualstudio.com/api/extension-guides/command#using-a-custom-when-clause-context).
+-   Use brevity. Less is more.
+-   Document it in the list below.
+
+### Toolkit setContext keys
+
+We set the following keys:
+
+-   `isCloud9`: This is hardcoded by Cloud9 itself, not the Toolkit.
+    -   Cloud9 _does not support setContext_. So this is the only usable key there.
 -   `aws.codecatalyst.connected`: CodeCatalyst connection is active.
 -   `CODEWHISPERER_ENABLED`: CodeWhisperer connection is active.
 -   `aws.isDevMode`: AWS Toolkit is running in "developer mode".

docs/telemetry.md

@@ -1,21 +1,35 @@
 # Telemetry
 
+## Development
+
 See [aws-toolkit-common/telemetry](https://github.com/aws/aws-toolkit-common/tree/main/telemetry#telemetry) for full details about defining telemetry metrics.
 
 -   You can define new metrics during development by adding items to
     [telemetry/vscodeTelemetry.json](https://github.com/aws/aws-toolkit-vscode/blob/21ca0fca26d677f105caef81de2638b2e4796804/src/shared/telemetry/vscodeTelemetry.json).
-    -   Building the project will trigger the `generateClients` build task, which generates new symbols in `shared/telemetry/telemetry`, which you can import via:
+    -   The `generateClients` build task generates new symbols in `shared/telemetry/telemetry`, which you can import via:
         ```
-        import { telemetry } from '../../shared/telemetry/telemetry'
+        import { telemetry } from '/shared/telemetry/telemetry'
         ```
-    -   The metrics defined in `vscodeTelemetry.json` should be upstreamed to [aws-toolkit-common](https://github.com/aws/aws-toolkit-common/blob/main/telemetry/definitions/commonDefinitions.json) after launch (at the latest).
+    -   When your feature is released, the "development" metrics you defined in `vscodeTelemetry.json` should be upstreamed to [aws-toolkit-common](https://github.com/aws/aws-toolkit-common/blob/main/telemetry/definitions/commonDefinitions.json).
 -   Metrics are dropped (not posted to the service) if the extension is running in [CI or other
     automation tasks](https://github.com/aws/aws-toolkit-vscode/blob/21ca0fca26d677f105caef81de2638b2e4796804/src/shared/vscode/env.ts#L71-L73).
     -   You can always _test_ telemetry via [assertTelemetry()](https://github.com/aws/aws-toolkit-vscode/blob/21ca0fca26d677f105caef81de2638b2e4796804/src/test/testUtil.ts#L164), regardless of the current environment.
 
-### Incrementally Building a Metric
+## Guidelines
+
+-   Use `run()` where possible. It automatically sets the `result` and `reason` fields. See below for details.
+    -   `run()` gets the `reason` value from the `Error.code` property of any exception that is thrown.
+    -   Your code can throw `ToolkitError` with a `code` field to communicate errors, validation issues, or [cancellation](https://github.com/aws/aws-toolkit-vscode/blob/06661f84530c6b5331579871d685515700b7767e/src/auth/sso/model.ts#L138). See below.
+-   The `reason` and `result` fields are standard metric fields shared by all Toolkits (VSCode, JetBrains, VisualStudio).
+    They should be used instead of special-purpose metrics or fields.
+-   `result` allows the Toolkits team to monitor all features for potential regressions.
+-   `reason` gives insight into the cause of a `result=Failed` metric.
+-   `telemetry.record()` called during a `telemetry.foo.run(…)` context will automatically annotate the current `foo` metric.
+    -   For example, the cloudwatch logs feature adds `hasTimeFilter` info its metrics by [calling telemetry.record()](https://github.com/aws/aws-toolkit-vscode/blob/06661f84530c6b5331579871d685515700b7767e/src/cloudWatchLogs/cloudWatchLogsUtils.ts#L21-L24).
 
-In certain scenarios, you may have some code that has multiple stages/steps in its execution.
+## Incrementally Building a Metric
+
+User actions or other features may have multiple stages/steps, called a "workflow" or just "flow". A telemetry ["trace"](https://opentelemetry.io/docs/concepts/signals/traces/) captures a flow as tree of ["spans"](https://opentelemetry.io/docs/concepts/signals/traces/#spans).
 
 For example, `setupThing()` has multiple steps until it is completed, ending with `lastSetupStep()`.
 
@@ -49,12 +63,24 @@ Here we emitted a final metric based on the failure or success of the entire exe
 
 <br>
 
-But usually code is not flat and there are many nested calls. If something goes wrong during the execution it would be useful to have more specific information at the area of failure. So what we can do is use `run()` along with `record()`.
+But usually code is not flat and there are many nested calls. If something goes wrong during the execution it would be useful to have more specific information at the area of failure. For that we can use `run()` along with `telemetry.record()`.
+
+`run()` accepts a callback, and when the callback is executed, any uses of `telemetry.record()` at _any nesting level_ during execution of that callback, will update the
+attributes of the ["current metric"](https://github.com/aws/aws-toolkit-vscode/blob/13cb98d5315092ddc9eb5ba898e5f26810dada25/src/shared/telemetry/spans.ts#L233).
+And at the end (that is, when `run()` returns) we will emit a single metric with the last updated attributes.
+[Example](https://github.com/aws/aws-toolkit-vscode/blob/06661f84530c6b5331579871d685515700b7767e/src/cloudWatchLogs/cloudWatchLogsUtils.ts#L21-L24)
+
+When an exception is thrown from a `run()` context, `run()` will [automatically set](https://github.com/aws/aws-toolkit-vscode/blob/a583825bec6cb68c4942fa60d185644833528532/src/shared/errors.ts#L273-L289)
+the `reason` field based on the Error `code` field. You can explicitly set `code` when throwing
+a `ToolkitError`, for [example](https://github.com/aws/aws-toolkit-vscode/blob/d08e59952a6c75a5c6c00fdc464e26750c0e85f5/src/auth/auth.ts#L530):
+
+    throw new ToolkitError('No sso-session name found in ~/.aws/config', { code: 'NoSsoSessionName' })
+
+Note: prefer reason codes with a format similar to existing codes (not sentences). You can find existing codes by searching the codebase:
 
-`run()` takes in a callable, and when the callable is executed, any uses of `record()` within that callable will update the
-attributes of the specific metric. And at the end we will emit a single metric with the last updated attributes.
+    git grep 'code: '
 
-For example:
+### Example
 
 ```typescript
 setupThing()