Proposal: sequelize config.queryHook to aid deeper instrumentation

[drewfish]

Hi,

I noticed that opentelemetry-instrumentation-sequelize@0.25.0 is just logging the statement, which might have parameter placeholders (e.g. $1, $2, etc). I would like to add these parameters to the span attributes. An example is INSERT INTO "addresses" ("id","city","lineOne","zipCode","state","createdAt","updatedAt") VALUES ($1,$2,$3,$4,$5,$6,$7);.

As a more general mechanism, perhaps a new config.queryHook(span, sql, option) (or something like that) could be created, allowing the service author (me 🙂 ) to add span attributes based on the sql.

(If you'd like I can attempt a PR to implement this, following the pattern of config.responseHook(span, response).)

[blumamir]

Hi @drewfish , Thanks for bringing it up.
I support adding the queryHook. If you can open a PR for that I'll be happy to review and merge that in.

Regarding the function signature, the up-to-date pattern is to use a hook function with two parameters: span and params. That way we can add additional params in the future without creating long function signature and breaking changes. Something like that:

export interface QueryParams {
    // any query related data that might be interesting to capture in a hook
}

export interface SequelizeQueryCustomAttributesFunction {
    (span: Span, queryParams: QueryParams): void;
}

export interface SequelizeInstrumentationConfig extends InstrumentationConfig {
    queryHook?: SequelizeQueryCustomAttributesFunction;
}

I'll be happy to help if you encounter any issues.