Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(logs): drawer with details #2155

Merged
merged 18 commits into from
May 22, 2024
Merged
534 changes: 534 additions & 0 deletions package-lock.json

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions packages/logs/lib/models/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { nanoid } from '@nangohq/utils';
import type { MessageRow } from '@nangohq/types';
import { z } from 'zod';

export const operationIdRegex = z.string().regex(/([0-9]|[a-zA-Z0-9]{20})/);

export interface FormatMessageData {
account?: { id: number; name?: string };
Expand Down
53 changes: 40 additions & 13 deletions packages/logs/lib/models/messages.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { client } from '../es/client.js';
import type { MessageRow, OperationRow, SearchLogsState } from '@nangohq/types';
import type { MessageRow, OperationRow, SearchOperationsState } from '@nangohq/types';
import { indexMessages } from '../es/schema.js';
import type { opensearchtypes } from '@opensearch-project/opensearch';
import { errors } from '@opensearch-project/opensearch';
Expand Down Expand Up @@ -30,7 +30,12 @@ export async function createMessage(row: MessageRow): Promise<void> {
/**
* List operations
*/
export async function listOperations(opts: { accountId: number; environmentId?: number; limit: number; states: SearchLogsState[] }): Promise<ListOperations> {
export async function listOperations(opts: {
accountId: number;
environmentId?: number;
limit: number;
states?: SearchOperationsState[] | undefined;
}): Promise<ListOperations> {
const query: opensearchtypes.QueryDslQueryContainer = {
bool: {
must: [{ term: { accountId: opts.accountId } }],
Expand All @@ -41,7 +46,8 @@ export async function listOperations(opts: { accountId: number; environmentId?:
if (opts.environmentId) {
(query.bool!.must as opensearchtypes.QueryDslQueryContainer[]).push({ term: { environmentId: opts.environmentId } });
}
if (opts.states.length > 1 || opts.states[0] !== 'all') {
if (opts.states && (opts.states.length > 1 || opts.states[0] !== 'all')) {
// Where or
(query.bool!.must as opensearchtypes.QueryDslQueryContainer[]).push({
bool: {
should: opts.states.map((state) => {
Expand Down Expand Up @@ -134,21 +140,42 @@ export async function setTimeouted(opts: Pick<MessageRow, 'id'>): Promise<void>
/**
* List messages
*/
export async function listMessages(opts: { parentId: MessageRow['parentId']; limit: number }): Promise<ListMessages> {
export async function listMessages(opts: {
parentId: string;
limit: number;
states?: SearchOperationsState[] | undefined;
search?: string | undefined;
}): Promise<ListMessages> {
const query: opensearchtypes.QueryDslQueryContainer = {
bool: {
must: [{ term: { parentId: opts.parentId } }],
should: []
}
};

if (opts.states && (opts.states.length > 1 || opts.states[0] !== 'all')) {
// Where or
(query.bool!.must as opensearchtypes.QueryDslQueryContainer[]).push({
bool: {
should: opts.states.map((state) => {
return { term: { state } };
})
}
});
}
if (opts.search) {
(query.bool!.must as opensearchtypes.QueryDslQueryContainer[]).push({
match_phrase_prefix: { message: { query: opts.search } }
});
}

const res = await client.search<{ hits: { total: number; hits: { _source: MessageRow }[] } }>({
index: indexMessages.index,
size: 5000,
size: opts.limit,
sort: ['createdAt:desc', '_score'],
track_total_hits: true,
body: {
query: {
bool: {
must: [{ term: { parentId: opts.parentId } }]
}
}
}
body: { query }
});

const hits = res.body.hits;

return {
Expand Down
4 changes: 2 additions & 2 deletions packages/server/lib/controllers/v1/logs/getOperation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { z } from 'zod';
import { asyncWrapper } from '../../../utils/asyncWrapper.js';
import { requireEmptyQuery, zodErrorToHTTP } from '@nangohq/utils';
import type { GetOperation } from '@nangohq/types';
import { model, envs } from '@nangohq/logs';
import { model, envs, operationIdRegex } from '@nangohq/logs';

const validation = z
.object({
operationId: z.string().regex(/^([0-9]+|[a-zA-Z0-9]{20})$/)
operationId: operationIdRegex
})
.strict();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
import { logContextGetter, migrateMapping } from '@nangohq/logs';
import { multipleMigrations, seeders } from '@nangohq/shared';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { isError, isSuccess, runServer, shouldBeProtected, shouldRequireQueryEnv } from '../../../utils/tests.js';

let api: Awaited<ReturnType<typeof runServer>>;
describe('GET /logs', () => {
beforeAll(async () => {
await multipleMigrations();
await migrateMapping();

api = await runServer();
});
afterAll(() => {
api.server.close();
});

it('should be protected', async () => {
const res = await api.fetch('/api/v1/logs/messages', { method: 'POST', query: { env: 'dev' }, body: { operationId: '1' } });

shouldBeProtected(res);
});

it('should enforce env query params', async () => {
const { env } = await seeders.seedAccountEnvAndUser();
// @ts-expect-error missing query on purpose
const res = await api.fetch('/api/v1/logs/messages', { method: 'POST', token: env.secret_key, body: { operationId: '1' } });

shouldRequireQueryEnv(res);
});

it('should validate body', async () => {
const { env } = await seeders.seedAccountEnvAndUser();
const res = await api.fetch('/api/v1/logs/messages', {
method: 'POST',
query: { env: 'dev' },
token: env.secret_key,
// @ts-expect-error on purpose
body: { limit: 'a', foo: 'bar' }
});

expect(res.json).toStrictEqual({
error: {
code: 'invalid_body',
errors: [
{
code: 'invalid_type',
message: 'Required',
path: ['operationId']
},
{
code: 'invalid_type',
message: 'Expected number, received string',
path: ['limit']
},
{
code: 'unrecognized_keys',
message: "Unrecognized key(s) in object: 'foo'",
path: []
}
]
}
});
expect(res.res.status).toBe(400);
});

it('should search messages and get empty results', async () => {
const { account, env } = await seeders.seedAccountEnvAndUser();

const logCtx = await logContextGetter.create({ message: 'test 1', operation: { type: 'auth' } }, { account, environment: env });
await logCtx.success();

const res = await api.fetch('/api/v1/logs/messages', {
method: 'POST',
query: { env: 'dev' },
token: env.secret_key,
body: { operationId: logCtx.id, limit: 10 }
});

isSuccess(res.json);
expect(res.res.status).toBe(200);
expect(res.json).toStrictEqual({
data: [],
pagination: { total: 0 }
});
});

it('should search messages and get one result', async () => {
const { env, account } = await seeders.seedAccountEnvAndUser();

const logCtx = await logContextGetter.create({ message: 'test 1', operation: { type: 'auth' } }, { account, environment: env });
await logCtx.info('test info');
await logCtx.success();

const res = await api.fetch('/api/v1/logs/messages', {
method: 'POST',
query: { env: 'dev' },
token: env.secret_key,
body: { operationId: logCtx.id, limit: 10 }
});

isSuccess(res.json);
expect(res.res.status).toBe(200);
expect(res.json).toStrictEqual<typeof res.json>({
data: [
{
accountId: null,
accountName: null,
code: null,
configId: null,
configName: null,
connectionId: null,
connectionName: null,
createdAt: expect.toBeIsoDate(),
endedAt: null,
environmentId: null,
environmentName: null,
error: null,
id: expect.any(String),
jobId: null,
level: 'info',
message: 'test info',
meta: null,
operation: null,
parentId: logCtx.id,
request: null,
response: null,
source: 'internal',
startedAt: null,
state: 'waiting',
syncId: null,
syncName: null,
title: null,
type: 'log',
updatedAt: expect.toBeIsoDate(),
userId: null
}
],
pagination: { total: 1 }
});
});

it('should search messages and not return results from an other account', async () => {
const { account, env } = await seeders.seedAccountEnvAndUser();
const env2 = await seeders.seedAccountEnvAndUser();

const logCtx = await logContextGetter.create({ message: 'test 1', operation: { type: 'auth' } }, { account, environment: env });
await logCtx.info('test info');
await logCtx.success();

const res = await api.fetch('/api/v1/logs/messages', {
method: 'POST',
query: { env: 'dev' },
token: env2.env.secret_key,
body: { operationId: logCtx.id, limit: 10 }
});

isError(res.json);
expect(res.res.status).toBe(404);
expect(res.json).toStrictEqual<typeof res.json>({ error: { code: 'not_found' } });
});
});
69 changes: 69 additions & 0 deletions packages/server/lib/controllers/v1/logs/searchMessages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { z } from 'zod';
import { asyncWrapper } from '../../../utils/asyncWrapper.js';
import type { SearchMessages } from '@nangohq/types';
import { model, envs, operationIdRegex } from '@nangohq/logs';
import { requireEmptyQuery, zodErrorToHTTP } from '@nangohq/utils';

const validation = z
.object({
operationId: operationIdRegex,
limit: z.number().optional().default(100),
search: z.string().optional(),
states: z
.array(z.enum(['all', 'waiting', 'running', 'success', 'failed', 'timeout', 'cancelled']))
.optional()
.default(['all'])
})
.strict();

export const searchMessages = asyncWrapper<SearchMessages>(async (req, res) => {
if (!envs.NANGO_LOGS_ENABLED) {
res.status(404).send({ error: { code: 'feature_disabled' } });
return;
}

const emptyQuery = requireEmptyQuery(req, { withEnv: true });
if (emptyQuery) {
res.status(400).send({ error: { code: 'invalid_query_params', errors: zodErrorToHTTP(emptyQuery.error) } });
return;
}

const val = validation.safeParse(req.body);
if (!val.success) {
res.status(400).send({
error: { code: 'invalid_body', errors: zodErrorToHTTP(val.error) }
});
return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those 2 checks come back regularly. I imagine we could abstract them

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using your wrapper might be the best solution because I could not come up with an abstraction that save more than 1 line (you still need to return something so you still need a if)
but if you have something in the meantime, happy to change ☺️

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a concrete solution right now. my comment was more to keep it in the back on our mind so at some point we come up with something

}

const { environment, account } = res.locals;

// Manually ensure that `operationId` belongs to the account for now
// Because not all the logs have accountId/environmentId
try {
const operation = await model.getOperation({ id: val.data.operationId });
if (operation.accountId !== account.id || operation.environmentId !== environment.id) {
res.status(404).send({ error: { code: 'not_found' } });
return;
}
} catch (err) {
if (err instanceof model.ResponseError && err.statusCode === 404) {
res.status(404).send({ error: { code: 'not_found' } });
return;
}
throw err;
}

const body: SearchMessages['Body'] = val.data;
const rawOps = await model.listMessages({
parentId: body.operationId,
limit: body.limit!,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way make the type reflect the fact that after validation, since limit has a default value it cannot be undefined anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's a bit annoying, with types you often need to produce multiple variants: before/after validation, before/after creation, etc.
I'm not sure if there is a good answer expect maybe having a Body variant in the Endpoint types that you can alter to reflect post-validation type.

states: body.states,
search: body.search
});

res.status(200).send({
data: rawOps.items,
pagination: { total: rawOps.count }
});
});
Loading
Loading