Skip to content

Conversation

@Hweinstock
Copy link
Contributor

Problem

The EC2 ARN sneaks its away into the logs via the Ec2InstanceNode tooltip property from packages/core/src/awsService/ec2/explorer/ec2InstanceNode.ts. When we run any command on this instance, we get the following logs with the middle section omitted for brevity.

2024-12-04 17:25:10.859 [debug] command: running "aws.ec2.openTerminal" with arguments: [
  {
    collapsibleState: 0,
    label: 'testInstance ({INSTANCE_ID}) RUNNING',
   ...
    contextValue: 'awsEc2RunningNode',
    iconPath: { id: 'pass', color: undefined },
    tooltip: 'testInstance\n' +
      '{INSTANCE_ID}\n' +
      'running\n' +
      'arn:aws:ec2:us-east-1:{ACCOUNT_ID}:instance/{INSTANCE_ID}',
    id: '{INSTANCE_ID}'
  },
  undefined
]

The actual AWS account ID in use is included in the logs.

What makes this difficult is that this node is passed directly from VSCode here:

Commands.register('aws.ec2.openTerminal', async (node?: Ec2InstanceNode) => {
await telemetry.ec2_connectToInstance.run(async (span) => {
span.record({ ec2ConnectionType: 'ssm' })
await openTerminal(connectionManagers, node)
})
}),

and is processed by our commands wrapper here:
async function runCommand<T extends Callback>(fn: T, info: CommandInfo<T>): Promise<ReturnType<T> | void> {
const { id, args, label, logging, compositeKey } = { logging: true, ...info }
const logger = logging ? getLogger() : new NullLogger()
const threshold = info.telemetryThrottleMs ?? defaultTelemetryThrottleMs
const instrumenter = logging
? getInstrumenter({ id, args, compositeKey: compositeKey ?? {} }, threshold || 0, info.telemetryName)
: undefined
logger.debug(
`command: running ${label} with arguments: %O`,
partialClone(args, 3, ['clientSecret', 'accessToken', 'refreshToken'], '[omitted]')
)

The wrapper is logging the node directly from vscode, not giving us a chance to use partialClone on it first.

Solution

  • omit all tooltips from the logs, since this is usually redundant information anyway.

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

github-actions bot commented Dec 4, 2024

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@Hweinstock Hweinstock marked this pull request as ready for review December 4, 2024 22:49
@Hweinstock Hweinstock requested a review from a team as a code owner December 4, 2024 22:49
@justinmk3 justinmk3 merged commit e46500e into aws:feature/postreinvent Dec 4, 2024
22 of 24 checks passed
@Hweinstock Hweinstock deleted the ec2/noArnInLogs branch December 5, 2024 16:20
@Hweinstock Hweinstock restored the ec2/noArnInLogs branch December 5, 2024 16:55
@Hweinstock Hweinstock deleted the ec2/noArnInLogs branch December 5, 2024 16:56
karanA-aws pushed a commit to karanA-aws/aws-toolkit-vscode that referenced this pull request Jan 17, 2025
## Problem
The EC2 ARN sneaks its away into the logs via the `Ec2InstanceNode`
tooltip property from
`packages/core/src/awsService/ec2/explorer/ec2InstanceNode.ts`. When we
run any command on this instance, we get the following logs with the
middle section omitted for brevity.
``` 
2024-12-04 17:25:10.859 [debug] command: running "aws.ec2.openTerminal" with arguments: [
  {
    collapsibleState: 0,
    label: 'testInstance ({INSTANCE_ID}) RUNNING',
   ...
    contextValue: 'awsEc2RunningNode',
    iconPath: { id: 'pass', color: undefined },
    tooltip: 'testInstance\n' +
      '{INSTANCE_ID}\n' +
      'running\n' +
      'arn:aws:ec2:us-east-1:{ACCOUNT_ID}:instance/{INSTANCE_ID}',
    id: '{INSTANCE_ID}'
  },
  undefined
]
```
The actual AWS account ID in use is included in the logs. 

What makes this difficult is that this node is passed directly from
VSCode here:

https://github.com/aws/aws-toolkit-vscode/blob/d74f96c61f79716edf8a9a706a86c587887d3b9b/packages/core/src/awsService/ec2/activation.ts#L32-L37
and is processed by our commands wrapper here: 
https://github.com/aws/aws-toolkit-vscode/blob/d74f96c61f79716edf8a9a706a86c587887d3b9b/packages/core/src/shared/vscode/commands2.ts#L649-L660
The wrapper is logging the node directly from vscode, not giving us a
chance to use `partialClone` on it first.
## Solution
- omit all tooltips from the logs, since this is usually redundant
information anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants