Skip to content

Commit 0d1fbc4

Browse files
authored
Cloudwatch: Add support for adding account id to sql query (#90880)
1 parent 94dd410 commit 0d1fbc4

File tree

3 files changed

+78
-35
lines changed

3 files changed

+78
-35
lines changed

public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLBuilderEditor.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const SQLBuilderEditor = ({ query, datasource, onChange }: React.PropsWit
2626
const onQueryChange = useCallback(
2727
(query: CloudWatchMetricsQuery) => {
2828
const sqlGenerator = new SQLGenerator();
29-
const sqlString = sqlGenerator.expressionToSqlQuery(query.sql ?? {});
29+
const sqlString = sqlGenerator.expressionToSqlQuery(query.sql ?? {}, query.accountId);
3030
const fullQuery = {
3131
...query,
3232
sqlExpression: sqlString,
@@ -40,7 +40,7 @@ export const SQLBuilderEditor = ({ query, datasource, onChange }: React.PropsWit
4040
const [sqlPreview, setSQLPreview] = useState<string | undefined>();
4141
useEffect(() => {
4242
const sqlGenerator = new SQLGenerator();
43-
const sqlString = sqlGenerator.expressionToSqlQuery(query.sql ?? {});
43+
const sqlString = sqlGenerator.expressionToSqlQuery(query.sql ?? {}, query.accountId);
4444
if (sqlPreview !== sqlString) {
4545
setSQLPreview(sqlString);
4646
}

public/app/plugins/datasource/cloudwatch/language/cloudwatch-sql/SQLGenerator.test.ts

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,30 @@ describe('SQLGenerator', () => {
110110
});
111111
});
112112

113-
function assertQueryEndsWith(rest: Partial<SQLExpression>, expectedFilter: string) {
114-
expect(new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery, ...rest })).toEqual(
115-
`SELECT SUM(CPUUtilization) FROM SCHEMA("AWS/EC2") ${expectedFilter}`
116-
);
113+
function assertQueryEndsWith(args: { sql: Partial<SQLExpression>; accountId?: string }, expectedFilter: string) {
114+
expect(
115+
new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery, ...args.sql }, args.accountId)
116+
).toEqual(`SELECT SUM(CPUUtilization) FROM SCHEMA("AWS/EC2") ${expectedFilter}`);
117117
}
118+
describe('accountId', () => {
119+
it('should add where clause if account ID is defined', () => {
120+
expect(new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery }, '12345')).toEqual(
121+
'SELECT SUM(CPUUtilization) FROM SCHEMA("AWS/EC2") WHERE AWS.AccountId = \'12345\''
122+
);
123+
});
124+
125+
it('should not add where clause if account ID is not defined', () => {
126+
expect(new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery })).toEqual(
127+
'SELECT SUM(CPUUtilization) FROM SCHEMA("AWS/EC2")'
128+
);
129+
});
118130

131+
it('should not add where clause if account ID is all', () => {
132+
expect(new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery })).toEqual(
133+
'SELECT SUM(CPUUtilization) FROM SCHEMA("AWS/EC2")'
134+
);
135+
});
136+
});
119137
describe('filter', () => {
120138
it('should not add WHERE clause in case its empty', () => {
121139
expect(new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery })).not.toContain('WHERE');
@@ -126,6 +144,21 @@ describe('SQLGenerator', () => {
126144
expect(new SQLGenerator(mockTemplateSrv).expressionToSqlQuery({ ...baseQuery, where })).not.toContain('WHERE');
127145
});
128146

147+
it('should add where clauses with AND if accountID is defined', () => {
148+
const where = createArray([createOperator('Instance-Id', '=', 'I-123')]);
149+
assertQueryEndsWith(
150+
{ sql: { where }, accountId: '12345' },
151+
`WHERE AWS.AccountId = '12345' AND "Instance-Id" = 'I-123'`
152+
);
153+
});
154+
it('should add where clauses with WHERE if accountID is not defined', () => {
155+
const where = createArray([createOperator('Instance-Id', '=', 'I-123')]);
156+
assertQueryEndsWith({ sql: { where } }, `WHERE "Instance-Id" = 'I-123'`);
157+
});
158+
it('should add where clauses with WHERE if accountID is all', () => {
159+
const where = createArray([createOperator('Instance-Id', '=', 'I-123')]);
160+
assertQueryEndsWith({ sql: { where }, accountId: 'all' }, `WHERE "Instance-Id" = 'I-123'`);
161+
});
129162
// TODO: We should handle this scenario
130163
it.skip('should not add WHERE clause when the operator is incomplete', () => {
131164
const where = createArray([createOperator('Instance-Id', '=')]);
@@ -134,12 +167,12 @@ describe('SQLGenerator', () => {
134167

135168
it('should handle one top level filter with AND', () => {
136169
const where = createArray([createOperator('Instance-Id', '=', 'I-123')]);
137-
assertQueryEndsWith({ where }, `WHERE "Instance-Id" = 'I-123'`);
170+
assertQueryEndsWith({ sql: { where } }, `WHERE "Instance-Id" = 'I-123'`);
138171
});
139172

140173
it('should handle one top level filter with OR', () => {
141174
assertQueryEndsWith(
142-
{ where: createArray([createOperator('InstanceId', '=', 'I-123')]) },
175+
{ sql: { where: createArray([createOperator('InstanceId', '=', 'I-123')]) } },
143176
`WHERE InstanceId = 'I-123'`
144177
);
145178
});
@@ -149,15 +182,15 @@ describe('SQLGenerator', () => {
149182
[createOperator('InstanceId', '=', 'I-123'), createOperator('Instance-Id', '!=', 'I-456')],
150183
QueryEditorExpressionType.And
151184
);
152-
assertQueryEndsWith({ where: filter }, `WHERE InstanceId = 'I-123' AND "Instance-Id" != 'I-456'`);
185+
assertQueryEndsWith({ sql: { where: filter } }, `WHERE InstanceId = 'I-123' AND "Instance-Id" != 'I-456'`);
153186
});
154187

155188
it('should handle multiple top level filters combined with OR', () => {
156189
const filter = createArray(
157190
[createOperator('InstanceId', '=', 'I-123'), createOperator('InstanceId', '!=', 'I-456')],
158191
QueryEditorExpressionType.Or
159192
);
160-
assertQueryEndsWith({ where: filter }, `WHERE InstanceId = 'I-123' OR InstanceId != 'I-456'`);
193+
assertQueryEndsWith({ sql: { where: filter } }, `WHERE InstanceId = 'I-123' OR InstanceId != 'I-456'`);
161194
});
162195

163196
it('should handle one top level filters with one nested filter', () => {
@@ -168,7 +201,7 @@ describe('SQLGenerator', () => {
168201
],
169202
QueryEditorExpressionType.And
170203
);
171-
assertQueryEndsWith({ where: filter }, `WHERE InstanceId = 'I-123' AND InstanceId != 'I-456'`);
204+
assertQueryEndsWith({ sql: { where: filter } }, `WHERE InstanceId = 'I-123' AND InstanceId != 'I-456'`);
172205
});
173206

174207
it('should handle one top level filter with two nested filters combined with AND', () => {
@@ -184,7 +217,7 @@ describe('SQLGenerator', () => {
184217
);
185218
// In this scenario, the parenthesis are redundant. However, they're not doing any harm and it would be really complicated to remove them
186219
assertQueryEndsWith(
187-
{ where: filter },
220+
{ sql: { where: filter } },
188221
`WHERE "Instance.Type" = 'I-123' AND (InstanceId != 'I-456' AND Type != 'some-type')`
189222
);
190223
});
@@ -201,7 +234,7 @@ describe('SQLGenerator', () => {
201234
QueryEditorExpressionType.And
202235
);
203236
assertQueryEndsWith(
204-
{ where: filter },
237+
{ sql: { where: filter } },
205238
`WHERE InstanceId = 'I-123' AND (InstanceId != 'I-456' OR Type != 'some-type')`
206239
);
207240
});
@@ -222,7 +255,7 @@ describe('SQLGenerator', () => {
222255
);
223256

224257
assertQueryEndsWith(
225-
{ where: filter },
258+
{ sql: { where: filter } },
226259
`WHERE (InstanceId = 'I-123' AND Type != 'some-type') AND (InstanceId != 'I-456' OR Type != 'some-type')`
227260
);
228261
});
@@ -242,7 +275,7 @@ describe('SQLGenerator', () => {
242275
QueryEditorExpressionType.Or
243276
);
244277
assertQueryEndsWith(
245-
{ where: filter },
278+
{ sql: { where: filter } },
246279
`WHERE (InstanceId = 'I-123' OR Type != 'some-type') OR (InstanceId != 'I-456' OR Type != 'some-type')`
247280
);
248281
});
@@ -257,7 +290,7 @@ describe('SQLGenerator', () => {
257290
QueryEditorExpressionType.Or
258291
);
259292
assertQueryEndsWith(
260-
{ where: filter },
293+
{ sql: { where: filter } },
261294
`WHERE InstanceId = 'I-123' OR Type != 'some-type' OR InstanceId != 'I-456'`
262295
);
263296
});
@@ -272,7 +305,7 @@ describe('SQLGenerator', () => {
272305
QueryEditorExpressionType.And
273306
);
274307
assertQueryEndsWith(
275-
{ where: filter },
308+
{ sql: { where: filter } },
276309
`WHERE InstanceId = 'I-123' AND Type != 'some-type' AND InstanceId != 'I-456'`
277310
);
278311
});
@@ -284,14 +317,14 @@ describe('SQLGenerator', () => {
284317
});
285318
it('should handle single label', () => {
286319
const groupBy = createArray([createGroupBy('InstanceId')], QueryEditorExpressionType.And);
287-
assertQueryEndsWith({ groupBy }, `GROUP BY InstanceId`);
320+
assertQueryEndsWith({ sql: { groupBy } }, `GROUP BY InstanceId`);
288321
});
289322
it('should handle multiple label', () => {
290323
const groupBy = createArray(
291324
[createGroupBy('InstanceId'), createGroupBy('Type'), createGroupBy('Group')],
292325
QueryEditorExpressionType.And
293326
);
294-
assertQueryEndsWith({ groupBy }, `GROUP BY InstanceId, Type, Group`);
327+
assertQueryEndsWith({ sql: { groupBy } }, `GROUP BY InstanceId, Type, Group`);
295328
});
296329
});
297330

@@ -301,16 +334,16 @@ describe('SQLGenerator', () => {
301334
});
302335
it('should handle SUM ASC', () => {
303336
const orderBy = createFunction('SUM');
304-
assertQueryEndsWith({ orderBy, orderByDirection: 'ASC' }, `ORDER BY SUM() ASC`);
337+
assertQueryEndsWith({ sql: { orderBy, orderByDirection: 'ASC' } }, `ORDER BY SUM() ASC`);
305338
});
306339

307340
it('should handle SUM ASC', () => {
308341
const orderBy = createFunction('SUM');
309-
assertQueryEndsWith({ orderBy, orderByDirection: 'ASC' }, `ORDER BY SUM() ASC`);
342+
assertQueryEndsWith({ sql: { orderBy, orderByDirection: 'ASC' } }, `ORDER BY SUM() ASC`);
310343
});
311344
it('should handle COUNT DESC', () => {
312345
const orderBy = createFunction('COUNT');
313-
assertQueryEndsWith({ orderBy, orderByDirection: 'DESC' }, `ORDER BY COUNT() DESC`);
346+
assertQueryEndsWith({ sql: { orderBy, orderByDirection: 'DESC' } }, `ORDER BY COUNT() DESC`);
314347
});
315348
});
316349
describe('limit', () => {
@@ -319,7 +352,7 @@ describe('SQLGenerator', () => {
319352
});
320353

321354
it('should be added in case its specified', () => {
322-
assertQueryEndsWith({ limit: 10 }, `LIMIT 10`);
355+
assertQueryEndsWith({ sql: { limit: 10 } }, `LIMIT 10`);
323356
});
324357
});
325358

public/app/plugins/datasource/cloudwatch/language/cloudwatch-sql/SQLGenerator.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,24 @@ import {
1010
} from '../../expressions';
1111
import { SQLExpression } from '../../types';
1212

13+
const isAccountIdDefined = (accountId: string | undefined): boolean => !!(accountId && accountId !== 'all');
14+
1315
export default class SQLGenerator {
1416
constructor(private templateSrv: TemplateSrv = getTemplateSrv()) {}
1517

16-
expressionToSqlQuery({
17-
select,
18-
from,
19-
where,
20-
groupBy,
21-
orderBy,
22-
orderByDirection,
23-
limit,
24-
}: SQLExpression): string | undefined {
18+
expressionToSqlQuery(
19+
{ select, from, where, groupBy, orderBy, orderByDirection, limit }: SQLExpression,
20+
accountId?: string
21+
): string | undefined {
2522
if (!from || !select?.name || !select?.parameters?.length) {
2623
return undefined;
2724
}
2825

2926
let parts: string[] = [];
3027
this.appendSelect(select, parts);
3128
this.appendFrom(from, parts);
32-
this.appendWhere(where, parts, true, where?.expressions?.length ?? 0);
29+
this.appendAccountId(parts, accountId);
30+
this.appendWhere(where, parts, true, where?.expressions?.length ?? 0, accountId);
3331
this.appendGroupBy(groupBy, parts);
3432
this.appendOrderBy(orderBy, orderByDirection, parts);
3533
this.appendLimit(limit, parts);
@@ -49,19 +47,31 @@ export default class SQLGenerator {
4947
: parts.push(this.formatValue(from?.property?.name ?? ''));
5048
}
5149

50+
private appendAccountId(parts: string[], accountId?: string) {
51+
if (!isAccountIdDefined(accountId)) {
52+
return;
53+
}
54+
parts.push(`WHERE AWS.AccountId = '${accountId}'`);
55+
}
56+
5257
private appendWhere(
5358
filter: QueryEditorExpression | undefined,
5459
parts: string[],
5560
isTopLevelExpression: boolean,
56-
topLevelExpressionsCount: number
61+
topLevelExpressionsCount: number,
62+
accountId?: string
5763
) {
5864
if (!filter) {
5965
return;
6066
}
6167

6268
const hasChildExpressions = 'expressions' in filter && filter.expressions.length > 0;
6369
if (isTopLevelExpression && hasChildExpressions) {
64-
parts.push('WHERE');
70+
if (isAccountIdDefined(accountId)) {
71+
parts.push('AND');
72+
} else {
73+
parts.push('WHERE');
74+
}
6575
}
6676

6777
if (filter.type === QueryEditorExpressionType.And) {

0 commit comments

Comments
 (0)