Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ jobs:
- name: Typecheck the renderer
working-directory: packages/aws-durable-execution-sdk-js-telemetry-vscode/webview-ui
run: npx tsc --noEmit
- name: Test the renderer
working-directory: packages/aws-durable-execution-sdk-js-telemetry-vscode/webview-ui
run: npm test
- name: Bundle the VS Code extension (renderer + extension host)
run: npm run build -w packages/aws-durable-execution-sdk-js-telemetry-vscode
- name: Bundle the desktop app
Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Telemetry MCP 1.1.0

- The server asks you to confirm, once per session, that giving it read-only
credentials is your responsibility. It no longer checks the credentials, so
`test_destination` and blocked data-tool results no longer return a
`writeAccess` field. On S3 + Athena the prompt also names a result location
inside the table's data. See the
[release notes](release-notes/telemetry-mcp-1.1.0.md).

### Coordinated release summary

Core 3.0.0, OTel 2.0.0 and Testing SDK 1.3.2 release together. The entries
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe("getTableLocation", () => {
expect(await getTableLocation(table)).toBeUndefined();
});

it("rethrows any other Glue error, so the write check reports unknown", async () => {
it("rethrows any other Glue error, so the location check reports undetermined", async () => {
const err = Object.assign(new Error("denied"), {
name: "AccessDeniedException",
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
jest.mock("./athena", () => ({
getTableLocation: jest.fn(),
getWorkGroupOutput: jest.fn(),
}));

import { getTableLocation, getWorkGroupOutput } from "./athena";
import { checkAthenaResultLocation } from "./athenaResultLocation";
import type { TelemetryConfig } from "./configCore";

function cfg(overrides: Partial<TelemetryConfig>): TelemetryConfig {
return {
region: "us-east-1",
awsProfile: undefined,
destinationType: "aurora",
auroraResourceArn: "arn:rds",
auroraSecretArn: "arn:secret",
auroraDatabase: "postgres",
redshiftDatabase: "dev",
redshiftWorkgroupName: "wg",
redshiftClusterIdentifier: "",
redshiftDbUser: "",
redshiftSecretArn: "",
athenaDatabase: "db",
athenaTable: "durable_execution_telemetry",
athenaWorkgroup: "",
athenaOutputLocation: "",
athenaS3Location: "",
...overrides,
} as TelemetryConfig;
}

beforeEach(() => jest.resetAllMocks());

describe("checkAthenaResultLocation", () => {
const location = getTableLocation as jest.Mock;
const workgroup = getWorkGroupOutput as jest.Mock;
const credentials = (() => Promise.resolve({})) as never;
beforeEach(() => workgroup.mockResolvedValue({ enforced: false }));

it("is inside when the result location is inside the table's data", async () => {
location.mockResolvedValue("s3://bucket/telemetry");
const r = await checkAthenaResultLocation(
cfg({ athenaOutputLocation: "s3://bucket/telemetry/athena-results/" }),
credentials,
);
expect(r.kind).toBe("inside");
});

it("is outside when only a name prefix matches", async () => {
// A plain prefix test would treat "telemetry-results" as inside "telemetry".
location.mockResolvedValue("s3://bucket/telemetry");
const r = await checkAthenaResultLocation(
cfg({ athenaOutputLocation: "s3://bucket/telemetry-results/" }),
credentials,
);
expect(r).toEqual({ kind: "outside" });
});

it("reads the primary workgroup when none is configured", async () => {
await checkAthenaResultLocation(cfg({}), credentials);
expect(workgroup).toHaveBeenCalledWith(
expect.objectContaining({ workgroup: "primary" }),
);
});

it("uses the workgroup's location when the settings set none", async () => {
workgroup.mockResolvedValue({
enforced: false,
outputLocation: "s3://bucket/telemetry/r/",
});
location.mockResolvedValue("s3://bucket/telemetry/");
expect((await checkAthenaResultLocation(cfg({}), credentials)).kind).toBe(
"inside",
);
});

it("uses an enforced workgroup location instead of the configured one", async () => {
workgroup.mockResolvedValue({
enforced: true,
outputLocation: "s3://bucket/telemetry/r/",
});
location.mockResolvedValue("s3://bucket/telemetry/");
const r = await checkAthenaResultLocation(
cfg({ athenaWorkgroup: "team", athenaOutputLocation: "s3://elsewhere/" }),
credentials,
);
expect(r.kind).toBe("inside");
expect(workgroup).toHaveBeenCalledWith(
expect.objectContaining({ workgroup: "team" }),
);
});

it("uses athenaS3Location when the table does not exist yet", async () => {
location.mockResolvedValue(undefined);
const r = await checkAthenaResultLocation(
cfg({
athenaOutputLocation: "s3://b/data/results/",
athenaS3Location: "s3://b/data/",
}),
credentials,
);
expect(r.kind).toBe("inside");
});

it.each([
["no result location anywhere", {}, "s3://b/data/"],
[
"no data location",
{ athenaOutputLocation: "s3://b/results/" },
undefined,
],
] as const)("is undetermined with %s", async (_name, overrides, table) => {
location.mockResolvedValue(table);
expect(
(await checkAthenaResultLocation(cfg(overrides), credentials)).kind,
).toBe("undetermined");
});

it("is undetermined, with the error, when the workgroup cannot be read", async () => {
workgroup.mockRejectedValue(new Error("AccessDenied: athena:GetWorkGroup"));
const r = await checkAthenaResultLocation(
cfg({ athenaOutputLocation: "s3://b/results/" }),
credentials,
);
expect(r).toEqual({
kind: "undetermined",
reason: expect.stringContaining("AccessDenied: athena:GetWorkGroup"),
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/**
* Where Athena writes query results, relative to the table's data.
*
* Athena writes a result file for every query, including Test connection's
* `SELECT 1`. If the result location is inside the table's data location, the
* table reads those files as records, so every query adds rows. Read-only
* credentials do not prevent this, because Athena must be allowed to write its
* results. So Test connection checks the location before it runs its probe,
* and the MCP server names it in the consent prompt before the first query.
*/
import type { TelemetryConfig } from "./configCore";
import type { resolveCredentials } from "./configCore";
import { getTableLocation, getWorkGroupOutput } from "./athena";

/**
* Ends an S3 URI with `/`, so a prefix comparison cannot match `s3://b/data`
* against `s3://b/data-results`.
*/
function withTrailingSlash(uri: string): string {
const trimmed = uri.trim();
return trimmed.endsWith("/") ? trimmed : `${trimmed}/`;
}

/**
* Where Athena writes query results relative to the table's data.
*
* - `inside`: every query writes a result file the table then reads as rows.
* - `outside`: queries do not add rows.
* - `undetermined`: the location could not be established; `reason` says why.
*
* A workgroup with EnforceWorkGroupConfiguration replaces the location a query
* submits, so the workgroup is read (with `athena:GetWorkGroup`) even when
* `athenaOutputLocation` is set. Athena uses `primary` when no workgroup is
* configured. Test connection calls this before its Athena probe, because the
* probe itself writes a result file. The MCP consent prompt calls it too.
*/
export type AthenaResultLocation =
| { kind: "inside"; finding: string }
| { kind: "outside" }
| { kind: "undetermined"; reason: string };

export async function checkAthenaResultLocation(
cfg: TelemetryConfig,
credentials: ReturnType<typeof resolveCredentials>,
): Promise<AthenaResultLocation> {
let outputLocation: string | undefined;
let dataLocation: string | undefined;
try {
const workgroup = await getWorkGroupOutput({
region: cfg.region,
credentials,
workgroup: cfg.athenaWorkgroup || "primary",
});
outputLocation = workgroup.enforced
? workgroup.outputLocation
: cfg.athenaOutputLocation || workgroup.outputLocation;
const tableLocation = await getTableLocation({
region: cfg.region,
credentials,
database: cfg.athenaDatabase,
table: cfg.athenaTable,
});
dataLocation = tableLocation ?? (cfg.athenaS3Location || undefined);
} catch (err) {
return {
kind: "undetermined",
reason: `Could not read the Athena workgroup or Glue table: ${errorMessage(err)}`,
};
}
if (!outputLocation) {
return {
kind: "undetermined",
reason:
"Neither the settings nor the Athena workgroup set a result location.",
};
}
if (!dataLocation) {
return {
kind: "undetermined",
reason: "The table does not exist and no S3 data location is set.",
};
}
if (
withTrailingSlash(outputLocation).startsWith(
withTrailingSlash(dataLocation),
)
) {
return {
kind: "inside",
finding:
`Athena writes query results to ${outputLocation}, which is inside ` +
`the table's data location ${dataLocation}. The table reads those ` +
`result files as records, so every query adds rows. Move the result ` +
`location outside the data location.`,
};
}
return { kind: "outside" };
}

function errorMessage(err: unknown): string {
return err instanceof Error ? err.message : String(err);
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,14 @@ jest.mock("./opensearch", () => ({
jest.mock("./configCore", () => ({
resolveCredentials: jest.fn(() => ({})),
}));
jest.mock("./writeAccess", () => ({
checkWriteAccess: jest.fn(async () => ({
status: "unverified",
reason: "r",
})),
jest.mock("./athenaResultLocation", () => ({
// Default: results land outside the data, so the Athena probe may run.
checkAthenaResultLocation: jest.fn(async () => ({ kind: "outside" })),
}));

import type { TelemetryConfig } from "./configCore";
import { testDestination } from "./destinationTest";
import { checkAthenaResultLocation, checkWriteAccess } from "./writeAccess";
import { checkAthenaResultLocation } from "./athenaResultLocation";
import { tableExists, runAthenaQuery } from "./athena";
import { runAuroraQuery } from "./aurora";
import { runRedshiftQuery } from "./redshift";
Expand Down Expand Up @@ -466,33 +462,3 @@ describe("testDestination — CloudWatch Logs", () => {
expect(r.checks.at(-1)?.detail).toMatch(/first invocation/);
});
});

describe("testDestination — write-access check", () => {
const aurora = baseCfg({
destinationType: "aurora",
auroraResourceArn: "arn:aws:rds:us-east-1:1:cluster:c",
auroraSecretArn: "arn:aws:secretsmanager:us-east-1:1:secret:s",
});

it("attaches the write-access result when every check passed", async () => {
(runAuroraQuery as jest.Mock).mockResolvedValue({ rows: [] });
(checkWriteAccess as jest.Mock).mockResolvedValueOnce({
status: "can-write",
findings: ["The database user is a superuser."],
});
const r = await testDestination(aurora);
expect(r.ok).toBe(true);
expect(r.writeAccess).toEqual({
status: "can-write",
findings: ["The database user is a superuser."],
});
});

it("skips the write-access check when a check failed", async () => {
(runAuroraQuery as jest.Mock).mockRejectedValue(new Error("unreachable"));
const r = await testDestination(aurora);
expect(r.ok).toBe(false);
expect(r.writeAccess).toBeUndefined();
expect(checkWriteAccess).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ import { tableExists, runAthenaQuery } from "./athena";
import { runAuroraQuery } from "./aurora";
import { runRedshiftQuery } from "./redshift";
import { pingOpenSearch, countOpenSearchDocs } from "./opensearch";
import {
checkAthenaResultLocation,
checkWriteAccess,
type WriteAccessResult,
} from "./writeAccess";
import { checkAthenaResultLocation } from "./athenaResultLocation";

/**
* Result of a single check within a destination test (e.g. "Glue table exists",
Expand All @@ -37,12 +33,6 @@ export interface DestinationTestReport {
ok: boolean;
summary: string;
checks: DestinationCheck[];
/**
* Whether queries could change data at this destination. Present only when
* every check passed, because the write check needs a reachable destination.
* Both results are warnings, not failed checks, so they do not change `ok`.
*/
writeAccess?: WriteAccessResult;
}

/** Awaits a connectivity probe and turns success/throw into a DestinationCheck. */
Expand Down Expand Up @@ -99,15 +89,6 @@ export interface DestinationTestOptions {
export async function testDestination(
cfg: TelemetryConfig,
options: DestinationTestOptions = {},
): Promise<DestinationTestReport> {
const result = await runChecks(cfg, options);
if (!result.ok) return result;
return { ...result, writeAccess: await checkWriteAccess(cfg) };
}

async function runChecks(
cfg: TelemetryConfig,
options: DestinationTestOptions,
): Promise<DestinationTestReport> {
const credentials = resolveCredentials(cfg.awsProfile);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export * from "./chartSpec";
export * from "./configCore";
export * from "./copilotBridge";
export * from "./destinationTest";
export * from "./writeAccess";
export * from "./athenaResultLocation";
export * from "./dynamodb";
export * from "./explorerSession";
export * from "./hostCapabilities";
Expand Down
Loading
Loading