Skip to content

Commit

Permalink
Improve error handling and messaging for cloud-logging
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelDoyle committed Oct 16, 2024
1 parent 5eacd6d commit bfe5932
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 33 deletions.
1 change: 0 additions & 1 deletion js/plugins/google-cloud/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"@opentelemetry/sdk-trace-base": "^1.25.0",
"google-auth-library": "^9.6.3",
"node-fetch": "^3.3.2",
"prettier-plugin-organize-imports": "^3.2.4",
"winston": "^3.12.0"
},
"peerDependencies": {
Expand Down
42 changes: 38 additions & 4 deletions js/plugins/google-cloud/src/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,22 @@
* limitations under the License.
*/
import { logger } from 'genkit/logging';
import { GoogleAuth } from 'google-auth-library';
import { GcpTelemetryConfig } from './types';
import { auth, GoogleAuth } from 'google-auth-library';
import { GcpPrincipal, GcpTelemetryConfig } from './types';

/**
* Allow customers to pass in cloud credentials from environment variables
* following: https://github.com/googleapis/google-auth-library-nodejs?tab=readme-ov-file#loading-credentials-from-environment-variables
* Allows Google Cloud credentials to be to passed in "raw" as an environment
* variable. This is helpful in environments where the developer has limited
* ability to configure their compute environment, but has does have the ablilty
* to set environment variables.
*
* This is different from the GOOGLE_APPLICATION_CREDENTIALS used by ADC, which
* represents a path to a credential file on disk. In *most* cases, even for
* 3rd party cloud providers, developers *should* attempt to use ADC, which
* searches for credential files in standard locations, before using this
* method.
*
* See also: https://github.com/googleapis/google-auth-library-nodejs?tab=readme-ov-file#loading-credentials-from-environment-variables
*/
export async function credentialsFromEnvironment(): Promise<
Partial<GcpTelemetryConfig>
Expand Down Expand Up @@ -47,3 +57,27 @@ export async function credentialsFromEnvironment(): Promise<
}
return options;
}

/**
* Resolve the currently configured principal, either from the Genkit specific
* GCLOUD_SERVICE_ACCOUNT_CREDS environment variable, or from ADC.
*
* Since the Google Cloud Telemetry Exporter will discover credentials on its
* own, we don't immediately have access to the current principal. This method
* can be handy to get access to the current credential for logging debugging
* information or other purposes.
**/
export async function resolveCurrentPrincipal(): Promise<GcpPrincipal> {
const envCredentials = await credentialsFromEnvironment();
const adcCredentials = await auth.getCredentials();

// TODO(michaeldoyle): How to look up if the user provided credentials in the
// plugin config (i.e. GcpTelemetryOptions)
let serviceAccountEmail =
envCredentials.credentials?.client_email ?? adcCredentials.client_email;

return {
projectId: envCredentials.projectId,
serviceAccountEmail,
};
}
38 changes: 38 additions & 0 deletions js/plugins/google-cloud/src/gcpLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,16 @@
*/

import { LoggingWinston } from '@google-cloud/logging-winston';
import { logger } from 'genkit/logging';
import { Writable } from 'stream';
import { resolveCurrentPrincipal } from './auth';
import { GcpTelemetryConfig } from './types';

type GrpcError = Error & {
code?: number;
statusDetails?: Record<string, any>[];
};

/**
* Additional streams for writing log data to. Useful for unit testing.
*/
Expand Down Expand Up @@ -46,6 +53,7 @@ export class GcpLogger {
};

let transports: any[] = [];
let instructionsLogged = false; // only log the first time
transports.push(
this.shouldExport(env)
? new LoggingWinston({
Expand All @@ -54,6 +62,22 @@ export class GcpLogger {
prefix: 'genkit',
logName: 'genkit_log',
credentials: this.config.credentials,
defaultCallback: async (err) => {
const defaultLogger = logger.defaultLogger;
const grpcError = err as GrpcError;
if (err && this.loggingDenied(grpcError)) {
if (!instructionsLogged) {
instructionsLogged = true;
defaultLogger.error(
`Unable to send logs to Google Cloud: ${grpcError.message}\n\n${await this.loggingDeniedHelpMessage()}\n`
);
}
} else if (err) {
defaultLogger.error(
`Unable to send logs to Google Cloud: ${err}`
);
}
},
})
: new winston.transports.Console()
);
Expand All @@ -68,6 +92,20 @@ export class GcpLogger {
});
}

private loggingDenied(err: GrpcError) {
return (
err.code === 7 &&
err.statusDetails?.some((details) => {
return details?.metadata?.permission === 'logging.logEntries.create';
})
);
}

private async loggingDeniedHelpMessage() {
const principal = await resolveCurrentPrincipal();
return `Add the role 'roles/logging.logWriter' to your Service Account in the IAM & Admin page on the Google Cloud console, or use the following command:\n\ngcloud projects add-iam-policy-binding ${principal.projectId ?? '${PROJECT_ID}'} \\\n --member=serviceAccount:${principal.serviceAccountEmail || '${SERVICE_ACCT}'} \\\n --role=roles/logging.logWriter`;
}

private shouldExport(env?: string) {
return this.config.export;
}
Expand Down
4 changes: 4 additions & 0 deletions js/plugins/google-cloud/src/gcpOpenTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export class GcpOpenTelemetry {
spanExporter = new AdjustingTraceExporter(
this.shouldExportTraces()
? new TraceExporter({
// Creds for non-GCP environments; otherwise credentials will be
// automatically detected via ADC
credentials: this.config.credentials,
})
: new InMemorySpanExporter(),
Expand Down Expand Up @@ -157,6 +159,8 @@ export class GcpOpenTelemetry {
product: 'genkit',
version: GENKIT_VERSION,
},
// Creds for non-GCP environments; otherwise credentials will be
// automatically detected via ADC
credentials: this.config.credentials,
})
: new InMemoryMetricExporter(AggregationTemporality.DELTA);
Expand Down
5 changes: 5 additions & 0 deletions js/plugins/google-cloud/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,8 @@ export interface GcpTelemetryConfig {
exportIO: boolean;
export: boolean;
}

export interface GcpPrincipal {
projectId?: string;
serviceAccountEmail?: string;
}
31 changes: 3 additions & 28 deletions js/pnpm-lock.yaml

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

1 change: 1 addition & 0 deletions js/testapps/dev-ui-gallery/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"@genkit-ai/dev-local-vectorstore": "workspace:*",
"@genkit-ai/evaluator": "workspace:*",
"@genkit-ai/firebase": "workspace:*",
"@genkit-ai/google-cloud": "workspace:*",
"@genkit-ai/googleai": "workspace:*",
"@genkit-ai/vertexai": "workspace:*",
"firebase-admin": "^12.1.0",
Expand Down

0 comments on commit bfe5932

Please sign in to comment.