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

Wrap google cloud functions with a Proxy(). #3035

Merged
merged 2 commits into from
Nov 12, 2020
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
3 changes: 2 additions & 1 deletion packages/serverless/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
"@sentry/node": "5.27.3",
"@sentry/types": "5.27.3",
"@sentry/utils": "5.27.3",
"@types/aws-lambda": "^8.10.62",
"@types/express": "^4.17.2",
"tslib": "^1.9.3"
},
"devDependencies": {
Expand All @@ -28,7 +30,6 @@
"@google-cloud/functions-framework": "^1.7.1",
"@google-cloud/pubsub": "^2.5.0",
"@sentry-internal/eslint-config-sdk": "5.27.3",
"@types/aws-lambda": "^8.10.62",
"@types/node": "^14.6.4",
"aws-sdk": "^2.765.0",
"eslint": "7.6.0",
Expand Down
24 changes: 16 additions & 8 deletions packages/serverless/src/gcpfunction/cloud_events.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
import {
CloudEventFunction,
CloudEventFunctionWithCallback,
} from '@google-cloud/functions-framework/build/src/functions';
import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
import { logger } from '@sentry/utils';

import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
import { domainify, getActiveDomain, proxyFunction } from '../utils';
import {
CloudEventFunction,
CloudEventFunctionWithCallback,
configureScopeWithContext,
WrapperOptions,
} from './general';

export type CloudEventFunctionWrapperOptions = WrapperOptions;

Expand All @@ -21,6 +21,14 @@ export type CloudEventFunctionWrapperOptions = WrapperOptions;
export function wrapCloudEventFunction(
fn: CloudEventFunction | CloudEventFunctionWithCallback,
wrapOptions: Partial<CloudEventFunctionWrapperOptions> = {},
): CloudEventFunctionWithCallback {
return proxyFunction(fn, f => domainify(_wrapCloudEventFunction(f, wrapOptions)));
}

/** */
function _wrapCloudEventFunction(
fn: CloudEventFunction | CloudEventFunctionWithCallback,
wrapOptions: Partial<CloudEventFunctionWrapperOptions> = {},
): CloudEventFunctionWithCallback {
const options: CloudEventFunctionWrapperOptions = {
flushTimeout: 2000,
Expand All @@ -41,7 +49,7 @@ export function wrapCloudEventFunction(
scope.setSpan(transaction);
});

const activeDomain = getActiveDomain();
const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion

activeDomain.on('error', captureException);

Expand Down
16 changes: 11 additions & 5 deletions packages/serverless/src/gcpfunction/events.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
import { EventFunction, EventFunctionWithCallback } from '@google-cloud/functions-framework/build/src/functions';
import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
import { logger } from '@sentry/utils';

import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
import { domainify, getActiveDomain, proxyFunction } from '../utils';
import { configureScopeWithContext, EventFunction, EventFunctionWithCallback, WrapperOptions } from './general';

export type EventFunctionWrapperOptions = WrapperOptions;

Expand All @@ -18,6 +16,14 @@ export type EventFunctionWrapperOptions = WrapperOptions;
export function wrapEventFunction(
fn: EventFunction | EventFunctionWithCallback,
wrapOptions: Partial<EventFunctionWrapperOptions> = {},
): EventFunctionWithCallback {
return proxyFunction(fn, f => domainify(_wrapEventFunction(f, wrapOptions)));
}

/** */
function _wrapEventFunction(
fn: EventFunction | EventFunctionWithCallback,
wrapOptions: Partial<EventFunctionWrapperOptions> = {},
): EventFunctionWithCallback {
const options: EventFunctionWrapperOptions = {
flushTimeout: 2000,
Expand All @@ -38,7 +44,7 @@ export function wrapEventFunction(
scope.setSpan(transaction);
});

const activeDomain = getActiveDomain();
const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion

activeDomain.on('error', captureException);

Expand Down
53 changes: 41 additions & 12 deletions packages/serverless/src/gcpfunction/general.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,48 @@
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
import { Context } from '@google-cloud/functions-framework/build/src/functions';
import { Scope } from '@sentry/node';
import { Context as SentryContext } from '@sentry/types';
import * as domain from 'domain';
import { Request, Response } from 'express'; // eslint-disable-line import/no-extraneous-dependencies
import { hostname } from 'os';

export interface HttpFunction {
(req: Request, res: Response): any; // eslint-disable-line @typescript-eslint/no-explicit-any
}

export interface EventFunction {
(data: Record<string, any>, context: Context): any; // eslint-disable-line @typescript-eslint/no-explicit-any
}

export interface EventFunctionWithCallback {
(data: Record<string, any>, context: Context, callback: Function): any; // eslint-disable-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
}

export interface CloudEventFunction {
(cloudevent: CloudEventsContext): any; // eslint-disable-line @typescript-eslint/no-explicit-any
}

export interface CloudEventFunctionWithCallback {
(cloudevent: CloudEventsContext, callback: Function): any; // eslint-disable-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
}

export interface CloudFunctionsContext {
eventId?: string;
timestamp?: string;
eventType?: string;
resource?: string;
}

export interface CloudEventsContext {
[key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
type?: string;
specversion?: string;
source?: string;
id?: string;
time?: string;
schemaurl?: string;
contenttype?: string;
}

export type Context = CloudFunctionsContext | CloudEventsContext;

export interface WrapperOptions {
flushTimeout: number;
}
Expand All @@ -24,11 +61,3 @@ export function configureScopeWithContext(scope: Scope, context: Context): void
scope.setTag('server_name', process.env.SENTRY_NAME || hostname());
scope.setContext('gcp.function.context', { ...context } as SentryContext);
}

/**
* @returns Current active domain with a correct type.
*/
export function getActiveDomain(): domain.Domain {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
return (domain as any).active as domain.Domain;
}
31 changes: 21 additions & 10 deletions packages/serverless/src/gcpfunction/http.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,15 @@
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
import { HttpFunction } from '@google-cloud/functions-framework/build/src/functions';
import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
import { logger, stripUrlQueryAndFragment } from '@sentry/utils';

import { getActiveDomain, WrapperOptions } from './general';
import { domainify, getActiveDomain, proxyFunction } from './../utils';
import { HttpFunction, WrapperOptions } from './general';

type Request = Parameters<HttpFunction>[0];
type Response = Parameters<HttpFunction>[1];
type ParseRequestOptions = Handlers.ParseRequestOptions;

export interface HttpFunctionWrapperOptions extends WrapperOptions {
parseRequestOptions: ParseRequestOptions;
}

export { Request, Response };

const { parseRequest } = Handlers;

/**
Expand All @@ -29,14 +23,30 @@ export function wrapHttpFunction(
fn: HttpFunction,
wrapOptions: Partial<HttpFunctionWrapperOptions> = {},
): HttpFunction {
const wrap = (f: HttpFunction): HttpFunction => domainify(_wrapHttpFunction(f, wrapOptions));

let overrides: Record<PropertyKey, unknown> | undefined;

// Functions emulator from firebase-tools has a hack-ish workaround that saves the actual function
// passed to `onRequest(...)` and in fact runs it so we need to wrap it too.
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
const emulatorFunc = (fn as any).__emulator_func as HttpFunction | undefined;
if (emulatorFunc) {
overrides = { __emulator_func: proxyFunction(emulatorFunc, wrap) };
}
return proxyFunction(fn, wrap, overrides);
}

/** */
function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWrapperOptions> = {}): HttpFunction {
const options: HttpFunctionWrapperOptions = {
flushTimeout: 2000,
parseRequestOptions: {},
...wrapOptions,
};
return (req, res) => {
const reqMethod = (req.method || '').toUpperCase();
const reqUrl = req.url && stripUrlQueryAndFragment(req.url);
const reqUrl = stripUrlQueryAndFragment(req.originalUrl || req.url || '');

const transaction = startTransaction({
name: `${reqMethod} ${reqUrl}`,
Expand All @@ -59,7 +69,8 @@ export function wrapHttpFunction(

// functions-framework creates a domain for each incoming request so we take advantage of this fact and add an error handler.
// BTW this is the only way to catch any exception occured during request lifecycle.
getActiveDomain().on('error', err => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
getActiveDomain()!.on('error', err => {
captureException(err);
});

Expand Down
55 changes: 55 additions & 0 deletions packages/serverless/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Event, SDK_VERSION } from '@sentry/node';
import { addExceptionMechanism } from '@sentry/utils';
import * as domain from 'domain';

/**
* Event processor that will override SDK details to point to the serverless SDK instead of Node,
Expand Down Expand Up @@ -31,3 +32,57 @@ export function serverlessEventProcessor(integration: string): (event: Event) =>
return event;
};
}

/**
* @returns Current active domain with a correct type.
*/
export function getActiveDomain(): domain.Domain | null {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
return (domain as any).active as domain.Domain | null;
}

/**
* @param fn function to run
* @returns function which runs in the newly created domain or in the existing one
*/
export function domainify<A extends unknown[], R>(fn: (...args: A) => R): (...args: A) => R {
return (...args) => {
if (getActiveDomain()) {
return fn(...args);
}
const dom = domain.create();
return dom.run(() => fn(...args));
};
}

/**
* @param source function to be wrapped
* @param wrap wrapping function that takes source and returns a wrapper
* @param overrides properties to override in the source
* @returns wrapped function
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function proxyFunction<A extends any[], R, F extends (...args: A) => R>(
source: F,
wrap: (source: F) => F,
overrides?: Record<PropertyKey, unknown>,
): F {
const wrapper = wrap(source);
const handler: ProxyHandler<F> = {
apply: <T>(_target: F, thisArg: T, args: A) => {
return wrapper.apply(thisArg, args);
},
};

if (overrides) {
handler.get = (target, prop) => {
// eslint-disable-next-line no-prototype-builtins
if (overrides.hasOwnProperty(prop)) {
return overrides[prop as string];
}
return (target as Record<PropertyKey, unknown>)[prop as string];
};
}

return new Proxy(source, handler);
}
14 changes: 8 additions & 6 deletions packages/serverless/test/gcpfunction.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { Event } from '@sentry/types';
import * as domain from 'domain';

import * as Sentry from '../src';
import { wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
import {
CloudEventFunction,
CloudEventFunctionWithCallback,
EventFunction,
EventFunctionWithCallback,
HttpFunction,
} from '@google-cloud/functions-framework/build/src/functions';
import { Event } from '@sentry/types';
import * as domain from 'domain';

import * as Sentry from '../src';
import { Request, Response, wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
Request,
Response,
} from '../src/gcpfunction/general';

/**
* Why @ts-ignore some Sentry.X calls
Expand Down