Skip to content

Commit

Permalink
instrument: allow SQL annotation if SPANNER_NODEJS_ANNOTATE_PII_SQL=1…
Browse files Browse the repository at this point in the history
… is set in environment
  • Loading branch information
odeke-em committed Jul 13, 2024
1 parent 1f03437 commit ce1681f
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 5 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ Please use a tracer named "nodejs-spanner".

> :warning: **Make sure that the OpenTelemetry imports are the first, before importing the Spanner library**
> :warning: **In order for your spans to be annotated with SQL, you MUST opt-in by setting environment variable
`SPANNER_NODEJS_ANNOTATE_PII_SQL=1`, this is because SQL statements can be
sensitive personally-identifiable-information (PII).**

To test out trace examination, you can use the Zipkin tracing service like this.

```javascript
Expand Down
2 changes: 1 addition & 1 deletion src/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2700,7 +2700,7 @@ class Database extends common.GrpcServiceObject {
optionsOrCallback?: TimestampBounds | RunCallback,
cb?: RunCallback
): void | Promise<RunResponse> {
const span = startTrace('Database.run');
const span = startTrace('Database.run', query);

let stats: ResultSetStats;
let metadata: ResultSetMetadata;
Expand Down
6 changes: 3 additions & 3 deletions src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ export class Snapshot extends EventEmitter {
query: string | ExecuteSqlRequest,
callback?: RunCallback
): void | Promise<RunResponse> {
const span = startTrace('Transaction.run');
const span = startTrace('Transaction.run', query);
const rows: Rows = [];
let stats: google.spanner.v1.ResultSetStats;
let metadata: google.spanner.v1.ResultSetMetadata;
Expand Down Expand Up @@ -1138,7 +1138,7 @@ export class Snapshot extends EventEmitter {
* ```
*/
runStream(query: string | ExecuteSqlRequest): PartialResultStream {
const span = startTrace('Transaction.runStream');
const span = startTrace('Transaction.runStream', query);
if (typeof query === 'string') {
query = {sql: query} as ExecuteSqlRequest;
}
Expand Down Expand Up @@ -1542,7 +1542,7 @@ export class Dml extends Snapshot {
query: string | ExecuteSqlRequest,
callback?: RunUpdateCallback
): void | Promise<RunUpdateResponse> {
const span = startTrace('Transaction.runUpdate');
const span = startTrace('Transaction.runUpdate', query);
if (typeof query === 'string') {
query = {sql: query} as ExecuteSqlRequest;
}
Expand Down
18 changes: 17 additions & 1 deletion src/v1/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,30 @@ registerInstrumentations({
instrumentations: [new GrpcInstrumentation(), new HttpInstrumentation()],
});

var optedInPII = process.env.SPANNER_NODEJS_ANNOTATE_PII_SQL === '1';

interface SQLStatement {
sql: string;
}

// startSpan synchronously returns a span to avoid the dramatic
// scope change in which trying to use tracer.startActiveSpan
// would change the meaning of this, and also introduction of callbacks
// would radically change all the code structures making it more invasive.
export function startTrace(spanNameSuffix: String): Span {
export function startTrace(spanNameSuffix: string, sql?: string | SQLStatement): Span {
const span = tracer.startSpan(
'cloud.google.com/nodejs/spanner/' + spanNameSuffix
);

if (optedInPII && sql) {
if (typeof sql === 'string') {
span.setAttribute('sql', sql as string);
} else {
const stmt = sql as SQLStatement;
span.setAttribute('sql', stmt.sql);
}
}

// Now set the span as the active one in the current context so that
// future invocations to startTrace will have this current span as
// the parent.
Expand Down

0 comments on commit ce1681f

Please sign in to comment.