Skip to content

Commit ea14efc

Browse files
committed
addressed comments and remove getExecutionId().
1 parent 409c5c9 commit ea14efc

File tree

8 files changed

+66
-81
lines changed

8 files changed

+66
-81
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ ignored.
190190
| `--target` | `FUNCTION_TARGET` | The name of the exported function to be invoked in response to requests. Default: `function` |
191191
| `--signature-type` | `FUNCTION_SIGNATURE_TYPE` | The signature used when writing your function. Controls unmarshalling rules and determines which arguments are used to invoke your function. Default: `http`; accepted values: `http` or `event` or `cloudevent` |
192192
| `--source` | `FUNCTION_SOURCE` | The path to the directory of your function. Default: `cwd` (the current working directory) |
193-
| / | `LOG_EXECUTION_ID` | Enables execution IDs in logs, either `true` or `false`. When not specified, default to enable. Requires Node.js 13.0.0 or later. |
193+
| `--log-execution-id`| `LOG_EXECUTION_ID` | Enables execution IDs in logs, either `true` or `false`. When not specified, default to disable. Requires Node.js 13.0.0 or later. |
194194
195195
You can set command-line flags in your `package.json` via the `start` script.
196196
For example:

docs/generated/api.json

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -779,29 +779,6 @@
779779
],
780780
"extendsTokenRanges": []
781781
},
782-
{
783-
"kind": "Variable",
784-
"canonicalReference": "@google-cloud/functions-framework!getExecutionId:var",
785-
"docComment": "/**\n * Gets the request-specific execution id\n *\n * @public\n */\n",
786-
"excerptTokens": [
787-
{
788-
"kind": "Content",
789-
"text": "getExecutionId: "
790-
},
791-
{
792-
"kind": "Content",
793-
"text": "() => string | undefined"
794-
}
795-
],
796-
"fileUrlPath": "src/function_registry.ts",
797-
"isReadonly": true,
798-
"releaseTag": "Public",
799-
"name": "getExecutionId",
800-
"variableTypeTokenRange": {
801-
"startIndex": 1,
802-
"endIndex": 2
803-
}
804-
},
805782
{
806783
"kind": "TypeAlias",
807784
"canonicalReference": "@google-cloud/functions-framework!HandlerFunction:type",

docs/generated/api.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,6 @@ export interface EventFunctionWithCallback {
5858
(data: {}, context: Context, callback: Function): any;
5959
}
6060

61-
// @public
62-
export const getExecutionId: () => string | undefined;
63-
6461
// @public
6562
export type HandlerFunction<T = unknown, U = unknown> = HttpFunction | EventFunction | EventFunctionWithCallback | CloudEventFunction<T> | CloudEventFunctionWithCallback<T> | TypedFunction<T, U>;
6663

src/async_local_storage.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ export async function asyncLocalStorageMiddleware(
2323
next();
2424
return;
2525
}
26-
const asyncHooks = await import('node:async_hooks');
2726
if (!asyncLocalStorage) {
27+
const asyncHooks = await import('node:async_hooks');
2828
asyncLocalStorage = new asyncHooks.AsyncLocalStorage();
2929
}
3030

@@ -46,7 +46,3 @@ export function getCurrentContext(): ExecutionContext | undefined {
4646
}
4747
return asyncLocalStorage.getStore();
4848
}
49-
50-
export const getCurrentExecutionId = (): string | undefined => {
51-
return getCurrentContext()?.executionId;
52-
};

src/function_registry.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import {
2020
JsonInvocationFormat,
2121
} from './functions';
2222
import {SignatureType} from './types';
23-
import {getCurrentExecutionId} from './async_local_storage';
2423

2524
interface RegisteredFunction<T, U> {
2625
signatureType: SignatureType;
@@ -66,14 +65,6 @@ export const isValidFunctionName = (functionName: string): boolean => {
6665
return regex.test(functionName);
6766
};
6867

69-
/**
70-
* Gets the request-specific execution id
71-
* @public
72-
*/
73-
export const getExecutionId = (): string | undefined => {
74-
return getCurrentExecutionId();
75-
};
76-
7768
/**
7869
* Get a declaratively registered function
7970
* @param functionName the name with which the function was registered

src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ export * from './functions';
2020
/**
2121
* @public
2222
*/
23-
export {http, cloudEvent, typed, getExecutionId} from './function_registry';
23+
export {http, cloudEvent, typed} from './function_registry';

src/options.ts

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -114,29 +114,27 @@ const SignatureOption = new ConfigurableOption(
114114
);
115115

116116
export const requiredNodeJsVersionForLogExecutionID = '13.0.0';
117-
const logExecutionIDEnvVar = 'LOG_EXECUTION_ID';
118-
function enableExecutionId(envVars: NodeJS.ProcessEnv): boolean {
119-
const logExecutionID = envVars[logExecutionIDEnvVar];
120-
const nodeVersion = process.versions.node;
121-
const isVersionSatisfied = semver.gte(
122-
nodeVersion,
123-
requiredNodeJsVersionForLogExecutionID
124-
);
125-
// If not specified, default to false.
126-
if (typeof logExecutionID === 'undefined') {
127-
return false;
128-
}
129-
const isTrue =
130-
(typeof logExecutionID === 'boolean' && logExecutionID) ||
131-
(typeof logExecutionID === 'string' &&
132-
logExecutionID.toLowerCase() === 'true');
133-
if (isTrue && !isVersionSatisfied) {
134-
throw new OptionsError(
135-
`Execution id is only supported with Node.js versions ${requiredNodeJsVersionForLogExecutionID} and above. Your current version is ${nodeVersion}. Please upgrade.`
117+
const ExecutionIdOption = new ConfigurableOption(
118+
'log-execution-id',
119+
'LOG_EXECUTION_ID',
120+
false,
121+
x => {
122+
const nodeVersion = process.versions.node;
123+
const isVersionSatisfied = semver.gte(
124+
nodeVersion,
125+
requiredNodeJsVersionForLogExecutionID
136126
);
127+
const isTrue =
128+
(typeof x === 'boolean' && x) ||
129+
(typeof x === 'string' && x.toLowerCase() === 'true');
130+
if (isTrue && !isVersionSatisfied) {
131+
throw new OptionsError(
132+
`Execution id is only supported with Node.js versions ${requiredNodeJsVersionForLogExecutionID} and above. Your current version is ${nodeVersion}. Please upgrade.`
133+
);
134+
}
135+
return isTrue;
137136
}
138-
return isTrue;
139-
}
137+
);
140138

141139
export const helpText = `Example usage:
142140
functions-framework --target=helloWorld --port=8080
@@ -168,6 +166,6 @@ export const parseOptions = (
168166
sourceLocation: SourceLocationOption.parse(argv, envVars),
169167
signatureType: SignatureOption.parse(argv, envVars),
170168
printHelp: cliArgs[2] === '-h' || cliArgs[2] === '--help',
171-
enableExecutionId: enableExecutionId(envVars),
169+
enableExecutionId: ExecutionIdOption.parse(argv, envVars),
172170
};
173171
};

test/options.ts

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ describe('parseOptions', () => {
2626
name: string;
2727
cliOpts: string[];
2828
envVars: {[key: string]: string};
29-
expectedOptions: Partial<FrameworkOptions>;
29+
expectedOptions?: Partial<FrameworkOptions>;
3030
}
3131

3232
const testData: TestData[] = [
@@ -134,6 +134,7 @@ describe('parseOptions', () => {
134134
it(testCase.name, () => {
135135
const options = parseOptions(testCase.cliOpts, testCase.envVars);
136136
const {expectedOptions} = testCase;
137+
assert(expectedOptions);
137138
let opt: keyof FrameworkOptions;
138139
for (opt in expectedOptions) {
139140
assert.deepStrictEqual(expectedOptions[opt], options[opt]);
@@ -142,33 +143,58 @@ describe('parseOptions', () => {
142143
});
143144

144145
const cliOpts = ['bin/node', '/index.js'];
145-
it('default execution id support', () => {
146+
it('default disable execution id support', () => {
146147
const options = parseOptions(cliOpts, {});
147148
assert.strictEqual(options.enableExecutionId, false);
148149
});
149150

150-
it('disable execution id support', () => {
151+
it('disable execution id support by cli flag', () => {
152+
const options = parseOptions(
153+
['bin/node', '/index.js', '--log-execution-id=false'],
154+
{}
155+
);
156+
assert.strictEqual(options.enableExecutionId, false);
157+
});
158+
159+
it('disable execution id support by env var', () => {
151160
const envVars = {
152161
LOG_EXECUTION_ID: 'False',
153162
};
154163
const options = parseOptions(cliOpts, envVars);
155164
assert.strictEqual(options.enableExecutionId, false);
156165
});
157166

158-
it('enable execution id support', () => {
159-
const envVars = {
160-
LOG_EXECUTION_ID: 'TRUE',
161-
};
162-
if (
163-
semver.lt(process.versions.node, requiredNodeJsVersionForLogExecutionID)
164-
) {
165-
assert.throws(() => {
166-
parseOptions(cliOpts, envVars);
167-
});
168-
} else {
169-
const options = parseOptions(cliOpts, envVars);
170-
assert.strictEqual(options.enableExecutionId, true);
171-
}
167+
const executionIdTestData: TestData[] = [
168+
{
169+
name: 'enable execution id support by cli flag',
170+
cliOpts: ['bin/node', '/index.js', '--log-execution-id'],
171+
envVars: {},
172+
},
173+
{
174+
name: 'enable execution id support by env var',
175+
cliOpts: ['bin/node', '/index.js'],
176+
envVars: {LOG_EXECUTION_ID: 'True'},
177+
},
178+
{
179+
name: 'execution id prioritizes cli flag over env var',
180+
cliOpts: ['bin/node', '/index.js', '--log-execution-id=true'],
181+
envVars: {LOG_EXECUTION_ID: 'False'},
182+
},
183+
];
184+
185+
executionIdTestData.forEach(testCase => {
186+
it(testCase.name, () => {
187+
if (
188+
semver.lt(process.versions.node, requiredNodeJsVersionForLogExecutionID)
189+
) {
190+
assert.throws(() => {
191+
parseOptions(testCase.cliOpts, testCase.envVars);
192+
});
193+
} else {
194+
const options = parseOptions(testCase.cliOpts, testCase.envVars);
195+
assert.strictEqual(options.enableExecutionId, true);
196+
}
197+
});
172198
});
173199

174200
it('throws an exception for invalid signature types', () => {

0 commit comments

Comments
 (0)