Merge pull request #15891 from afscrome/improvemsgraphperf

Improve MS Graph Performance
This commit is contained in:
Fredrik Adelöw
2023-02-03 13:24:11 +01:00
committed by GitHub
7 changed files with 246 additions and 192 deletions
+8
View File
@@ -0,0 +1,8 @@
---
'@backstage/plugin-catalog-backend-module-msgraph': minor
---
Improve performance when loading users via group membership.
Users data is now loaded from a paged query, rather than having to make an extra call per user to load each user's profiles.
Note, there are still additional per user calls made to load user avatars
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend-module-msgraph': patch
---
Increased default page size to 999 (from 100) to reduce the number of calls made to the Microsoft Graph API.
@@ -70,7 +70,11 @@ export const MICROSOFT_GRAPH_USER_ID_ANNOTATION = 'graph.microsoft.com/user-id';
export class MicrosoftGraphClient {
constructor(baseUrl: string, tokenCredential: TokenCredential);
static create(config: MicrosoftGraphProviderConfig): MicrosoftGraphClient;
getGroupMembers(groupId: string): AsyncIterable<GroupMember>;
getGroupMembers(
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
): AsyncIterable<GroupMember>;
// (undocumented)
getGroupPhoto(groupId: string, sizeId?: string): Promise<string | undefined>;
getGroupPhotoWithSizeLimit(
@@ -81,6 +85,11 @@ export class MicrosoftGraphClient {
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
): AsyncIterable<MicrosoftGraph.Group>;
getGroupUserMembers(
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
): AsyncIterable<MicrosoftGraph.User>;
getOrganization(tenantId: string): Promise<MicrosoftGraph.Organization>;
// (undocumented)
getUserPhoto(userId: string, sizeId?: string): Promise<string | undefined>;
@@ -88,10 +97,6 @@ export class MicrosoftGraphClient {
userId: string,
maxSize: number,
): Promise<string | undefined>;
getUserProfile(
userId: string,
query?: ODataQuery,
): Promise<MicrosoftGraph.User>;
getUsers(
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
@@ -231,6 +236,7 @@ export type ODataQuery = {
expand?: string;
select?: string[];
count?: boolean;
top?: number;
};
// @public
@@ -70,7 +70,7 @@ describe('MicrosoftGraphClient', () => {
expect(await response.json()).toEqual({ value: 'example' });
});
it('should perform api request with filter, select and expand', async () => {
it('should perform api request with filter, select, expand and top', async () => {
worker.use(
rest.get('https://example.com/users', (req, res, ctx) =>
res(ctx.status(200), ctx.json({ queryString: req.url.search })),
@@ -81,12 +81,13 @@ describe('MicrosoftGraphClient', () => {
filter: 'test eq true',
expand: 'children',
select: ['id', 'children'],
top: 471,
});
expect(response.status).toBe(200);
expect(await response.json()).toEqual({
queryString:
'?$filter=test%20eq%20true&$select=id,children&$expand=children',
'?$filter=test%20eq%20true&$select=id,children&$expand=children&$top=471',
});
});
@@ -134,33 +135,6 @@ describe('MicrosoftGraphClient', () => {
expect(values).toEqual(['first', 'second']);
});
it('should load user profile', async () => {
worker.use(
rest.get('https://example.com/users/user-id', (_, res, ctx) =>
res(
ctx.status(200),
ctx.json({
surname: 'Example',
}),
),
),
);
const userProfile = await client.getUserProfile('user-id');
expect(userProfile).toEqual({ surname: 'Example' });
});
it('should throw exception if load user profile fails', async () => {
worker.use(
rest.get('https://example.com/users/user-id', (_, res, ctx) =>
res(ctx.status(404)),
),
);
await expect(() => client.getUserProfile('user-id')).rejects.toThrow();
});
it('should load user profile photo with max size of 120', async () => {
worker.use(
rest.get('https://example.com/users/user-id/photos', (_, res, ctx) =>
@@ -329,6 +303,27 @@ describe('MicrosoftGraphClient', () => {
]);
});
it('should load user group members', async () => {
worker.use(
rest.get(
'https://example.com/groups/group-id/members/microsoft.graph.user',
(_, res, ctx) =>
res(
ctx.status(200),
ctx.json({
value: [{ id: '12345' }, { id: '67890' }],
}),
),
),
);
const values = await collectAsyncIterable(
client.getGroupUserMembers('group-id'),
);
expect(values).toEqual([{ id: '12345' }, { id: '67890' }]);
});
it('should load organization', async () => {
worker.use(
rest.get('https://example.com/organization/tentant-id', (_, res, ctx) =>
@@ -52,6 +52,10 @@ export type ODataQuery = {
* Retrieves the total count of matching resources.
*/
count?: boolean;
/**
* Maximum number of records to receive in one batch.
*/
top?: number;
};
/**
@@ -188,6 +192,7 @@ export class MicrosoftGraphClient {
$select: query?.select?.join(','),
$expand: query?.expand,
$count: query?.count,
$top: query?.top,
},
{
addQueryPrefix: true,
@@ -229,28 +234,6 @@ export class MicrosoftGraphClient {
});
}
/**
* Get {@link https://docs.microsoft.com/en-us/graph/api/resources/user | User}
* from Graph API
*
* @public
* @param userId - The unique identifier for the `User` resource
* @param query - OData Query {@link ODataQuery}
*
*/
async getUserProfile(
userId: string,
query?: ODataQuery,
): Promise<MicrosoftGraph.User> {
const response = await this.requestApi(`users/${userId}`, query);
if (response.status !== 200) {
await this.handleError('user profile', response);
}
return await response.json();
}
/**
* Get {@link https://docs.microsoft.com/en-us/graph/api/resources/profilephoto | profilePhoto}
* of `User` from Graph API with size limit
@@ -343,8 +326,37 @@ export class MicrosoftGraphClient {
* @param groupId - The unique identifier for the `Group` resource
*
*/
async *getGroupMembers(groupId: string): AsyncIterable<GroupMember> {
yield* this.requestCollection<GroupMember>(`groups/${groupId}/members`);
async *getGroupMembers(
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
): AsyncIterable<GroupMember> {
yield* this.requestCollection<GroupMember>(
`groups/${groupId}/members`,
query,
queryMode,
);
}
/**
* Get a collection of
* {@link https://docs.microsoft.com/en-us/graph/api/resources/user | User}
* belonging to a `Group` from Graph API and return as `AsyncIterable`
* @public
* @param groupId - The unique identifier for the `Group` resource
* @param query - OData Query {@link ODataQuery}
* @param queryMode - Mode to use while querying. Some features are only available at "advanced".
*/
async *getGroupUserMembers(
groupId: string,
query?: ODataQuery,
queryMode?: 'basic' | 'advanced',
): AsyncIterable<MicrosoftGraph.User> {
yield* this.requestCollection<MicrosoftGraph.User>(
`groups/${groupId}/members/microsoft.graph.user/`,
query,
queryMode,
);
}
/**
@@ -61,9 +61,9 @@ function group(data: Partial<GroupEntity>): GroupEntity {
describe('read microsoft graph', () => {
const client: jest.Mocked<MicrosoftGraphClient> = {
getUsers: jest.fn(),
getUserProfile: jest.fn(),
getGroups: jest.fn(),
getGroupMembers: jest.fn(),
getGroupUserMembers: jest.fn(),
getUserPhotoWithSizeLimit: jest.fn(),
getGroupPhotoWithSizeLimit: jest.fn(),
getOrganization: jest.fn(),
@@ -78,13 +78,6 @@ describe('read microsoft graph', () => {
mail: 'user.name@example.com',
};
}
async function getExampleUserProfile() {
return {
id: 'userid',
displayName: 'User Name',
mail: 'user.name@example.com',
};
}
async function* getExampleGroups() {
yield {
id: 'groupid',
@@ -140,6 +133,7 @@ describe('read microsoft graph', () => {
expect(client.getUsers).toHaveBeenCalledWith(
{
filter: 'accountEnabled eq true',
top: 999,
},
undefined,
);
@@ -186,6 +180,7 @@ describe('read microsoft graph', () => {
expect(client.getUsers).toHaveBeenCalledWith(
{
filter: 'accountEnabled eq true',
top: 999,
},
'advanced',
);
@@ -228,6 +223,7 @@ describe('read microsoft graph', () => {
{
expand: 'manager',
filter: 'accountEnabled eq true',
top: 999,
},
undefined,
);
@@ -242,9 +238,8 @@ describe('read microsoft graph', () => {
describe('readMicrosoftGraphUsersInGroups', () => {
it('should read users from Groups', async () => {
client.getGroups.mockImplementation(getExampleGroups);
client.getGroupMembers.mockImplementation(getExampleGroupMembers);
client.getGroupUserMembers.mockImplementation(getExampleUsers);
client.getUserProfile.mockResolvedValue(getExampleUserProfile());
client.getUserPhotoWithSizeLimit.mockResolvedValue(
'data:image/jpeg;base64,...',
);
@@ -278,16 +273,19 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq true',
select: ['id', 'displayName'],
top: 999,
},
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getUserProfile).toHaveBeenCalledTimes(1);
expect(client.getUserProfile).toHaveBeenCalledWith('userid', {
expand: undefined,
});
expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupUserMembers).toHaveBeenCalledWith(
'groupid',
{ top: 999 },
undefined,
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
'userid',
@@ -297,13 +295,8 @@ describe('read microsoft graph', () => {
it('should read users from Groups with advanced query mode', async () => {
client.getGroups.mockImplementation(getExampleGroups);
client.getGroupMembers.mockImplementation(getExampleGroupMembers);
client.getGroupUserMembers.mockImplementation(getExampleUsers);
client.getUserProfile.mockResolvedValue({
id: 'userid',
displayName: 'User Name',
mail: 'user.name@example.com',
});
client.getUserPhotoWithSizeLimit.mockResolvedValue(
'data:image/jpeg;base64,...',
);
@@ -338,16 +331,19 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq true',
select: ['id', 'displayName'],
top: 999,
},
'advanced',
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getUserProfile).toHaveBeenCalledTimes(1);
expect(client.getUserProfile).toHaveBeenCalledWith('userid', {
expand: undefined,
});
expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupUserMembers).toHaveBeenCalledWith(
'groupid',
{ top: 999 },
'advanced',
);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
'userid',
@@ -357,8 +353,8 @@ describe('read microsoft graph', () => {
it('should read users with userExpand, groupExpand and custom transformer', async () => {
client.getGroups.mockImplementation(getExampleGroups);
client.getGroupMembers.mockImplementation(getExampleGroupMembers);
client.getUserProfile.mockResolvedValue(getExampleUserProfile());
client.getGroupUserMembers.mockImplementation(getExampleUsers);
client.getUserPhotoWithSizeLimit.mockResolvedValue(
'data:image/jpeg;base64,...',
);
@@ -390,16 +386,22 @@ describe('read microsoft graph', () => {
{
expand: 'member',
filter: 'securityEnabled eq true',
select: ['id', 'displayName'],
top: 999,
},
undefined,
);
expect(client.getGroupUserMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupUserMembers).toHaveBeenCalledWith(
'groupid',
{
expand: 'manager',
top: 999,
},
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getUserProfile).toHaveBeenCalledTimes(1);
expect(client.getUserProfile).toHaveBeenCalledWith('userid', {
expand: 'manager',
});
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledWith(
'userid',
@@ -527,11 +529,14 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq false',
top: 999,
},
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -603,11 +608,14 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq false',
top: 999,
},
'advanced',
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -680,11 +688,14 @@ describe('read microsoft graph', () => {
{
expand: 'member',
filter: 'securityEnabled eq false',
top: 999,
},
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
// TODO: Loading groups photos doesn't work right now as Microsoft Graph
// doesn't allows this yet
// expect(client.getGroupPhotoWithSizeLimit).toBeCalledTimes(1);
@@ -750,11 +761,14 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq true',
top: 999,
},
undefined,
);
expect(client.getGroupMembers).toHaveBeenCalledTimes(1);
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid');
expect(client.getGroupMembers).toHaveBeenCalledWith('groupid', {
top: 999,
});
});
});
@@ -887,6 +901,7 @@ describe('read microsoft graph', () => {
expect(client.getUsers).toHaveBeenCalledWith(
{
filter: undefined,
top: 999,
},
undefined,
);
@@ -894,6 +909,7 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq false',
top: 999,
},
undefined,
);
@@ -925,6 +941,7 @@ describe('read microsoft graph', () => {
{
expand: 'manager',
filter: 'accountEnabled eq true',
top: 999,
},
undefined,
);
@@ -932,6 +949,7 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq false',
top: 999,
},
undefined,
);
@@ -960,6 +978,7 @@ describe('read microsoft graph', () => {
expect(client.getUsers).toHaveBeenCalledWith(
{
select: ['mail'],
top: 999,
},
undefined,
);
@@ -972,13 +991,13 @@ describe('read microsoft graph', () => {
});
client.getUsers.mockImplementation(getExampleUsers);
client.getUserProfile.mockImplementation(getExampleUserProfile);
client.getUserPhotoWithSizeLimit.mockResolvedValue(
'data:image/jpeg;base64,...',
);
client.getGroups.mockImplementation(getExampleGroups);
client.getGroupMembers.mockImplementation(getExampleGroupMembers);
client.getGroupUserMembers.mockImplementation(getExampleUsers);
client.getGroupPhotoWithSizeLimit.mockResolvedValue(
'data:image/jpeg;base64,...',
);
@@ -994,16 +1013,18 @@ describe('read microsoft graph', () => {
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'name eq backstage-group',
select: ['id', 'displayName'],
top: 999,
},
undefined,
);
expect(client.getGroups).toHaveBeenCalledWith(
{
filter: 'securityEnabled eq false',
top: 999,
},
undefined,
);
expect(client.getUserProfile).toHaveBeenCalledTimes(1);
expect(client.getUserPhotoWithSizeLimit).toHaveBeenCalledTimes(1);
});
});
@@ -38,6 +38,9 @@ import {
defaultOrganizationTransformer,
defaultUserTransformer,
} from './defaultTransformers';
import * as MicrosoftGraph from '@microsoft/microsoft-graph-types';
const PAGE_SIZE = 999;
export async function readMicrosoftGraphUsers(
client: MicrosoftGraphClient,
@@ -52,50 +55,24 @@ export async function readMicrosoftGraphUsers(
): Promise<{
users: UserEntity[]; // With all relations empty
}> {
const users: UserEntity[] = [];
const limiter = limiterFactory(10);
const transformer = options.transformer ?? defaultUserTransformer;
const promises: Promise<void>[] = [];
for await (const user of client.getUsers(
const users = client.getUsers(
{
filter: options.userFilter,
expand: options.userExpand,
select: options.userSelect,
top: PAGE_SIZE,
},
options.queryMode,
)) {
// Process all users in parallel, otherwise it can take quite some time
promises.push(
limiter(async () => {
let userPhoto;
try {
userPhoto = await client.getUserPhotoWithSizeLimit(
user.id!,
// We are limiting the photo size, as users with full resolution photos
// can make the Backstage API slow
120,
);
} catch (e) {
options.logger.warn(`Unable to load photo for ${user.id}, ${e}`);
}
);
const entity = await transformer(user, userPhoto);
if (!entity) {
return;
}
users.push(entity);
}),
);
}
// Wait for all users and photos to be downloaded
await Promise.all(promises);
return { users };
return {
users: await transformUsers(
client,
users,
options.logger,
options.transformer,
),
};
}
export async function readMicrosoftGraphUsersInGroups(
@@ -113,36 +90,41 @@ export async function readMicrosoftGraphUsersInGroups(
): Promise<{
users: UserEntity[]; // With all relations empty
}> {
const users: UserEntity[] = [];
const limiter = limiterFactory(10);
const transformer = options.transformer ?? defaultUserTransformer;
const userGroupMemberPromises: Promise<void>[] = [];
const userPromises: Promise<void>[] = [];
const groupMemberUsers: Set<string> = new Set();
const userGroupMembers = new Map<string, MicrosoftGraph.User>();
for await (const group of client.getGroups(
{
expand: options.groupExpand,
search: options.userGroupMemberSearch,
filter: options.userGroupMemberFilter,
select: ['id', 'displayName'],
top: PAGE_SIZE,
},
options.queryMode,
)) {
// Process all groups in parallel, otherwise it can take quite some time
userGroupMemberPromises.push(
limiter(async () => {
for await (const member of client.getGroupMembers(group.id!)) {
if (!member.id) {
continue;
}
if (member['@odata.type'] === '#microsoft.graph.user') {
groupMemberUsers.add(member.id);
}
let groupMemberCount = 0;
for await (const user of client.getGroupUserMembers(
group.id!,
{
expand: options.userExpand,
top: PAGE_SIZE,
},
options.queryMode,
)) {
userGroupMembers.set(user.id!, user);
groupMemberCount++;
}
options.logger.debug('Read users from group', {
groupId: group.id,
groupName: group.displayName,
memberCount: groupMemberCount,
});
}),
);
}
@@ -150,47 +132,19 @@ export async function readMicrosoftGraphUsersInGroups(
// Wait for all group members
await Promise.all(userGroupMemberPromises);
options.logger.info(`groupMemberUsers ${groupMemberUsers.size}`);
for (const userId of groupMemberUsers) {
// Process all users in parallel, otherwise it can take quite some time
userPromises.push(
limiter(async () => {
let user;
let userPhoto;
try {
user = await client.getUserProfile(userId, {
expand: options.userExpand,
});
} catch (e) {
options.logger.warn(`Unable to load user for ${userId}, ${e}`);
}
if (user) {
try {
userPhoto = await client.getUserPhotoWithSizeLimit(
user.id!,
// We are limiting the photo size, as users with full resolution photos
// can make the Backstage API slow
120,
);
} catch (e) {
options.logger.warn(`Unable to load userphoto for ${userId}, ${e}`);
}
options.logger.info('Read users from group membership', {
groupCount: userGroupMemberPromises.length,
userCount: userGroupMembers.size,
});
const entity = await transformer(user, userPhoto);
if (!entity) {
return;
}
users.push(entity);
}
}),
);
}
// Wait for all users and photos to be downloaded
await Promise.all(userPromises);
return { users };
return {
users: await transformUsers(
client,
userGroupMembers.values(),
options.logger,
options.transformer,
),
};
}
export async function readMicrosoftGraphOrganization(
@@ -248,6 +202,7 @@ export async function readMicrosoftGraphGroups(
search: options?.groupSearch,
filter: options?.groupFilter,
select: options?.groupSelect,
top: PAGE_SIZE,
},
options?.queryMode,
)) {
@@ -269,7 +224,9 @@ export async function readMicrosoftGraphGroups(
return;
}
for await (const member of client.getGroupMembers(group.id!)) {
for await (const member of client.getGroupMembers(group.id!, {
top: PAGE_SIZE,
})) {
if (!member.id) {
continue;
}
@@ -461,6 +418,56 @@ export async function readMicrosoftGraphOrg(
return { users, groups };
}
async function transformUsers(
client: MicrosoftGraphClient,
users: Iterable<MicrosoftGraph.User> | AsyncIterable<MicrosoftGraph.User>,
logger: Logger,
transformer?: UserTransformer,
) {
const limiter = limiterFactory(10);
const resolvedTransformer = transformer ?? defaultUserTransformer;
const promises: Promise<void>[] = [];
const entities: UserEntity[] = [];
// Process all users in parallel, otherwise it can take quite some time
for await (const user of users) {
promises.push(
limiter(async () => {
let userPhoto;
try {
userPhoto = await client.getUserPhotoWithSizeLimit(
user.id!,
// We are limiting the photo size, as users with full resolution photos
// can make the Backstage API slow
120,
);
} catch (e) {
logger.warn(`Unable to load user photo for`, {
user: user.id,
error: e,
});
}
const entity = await resolvedTransformer(user, userPhoto);
if (entity) {
entities.push(entity);
}
}),
);
}
// Wait for all users and photos to be downloaded
await Promise.all(promises);
logger.debug('Finished transforming users', {
microsoftUserCount: promises.length,
backstageUserCount: entities.length,
});
return entities;
}
function ensureItem(
target: Map<string, Set<string>>,
key: string,