Skip to content

Commit

Permalink
feat(logs): POST /logs/search (#2063)
Browse files Browse the repository at this point in the history
## Describe your changes

Fixes NAN-846

> [!NOTE]
> This is the proposed change for new API routes, discussed last week

- **New route** `POST /api/v1/logs/search` 
POST because it will require a lot of params that are easier to send
with a json body.

- **Integration test**
This is the first e2e API test. To achieve it I needed an auth method
not tied to a session, so when it's in test mode we are using SecretKey
auth, let me know if that's alright.

- Add async wrapper to catch errors
Note that it won't be useful anymore after express 5 but it's still a
long way before release
  • Loading branch information
bodinsamuel authored May 10, 2024
1 parent 0085347 commit e08d85f
Show file tree
Hide file tree
Showing 27 changed files with 346 additions and 33 deletions.
7 changes: 5 additions & 2 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ services:
- '${SERVER_PORT:-3003}:${SERVER_PORT:-3003}'
depends_on:
- nango-db
- nanog-opensearch
- nango-opensearch
networks:
- nango

Expand All @@ -58,7 +58,9 @@ services:
networks:
- nango

nanog-opensearch:
# Optional dependency
# To not load opensearch, set NANGO_LOGS_OS_URL=false in nango-server and disable this dependency
nango-opensearch:
image: opensearchproject/opensearch:2.13.0
container_name: nango-opensearch
ulimits:
Expand All @@ -79,6 +81,7 @@ services:
networks:
nango:


volumes:
opensearch-data1:
driver: local
4 changes: 3 additions & 1 deletion package-lock.json

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

4 changes: 2 additions & 2 deletions packages/logs/lib/client.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { logger } from './utils.js';
import { deleteIndex, migrateMapping } from './es/helpers.js';
import type { ListOperations } from './models/messages.js';
import { getOperation, listMessages, listOperations } from './models/messages.js';
import type { OperationRowInsert } from './types/messages.js';
import type { OperationRowInsert } from '@nangohq/types';
import { afterEach } from 'node:test';
import { logContextGetter } from './models/logContextGetter.js';

Expand Down Expand Up @@ -33,7 +33,7 @@ describe('client', () => {
expect(ctx).toMatchObject({ id: expect.any(String) });
expect(spy).toHaveBeenCalled();

const list = await listOperations({ limit: 1 });
const list = await listOperations({ accountId: account.id, limit: 1 });
expect(list).toStrictEqual<ListOperations>({
count: 1,
items: [
Expand Down
4 changes: 2 additions & 2 deletions packages/logs/lib/client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { MessageRow, MessageRowInsert, MessageMeta } from './types/messages.js';
import type { MessageRow, MessageRowInsert, MessageMeta } from '@nangohq/types';
import { setRunning, createMessage, setFailed, setCancelled, setTimeouted, setSuccess, update } from './models/messages.js';
import { getFormattedMessage } from './models/helpers.js';
import { errorToObject, metrics, stringifyError } from '@nangohq/utils';
Expand Down Expand Up @@ -65,7 +65,7 @@ export class LogContext {
async error(message: string, meta: (MessageMeta & { error?: unknown }) | null = null): Promise<void> {
const { error, ...rest } = meta || {};
const err = error ? { name: 'Unknown Error', message: 'unknown error', ...errorToObject(error) } : null;
await this.log({ type: 'log', level: 'error', message, error: err ? { name: err.name, message: err?.message } : null, meta: rest, source: 'internal' });
await this.log({ type: 'log', level: 'error', message, error: err ? { name: err.name, message: err.message } : null, meta: rest, source: 'internal' });
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/logs/lib/es/schema.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { opensearchtypes } from '@opensearch-project/opensearch';
import type { MessageRow } from '../types/messages';
import type { MessageRow } from '@nangohq/types';
import { envs } from '../env.js';

const props: Record<keyof MessageRow, opensearchtypes.MappingProperty> = {
Expand Down
2 changes: 2 additions & 0 deletions packages/logs/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ export * from './es/helpers.js';
export * from './client.js';
export * from './models/helpers.js';
export * from './models/logContextGetter.js';
export * as model from './models/messages.js';
export { envs } from './env.js';
2 changes: 1 addition & 1 deletion packages/logs/lib/models/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { nanoid } from '@nangohq/utils';
import type { MessageRow } from '../types/messages';
import type { MessageRow } from '@nangohq/types';

export interface FormatMessageData {
account?: { id: number; name?: string };
Expand Down
2 changes: 1 addition & 1 deletion packages/logs/lib/models/logContextGetter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { FormatMessageData } from './helpers.js';
import { getFormattedMessage } from './helpers.js';
import { LogContext } from '../client.js';
import { logger } from '../utils.js';
import type { MessageRow, OperationRowInsert } from '../types/messages.js';
import type { MessageRow, OperationRowInsert } from '@nangohq/types';

interface Options {
dryRun?: boolean;
Expand Down
26 changes: 16 additions & 10 deletions packages/logs/lib/models/messages.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { client } from '../es/client.js';
import type { MessageRow } from '../types/messages.js';
import type { MessageRow, OperationRow } from '@nangohq/types';
import { indexMessages } from '../es/schema.js';
import type { opensearchtypes } from '@opensearch-project/opensearch';

export interface ListOperations {
count: number;
items: MessageRow[];
items: OperationRow[];
}
export interface ListMessages {
count: number;
Expand All @@ -26,19 +27,24 @@ export async function createMessage(row: MessageRow): Promise<void> {
/**
* List operations
*/
export async function listOperations(opts: { limit: number }): Promise<ListOperations> {
export async function listOperations(opts: { accountId: number; environmentId?: number; limit: number }): Promise<ListOperations> {
const query: opensearchtypes.QueryDslQueryContainer = {
bool: {
must: [{ term: { accountId: opts.accountId } }],
must_not: { exists: { field: 'parentId' } },
should: []
}
};
if (opts.environmentId && Array.isArray(query.bool?.must)) {
query.bool.must.push({ term: { environmentId: opts.environmentId } });
}

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

Expand Down
1 change: 1 addition & 0 deletions packages/logs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"zod": "3.22.4"
},
"devDependencies": {
"@nangohq/types": "file:../types",
"type-fest": "4.14.0",
"vitest": "0.33.0",
"winston": "3.8.2"
Expand Down
3 changes: 3 additions & 0 deletions packages/logs/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"outDir": "dist"
},
"references": [
{
"path": "../types"
},
{
"path": "../utils"
}
Expand Down
160 changes: 160 additions & 0 deletions packages/server/lib/controllers/v1/logs/searchLogs.integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import { logContextGetter, migrateMapping } from '@nangohq/logs';
import { multipleMigrations, seeders } from '@nangohq/shared';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { 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/search', { method: 'POST', query: { env: 'dev' } });

shouldBeProtected(res);
});

it('should enforce env query params', async () => {
const { env } = await seeders.seedAccountEnvAndUser();
const res = await api.fetch('/api/v1/logs/search', { method: 'POST', token: env.secret_key });

shouldRequireQueryEnv(res);
});

it('should validate body', async () => {
const { env } = await seeders.seedAccountEnvAndUser();
const res = await api.fetch('/api/v1/logs/search', {
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: '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 logs and get empty results', async () => {
const { env } = await seeders.seedAccountEnvAndUser();
const res = await api.fetch('/api/v1/logs/search', {
method: 'POST',
query: { env: 'dev' },
token: env.secret_key,
body: { limit: 10 }
});

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

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

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

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

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

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

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

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

expect(res.res.status).toBe(200);
expect(res.json).toStrictEqual<typeof res.json>({
data: [],
pagination: { total: 0 }
});
});
});
41 changes: 41 additions & 0 deletions packages/server/lib/controllers/v1/logs/searchLogs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { z } from 'zod';
import { asyncWrapper } from '../../../utils/asyncWrapper.js';
import { requireEmptyQuery, zodErrorToHTTP } from '../../../utils/validation.js';
import type { SearchLogs } from '@nangohq/types';
import { model, envs } from '@nangohq/logs';

const validation = z
.object({
limit: z.number().optional().default(100)
})
.strict();

export const searchLogs = asyncWrapper<SearchLogs>(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;
}

const env = res.locals['environment'];
const body: Required<SearchLogs['Body']> = val.data;
const rawOps = await model.listOperations({ accountId: env.account_id, environmentId: env.id, limit: body.limit });

res.status(200).send({
data: rawOps.items,
pagination: { total: rawOps.count }
});
});
4 changes: 2 additions & 2 deletions packages/server/lib/middleware/ratelimit.middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ export const rateLimiterMiddleware = (req: Request, res: Response, next: NextFun
res.setHeader('Retry-After', Math.floor(rateLimiterRes.msBeforeNext / 1000));
setXRateLimitHeaders(rateLimiterRes);
logger.info(`Rate limit exceeded for ${key}. Request: ${req.method} ${req.path})`);
res.status(429).send('Too Many Requests');
res.status(429).send({ error: { code: 'too_many_request' } });
return;
}

res.status(500).send('Server error');
res.status(500).send({ error: { code: 'server_error' } });
});
};

Expand Down
Loading

0 comments on commit e08d85f

Please sign in to comment.