Add option to remove params from traces

[WaterWhisperer]

PR Info

• Part of [tracing-spans] Add option to remove params from traces #2941

New Features

• Add ConnectOptions::tracing_statement_logging(bool) to let users disable db.statement recording when tracing-spans is enabled.

Changes

• Propagate the statement-logging setting through DatabaseConnection and DatabaseTransaction.
• Keep the current default behavior unchanged, db.statement is still recorded unless explicitly disabled.

[WaterWhisperer]

I used expect instead of allow because of servo/servo#40383 (comment). However, to maintain consistency in the codebase, I could use allow, but personally, I might prefer expect.

[tyt2y3]

thank you! just to confirm, you used the shell script to regenerate sea-orm-sync right?

[WaterWhisperer]

thank you! just to confirm, you used the shell script to regenerate sea-orm-sync right?

Oh, sorry, I updated the sync side manually. I didn't know there was a script to use before. I’ll rerun the script and update the PR

[Huliiiiii]

#2941 wanted was to only remove the parameter. This change just disables statement recording in spans, so it doesn’t seem to fully resolve that issue.

[WaterWhisperer]

#2941 wanted was to only remove the parameter. This change just disables statement recording in spans, so it doesn’t seem to fully resolve that issue.

Yeah the current change only adds a switch for whether db.statement is recorded in spans.

After rechecking the current implementation and testing it locally, it looks like the tracing span path records stmt.sql (with placeholders), not the rendered SQL with bound parameters. The parameter injection seems to happen in Statement’s Display impl, which is used by debug logging, not by tracing-spans.

So I may be targeting a different concern than #2941 originally described. Does #2941 refer to another current path that records rendered SQL with parameters, or to an older behavior?

[Huliiiiii]

Perhaps it's because of tracing::instrument.

[WaterWhisperer]

Perhaps it's because of tracing::instrument.

Thanks! After looking into it again, I think the real concern in #2941 may be #[instrument] recording stmt: Statement, rather than db.statement itself.

If that’s the case, then this PR may be addressing a separate concern: whether raw SQL is recorded into db.statement.

Would you prefer me to keep this PR scoped as that separate option, or would it be better to rework/replace it with a focused fix for the #[instrument] related issue? If a new PR would be cleaner for review, I’m happy to close this one and open another.

[Huliiiiii]

Keep this PR and open a separate one for the tracing::instrument issue.