Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 6 additions & 6 deletions packages/aws-cdk/lib/cli/telemetry/endpoint-sink.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { IncomingMessage } from 'http';
import type { Agent } from 'https';
import { request } from 'https';
import type { UrlWithStringQuery } from 'url';
import { parse, type UrlWithStringQuery } from 'url';
import { ToolkitError } from '@aws-cdk/toolkit-lib';
import { IoHelper } from '../../api-private';
import type { IIoHost } from '../io-host';
Expand All @@ -17,7 +17,7 @@ export interface EndpointTelemetrySinkProps {
/**
* The external endpoint to hit
*/
readonly endpoint: UrlWithStringQuery;
readonly endpoint: string;

/**
* Where messages are going to be sent
Expand All @@ -44,7 +44,7 @@ export class EndpointTelemetrySink implements ITelemetrySink {
private agent?: Agent;

public constructor(props: EndpointTelemetrySinkProps) {
this.endpoint = props.endpoint;
this.endpoint = parse(props.endpoint);
this.ioHelper = IoHelper.fromActionAwareIoHost(props.ioHost);
this.agent = props.agent;

Expand All @@ -70,7 +70,7 @@ export class EndpointTelemetrySink implements ITelemetrySink {
return;
}

const res = await this.https(this.endpoint, this.events);
const res = await this.https(this.endpoint, { events: this.events });

// Clear the events array after successful output
if (res) {
Expand All @@ -87,7 +87,7 @@ export class EndpointTelemetrySink implements ITelemetrySink {
*/
private async https(
url: UrlWithStringQuery,
body: TelemetrySchema[],
body: { events: TelemetrySchema[] },
): Promise<boolean> {
try {
const res = await doRequest(url, body, this.agent);
Expand All @@ -112,7 +112,7 @@ export class EndpointTelemetrySink implements ITelemetrySink {
*/
function doRequest(
url: UrlWithStringQuery,
data: TelemetrySchema[],
data: { events: TelemetrySchema[] },
agent?: Agent,
) {
return new Promise<IncomingMessage>((ok, ko) => {
Expand Down
36 changes: 14 additions & 22 deletions packages/aws-cdk/test/cli/telemetry/endpoint-sink.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as https from 'https';
import { parse } from 'url';
import { IoHelper } from '../../../lib/api-private';
import { CliIoHost } from '../../../lib/cli/io-host';
import { EndpointTelemetrySink } from '../../../lib/cli/telemetry/endpoint-sink';
Expand Down Expand Up @@ -88,16 +87,15 @@ describe('EndpointTelemetrySink', () => {
test('makes a POST request to the specified endpoint', async () => {
// GIVEN
const mockRequest = setupMockRequest();
const endpoint = parse('https://example.com/telemetry');
const testEvent = createTestEvent('test', { foo: 'bar' });
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

// WHEN
await client.emit(testEvent);
await client.flush();

// THEN
const expectedPayload = JSON.stringify([testEvent]);
const expectedPayload = JSON.stringify({ events: [testEvent] });
expect(https.request).toHaveBeenCalledWith({
hostname: 'example.com',
port: null,
Expand All @@ -117,9 +115,8 @@ describe('EndpointTelemetrySink', () => {
test('silently catches request errors', async () => {
// GIVEN
const mockRequest = setupMockRequest();
const endpoint = parse('https://example.com/telemetry');
const testEvent = createTestEvent('test');
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

mockRequest.on.mockImplementation((event, callback) => {
if (event === 'error') {
Expand All @@ -137,18 +134,17 @@ describe('EndpointTelemetrySink', () => {
test('multiple events sent as one', async () => {
// GIVEN
const mockRequest = setupMockRequest();
const endpoint = parse('https://example.com/telemetry');
const testEvent1 = createTestEvent('test1', { foo: 'bar' });
const testEvent2 = createTestEvent('test2', { foo: 'bazoo' });
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

// WHEN
await client.emit(testEvent1);
await client.emit(testEvent2);
await client.flush();

// THEN
const expectedPayload = JSON.stringify([testEvent1, testEvent2]);
const expectedPayload = JSON.stringify({ events: [testEvent1, testEvent2] });
expect(https.request).toHaveBeenCalledTimes(1);
expect(https.request).toHaveBeenCalledWith({
hostname: 'example.com',
Expand All @@ -169,10 +165,9 @@ describe('EndpointTelemetrySink', () => {
test('successful flush clears events cache', async () => {
// GIVEN
setupMockRequest();
const endpoint = parse('https://example.com/telemetry');
const testEvent1 = createTestEvent('test1', { foo: 'bar' });
const testEvent2 = createTestEvent('test2', { foo: 'bazoo' });
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

// WHEN
await client.emit(testEvent1);
Expand All @@ -181,7 +176,7 @@ describe('EndpointTelemetrySink', () => {
await client.flush();

// THEN
const expectedPayload1 = JSON.stringify([testEvent1]);
const expectedPayload1 = JSON.stringify({ events: [testEvent1] });
expect(https.request).toHaveBeenCalledTimes(2);
expect(https.request).toHaveBeenCalledWith({
hostname: 'example.com',
Expand All @@ -196,7 +191,7 @@ describe('EndpointTelemetrySink', () => {
timeout: 500,
}, expect.anything());

const expectedPayload2 = JSON.stringify([testEvent2]);
const expectedPayload2 = JSON.stringify({ events: [testEvent2] });
expect(https.request).toHaveBeenCalledWith({
hostname: 'example.com',
port: null,
Expand Down Expand Up @@ -238,10 +233,9 @@ describe('EndpointTelemetrySink', () => {
return mockRequest;
});

const endpoint = parse('https://example.com/telemetry');
const testEvent1 = createTestEvent('test1', { foo: 'bar' });
const testEvent2 = createTestEvent('test2', { foo: 'bazoo' });
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

// WHEN
await client.emit(testEvent1);
Expand All @@ -255,7 +249,7 @@ describe('EndpointTelemetrySink', () => {
await client.flush();

// THEN
const expectedPayload1 = JSON.stringify([testEvent1]);
const expectedPayload1 = JSON.stringify({ events: [testEvent1] });
expect(https.request).toHaveBeenCalledTimes(2);
expect(https.request).toHaveBeenCalledWith({
hostname: 'example.com',
Expand All @@ -270,15 +264,15 @@ describe('EndpointTelemetrySink', () => {
timeout: 500,
}, expect.anything());

const expectedPayload2 = JSON.stringify([testEvent2]);
const expectedPayload2 = JSON.stringify({ events: [testEvent1, testEvent2] });
expect(https.request).toHaveBeenCalledWith({
hostname: 'example.com',
port: null,
path: '/telemetry',
method: 'POST',
headers: {
'content-type': 'application/json',
'content-length': expectedPayload1.length + expectedPayload2.length - 1,
'content-length': expectedPayload2.length,
},
agent: undefined,
timeout: 500,
Expand All @@ -289,13 +283,12 @@ describe('EndpointTelemetrySink', () => {
// GIVEN
jest.useFakeTimers();
setupMockRequest(); // Setup the mock request but we don't need the return value
const endpoint = parse('https://example.com/telemetry');

// Create a spy on setInterval
const setIntervalSpy = jest.spyOn(global, 'setInterval');

// Create the client
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

// Create a spy on the flush method
const flushSpy = jest.spyOn(client, 'flush');
Expand Down Expand Up @@ -337,8 +330,7 @@ describe('EndpointTelemetrySink', () => {
// Mock IoHelper.fromActionAwareIoHost to return our mock
jest.spyOn(IoHelper, 'fromActionAwareIoHost').mockReturnValue(mockIoHelper as any);

const endpoint = parse('https://example.com/telemetry');
const client = new EndpointTelemetrySink({ endpoint, ioHost });
const client = new EndpointTelemetrySink({ endpoint: 'https://example.com/telemetry', ioHost });

// Mock https.request to throw an error
(https.request as jest.Mock).mockImplementation(() => {
Expand Down
8 changes: 8 additions & 0 deletions packages/aws-cdk/test/cli/telemetry/file-sink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ describe('FileTelemetrySink', () => {
expect(fileContent).toBe(expectedSingleEvent + expectedSingleEvent);
});

test('constructor throws if file already exists', async () => {
// GIVEN
fs.writeFileSync(logFilePath, 'exists');

// WHEN & THEN
expect(() => new FileTelemetrySink({ logFilePath, ioHost })).toThrow(/Telemetry file already exists/);
});

test('handles errors gracefully and logs to trace without throwing', async () => {
// GIVEN
const testEvent: TelemetrySchema = {
Expand Down
Loading