Skip to content

Commit

Permalink
fix(pg): spec-compliant span naming
Browse files Browse the repository at this point in the history
BREAKING CHANGE. This improves the way that OTEL span names are
generated in two different ways.

First, if the query is a prepared statement with a `name`, the `name` is
put into the span name as-is. Before, the `name` was transformed first,
as though it were a full SQL query string (i.e., it was split on spaces
and only the first 'word' was used).

Second, when we do only have a full query string, and we take the first
word to form the span name, the code now calls `toUpperCase()` on that
string, so that a `select * from x` and a `SELECT * from x` query both
end up with a span name like `pg.query:SELECT`.
  • Loading branch information
ethanresnick committed Nov 26, 2022
1 parent d74d330 commit 5e6f995
Showing 1 changed file with 39 additions and 32 deletions.
71 changes: 39 additions & 32 deletions plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,34 @@ function arrayStringifyHelper(arr: Array<unknown>): string {
return '[' + arr.toString() + ']';
}

// Helper function to get a low cardinality command name from the full text query
function getCommandFromText(text?: string): string {
if (!text) return 'unknown';
const words = text.split(' ');
return words[0].length > 0 ? words[0] : 'unknown';
/**
* Helper function to get a low cardinality span name from whatever
* info we have about the query.
*
* This is tricky, because we don't have most of the information (table name,
* operation name, etc) the spec recommends using to build a low-cardinality
* value w/o parsing. So, we use db.name and assume that, if the query's a
* named prepared statement, those `name` values will be low cardinality.
* If we don't have a named prepared statement, we try to parse an operation
* (despite the spec's warnings).
*
* @params query The information we have about the query, typed to reflect
* only the validation we've actually done on the `client.query()` args.
*/
export function getQuerySpanName(
databaseName: string | undefined,
queryConfig?: { text: string; name?: unknown }
) {
if (!queryConfig) return PgInstrumentation.BASE_SPAN_NAME;

// Either the name of a prepared statement; or an attempted parse
// of the SQL command, normalized to uppercase; or unknown.
const command =
typeof queryConfig.name === 'string' && queryConfig.name
? queryConfig.name
: queryConfig.text.split(' ')[0].toUpperCase() || 'unknown';

return `${PgInstrumentation.BASE_SPAN_NAME}:${command} ${databaseName ?? ''}`;
}

function getConnectionString(params: PgClientConnectionParams) {
Expand Down Expand Up @@ -96,19 +119,6 @@ export function startSpan(
});
}

// Private helper function to start a span after a connection has been established
function startQuerySpan(
client: PgClientExtended,
tracer: Tracer,
instrumentationConfig: PgInstrumentationConfig,
name: string
) {
return startSpan(tracer, instrumentationConfig, name, {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, // required
...getSemanticAttributesFromConnection(client.connectionParameters),
});
}

// Create a span from our normalized queryConfig object,
// or return a basic span if no queryConfig was given/could be created.
export function handleConfigQuery(
Expand All @@ -117,23 +127,20 @@ export function handleConfigQuery(
instrumentationConfig: PgInstrumentationConfig,
queryConfig?: { text: string; values?: unknown; name?: unknown }
) {
// Create child span.
const { connectionParameters } = this;
const dbName = connectionParameters.database;

const spanName = getQuerySpanName(dbName, queryConfig);
const span = startSpan(tracer, instrumentationConfig, spanName, {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.POSTGRESQL, // required
...getSemanticAttributesFromConnection(connectionParameters),
});

if (!queryConfig) {
return startQuerySpan(
this,
tracer,
instrumentationConfig,
PgInstrumentation.BASE_SPAN_NAME
);
return span;
}

// Set child span name
const queryCommand = getCommandFromText(
(typeof queryConfig.name === 'string' ? queryConfig.name : undefined) ||
queryConfig.text
);
const name = `${PgInstrumentation.BASE_SPAN_NAME}:${queryCommand}`;
const span = startQuerySpan(this, tracer, instrumentationConfig, name);

// Set attributes
if (queryConfig.text) {
span.setAttribute(SemanticAttributes.DB_STATEMENT, queryConfig.text);
Expand Down

0 comments on commit 5e6f995

Please sign in to comment.