Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

postgres statement cache behavior with traceparent comments? #5017

Open
tmm1 opened this issue Oct 11, 2024 · 3 comments
Open

postgres statement cache behavior with traceparent comments? #5017

tmm1 opened this issue Oct 11, 2024 · 3 comments

Comments

@tmm1
Copy link

tmm1 commented Oct 11, 2024

the postgres driver uses prepared statements to prevent sql injection. a new statement is created per query, and then cached for subsequent queries. this works as expected.

however, when you enable tracing it adds sql comments to your statements:

fn add_trace_id(self, trace_id: Option<&str>) -> Self {
if let Some(traceparent) = trace_id {
if should_sample(&traceparent) {
self.comment(format!("traceparent='{}'", traceparent))

wouldn't this mean the statement cache, which uses the raw sql text as the cache key, would no longer be effective?

async fn fetch_cached(&self, sql: &str, params: &[Value<'_>]) -> crate::Result<Statement> {
let statement_key = StatementKey::new(sql, params);

let mut hasher = DefaultHasher::new();
sql.hash(&mut hasher);

cc @aqrln

@tmm1
Copy link
Author

tmm1 commented Oct 11, 2024

maybe cache misses are inevitable here because these are meant to be distinct queries sent the to the server. but i wonder if there's a way to put the comment on just the EXECUTE part and not the PREPARE

@aqrln
Copy link
Member

aqrln commented Oct 11, 2024

Good question. I'm gonna pull this issue into the ongoing Tracing GA project and think about it.

@aqrln
Copy link
Member

aqrln commented Oct 11, 2024

By the way let me know if you run into any other issues related to tracing, this is super relevant right now, and we will tackle them asap. A good chunk of important bugfixes might hopefully make it into the next release, and more are coming soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants