Skip to content

Commit

Permalink
feat!: require the project ID to be set before starting the profiling…
Browse files Browse the repository at this point in the history 
… agent (#516)

BREAKING CHANGE: `start({...}) ` now throws an error when the profiling agent cannot be set up because required fields are not set in the config and cannot be determined based on metadata or environment variables.  

It also now fails to start the profiling agent when the project ID cannot be determined based on metadata or environment variables (previously, the agent would start, and attempt to determine the project ID on every request).
  • Loading branch information
@nolanmar511
nolanmar511 authored Dec 12, 2019
1 parent 1305a36 commit 5b46b66
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 126 deletions.
51 changes: 42 additions & 9 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ to your [`package.json`](https://docs.npmjs.com/files/package.json#dependencies)
2. Include and start the profiler at the beginning of your application:

```js
var profiler = require('@google-cloud/profiler').start();
require('@google-cloud/profiler').start().catch((err) => {
console.log(`Failed to start profiler: ${err}`);
});
```

Some environments require a configuration to be passed to the `start()`
Expand Down Expand Up @@ -86,7 +88,7 @@ configuration options. These options can be passed to the agent through the
object argument to the start command shown below:

```js
require('@google-cloud/profiler').start({disableTime: true});
await require('@google-cloud/profiler').start({disableTime: true});
```

Alternatively, you can provide the configuration through a config file. This
Expand All @@ -111,7 +113,7 @@ So, for example, to start the profiler with the log level at debug, you would
do this:

```js
require('@google-cloud/profiler').start({logLevel: 4});
await require('@google-cloud/profiler').start({logLevel: 4});
```

### Disabling heap or time profile collection
Expand All @@ -124,13 +126,13 @@ disable the collection of either type of profile.
To disable time profile collection, set `disableTime` to true:

```js
require('@google-cloud/profiler').start({disableTime: true});
await require('@google-cloud/profiler').start({disableTime: true});
```

To disable heap profile collection, set `disableHeap` to true:

```js
require('@google-cloud/profiler').start({disableHeap: true});
await require('@google-cloud/profiler').start({disableHeap: true});
```

## Running on Google Cloud Platform
Expand All @@ -149,7 +151,7 @@ flexible environment, import the agent at the top of your application’s main
script or entry point by including the following code snippet:

```js
var profiler = require('@google-cloud/profiler').start();
require('@google-cloud/profiler').start();
```

You can specify which version of Node.js you're using by adding a snippet like
Expand Down Expand Up @@ -194,6 +196,37 @@ require('@google-cloud/profiler').start({
});
```

### Running on Istio

On Istio, the GCP Metadata server may not be available for a few seconds after
your application has started. When this occurs, the profiling agent may fail
to start because it cannot initialize required fields. One can retry when
starting the profiler with the following snippet.

```js
const profiler = require('@google-cloud/profiler');
async function startProfiler() {
for (let i = 0; i < 3; i++) {
try {
await profiler.start({
serviceContext: {
service: 'your-service',
version: '1.0.0',
},
});
} catch(e) {
console.log(`Failed to start profiler: ${e}`);
}

// Wait for 1 second before trying again.
await new Promise(r => setTimeout(r, 1000));
}
}
startProfiler();

```


## Running elsewhere

You can still use `@google-cloud/profiler` if your application is running
Expand All @@ -205,7 +238,7 @@ collected profiles to be associated with, and (optionally) the version of
the service when starting the profiler:

```js
require('@google-cloud/profiler').start({
await require('@google-cloud/profiler').start({
projectId: 'project-id',
serviceContext: {
service: 'your-service',
Expand Down Expand Up @@ -238,7 +271,7 @@ the service when starting the profiler:

This is how you would set `keyFilename`:
```js
require('@google-cloud/profiler').start({
await require('@google-cloud/profiler').start({
projectId: 'project-id',
serviceContext: {
service: 'your-service',
Expand All @@ -250,7 +283,7 @@ the service when starting the profiler:

This is how you would set `credentials`:
```js
require('@google-cloud/profiler').start({
await require('@google-cloud/profiler').start({
projectId: 'project-id',
serviceContext: {
service: 'your-service',
Expand Down
9 changes: 7 additions & 2 deletions ts/src/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ export interface Config extends GoogleAuthOptions {
disableSourceMaps?: boolean;
}

// Interface for an initialized config.
export interface ProfilerConfig extends GoogleAuthOptions {
// Interface for config after local initialization.
export interface LocalConfig extends GoogleAuthOptions {
apiEndpoint: string;
projectId?: string;
logLevel: number;
Expand All @@ -171,6 +171,11 @@ export interface ProfilerConfig extends GoogleAuthOptions {
disableSourceMaps: boolean;
}

// Interface for an initialized profiler config.
export interface ProfilerConfig extends LocalConfig {
projectId: string;
}

// Default values for configuration for a profiler.
export const defaultConfig = {
logLevel: 2,
Expand Down
91 changes: 48 additions & 43 deletions ts/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,13 @@ import * as gcpMetadata from 'gcp-metadata';
import {heap as heapProfiler} from 'pprof';
import * as semver from 'semver';

import {Config, defaultConfig, ProfilerConfig} from './config';
import {Config, defaultConfig, LocalConfig, ProfilerConfig} from './config';
import {createLogger} from './logger';
import {Profiler} from './profiler';

const pjson = require('../../package.json');
const serviceRegex = /^[a-z]([-a-z0-9_.]{0,253}[a-z0-9])?$/;

/**
* @return value of metadata field.
* Throws error if there is a problem accessing metadata API.
*/
async function getMetadataInstanceField(field: string): Promise<string> {
return gcpMetadata.instance(field);
}

function hasService(
config: Config
): config is {serviceContext: {service: string}} {
Expand All @@ -43,12 +35,16 @@ function hasService(
);
}

function hasProjectId(config: Config): config is {projectId: string} {
return typeof config.projectId === 'string';
}

/**
* Sets unset values in the configuration to the value retrieved from
* environment variables or specified in defaultConfig.
* Throws error if value that must be set cannot be initialized.
*/
function initConfigLocal(config: Config): ProfilerConfig {
function initConfigLocal(config: Config): LocalConfig {
const envConfig: Config = {
projectId: process.env.GCLOUD_PROJECT,
serviceContext: {
Expand Down Expand Up @@ -114,22 +110,43 @@ function initConfigLocal(config: Config): ProfilerConfig {
* metadata.
*/
async function initConfigMetadata(
config: ProfilerConfig
config: LocalConfig
): Promise<ProfilerConfig> {
if (!config.zone || !config.instance) {
const [instance, zone] = (await Promise.all([
getMetadataInstanceField('name'),
getMetadataInstanceField('zone'),
]).catch((_: Error) => {
// ignore errors, which will occur when not on GCE.
})) || [undefined, undefined];
const logger = createLogger(config.logLevel);
const getMetadataProperty = async (
f: (s: string) => Promise<string>,
field: string
) => {
try {
return await f(field);
} catch (e) {
logger.debug(`Failed to fetch ${field} from metadata: ${e}`);
}
return undefined;
};

if (!config.projectId || !config.zone || !config.instance) {
const [projectId, instance, zone] = await Promise.all([
getMetadataProperty(gcpMetadata.project, 'project-id'),
getMetadataProperty(gcpMetadata.instance, 'name'),
getMetadataProperty(gcpMetadata.instance, 'zone'),
]);

if (!config.zone && zone) {
config.zone = zone.substring(zone.lastIndexOf('/') + 1);
}
if (!config.instance && instance) {
config.instance = instance;
}
if (!config.projectId && projectId) {
config.projectId = projectId;
}
}

if (!hasProjectId(config)) {
throw new Error('Project ID must be specified in the configuration');
}

return config;
}

Expand Down Expand Up @@ -161,18 +178,24 @@ export async function createProfiler(config: Config = {}): Promise<Profiler> {
);
}

let profilerConfig = initConfigLocal(config);
const localConfig: LocalConfig = initConfigLocal(config);

// Start the heap profiler if profiler config does not indicate heap profiling
// is disabled. This must be done before any asynchronous calls are made so
// all memory allocations made after start() is called can be captured.
if (!profilerConfig.disableHeap) {
if (!localConfig.disableHeap) {
heapProfiler.start(
profilerConfig.heapIntervalBytes,
profilerConfig.heapMaxStackDepth
localConfig.heapIntervalBytes,
localConfig.heapMaxStackDepth
);
}
profilerConfig = await initConfigMetadata(profilerConfig);
let profilerConfig: ProfilerConfig;
try {
profilerConfig = await initConfigMetadata(localConfig);
} catch (err) {
heapProfiler.stop();
throw err;
}
return new Profiler(profilerConfig);
}

Expand All @@ -191,34 +214,16 @@ export async function createProfiler(config: Config = {}): Promise<Profiler> {
*
*/
export async function start(config: Config = {}): Promise<void> {
let profiler: Profiler;
try {
profiler = await createProfiler(config);
} catch (e) {
logError(`${e}`, config);
return;
}
const profiler = await createProfiler(config);
profiler.start();
}

function logError(msg: string, config: Config) {
// FIXME: do not create a new logger on each error.
const logger = createLogger(config.logLevel);
logger.error(msg);
}

/**
* For debugging purposes. Collects profiles and discards the collected
* profiles.
*/
export async function startLocal(config: Config = {}): Promise<void> {
let profiler: Profiler;
try {
profiler = await createProfiler(config);
} catch (e) {
logError(`${e}`, config);
return;
}
const profiler = await createProfiler(config);

// Set up periodic logging.
const logger = createLogger(config.logLevel);
Expand Down
39 changes: 21 additions & 18 deletions ts/src/profiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ function isDeployment(deployment: any): deployment is Deployment {
typeof deployment.projectId === 'string') &&
(deployment.target === undefined ||
typeof deployment.target === 'string') &&
(deployment.labels !== undefined &&
deployment.labels.language !== undefined &&
typeof deployment.labels.language === 'string')
deployment.labels !== undefined &&
deployment.labels.language !== undefined &&
typeof deployment.labels.language === 'string'
);
}

Expand Down Expand Up @@ -449,22 +449,25 @@ export class Profiler extends ServiceObject {

this.logger.debug(`Attempting to create profile.`);
return new Promise<RequestProfile>((resolve, reject) => {
this.request(options, (
err: Error | ApiError | null,
body?: object,
// tslint:disable-next-line: no-any
response?: r.Response<any>
) => {
try {
const prof = responseToProfileOrError(err, body, response);
this.logger.debug(
`Successfully created profile ${prof.profileType}.`
);
resolve(prof);
} catch (err) {
reject(err);
this.request(
options,
(
err: Error | ApiError | null,
body?: object,
// tslint:disable-next-line: no-any
response?: r.Response<any>
) => {
try {
const prof = responseToProfileOrError(err, body, response);
this.logger.debug(
`Successfully created profile ${prof.profileType}.`
);
resolve(prof);
} catch (err) {
reject(err);
}
}
});
);
});
}

Expand Down
10 changes: 8 additions & 2 deletions ts/test/profiles-for-tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,10 @@ const heapLeaf2 = {
scriptId: 1,
lineNumber: 10,
columnNumber: 5,
allocations: [{count: 8, sizeBytes: 10}, {count: 15, sizeBytes: 72}],
allocations: [
{count: 8, sizeBytes: 10},
{count: 15, sizeBytes: 72},
],
children: [],
};

Expand All @@ -224,7 +227,10 @@ const heapNode1 = {
scriptId: 0,
lineNumber: 1,
columnNumber: 5,
allocations: [{count: 1, sizeBytes: 5}, {count: 3, sizeBytes: 7}],
allocations: [
{count: 1, sizeBytes: 5},
{count: 3, sizeBytes: 7},
],
children: [heapNode2],
};

Expand Down
Loading

0 comments on commit 5b46b66

Please sign in to comment.