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

Add support for resolving bind variables in recorded SQL statements #7413

Open
ryanrupp opened this issue Dec 14, 2022 · 13 comments
Open

Add support for resolving bind variables in recorded SQL statements #7413

ryanrupp opened this issue Dec 14, 2022 · 13 comments
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request

Comments

@ryanrupp
Copy link

ryanrupp commented Dec 14, 2022

Is your feature request related to a problem? Please describe.
Currently, for PreparedStatement the recorded SQL statement will be the parameterized SQL statement that was used i.e.

select * from my_table where my_column = ?

When troubleshooting issues it could be useful to resolve the bind variables to provide more context i.e.

select * from my_table where my_column = 'example'

Benefits include:

  1. Performance examples
    a. Database engine performing bind-peeking where particular arguments can significantly affect the execution plan
    b. Data distribution that significantly affects query performance
    c. Database locking i.e. long running update my_table where id = ? vs update my_table where id = 10 - I can tell there's lock contention on id=10.
  2. Context examples
    a. Audit history for applications that do not provide this out of box i.e. in APM backend searching for all spans that updated my_table with the ID 9999 (this probably won't be exact i.e. not being able to know ID=9999 in the SQL but I currently use Elastic APM backend with a modification to analyze span.db.statement to run full text queries on it i.e. span.db.statement: "update my_table" AND span.db.statement: 9999)
    b. Understanding user behavior i.e. user is updating this entity frequently (can sometimes be gleaned from other context like API URL or something but not always)

Describe the solution you'd like
An opt-in option to resolve bind variables in the recorded SQL statement, possibly conditionally i.e. based on:

  1. If the query took more than <X>ms
  2. If the query is a write operation

Preferably these would be resolved inline in the SQL text rather than adding a separate span attribute for them although there's probably pros/cons to each approach.

This likely has performance (tracking and resolving binds, CPU/memory) as well as security and privacy concerns (capturing sensitive data) that need to be considered. At the very least this would be opt-in though under the appropriate circumstances. For example, I work with an application that:

  1. Doesn't store any PII and the minimal amount of sensitive data is either hashed or encrypted application side before registering as a bind variable.
  2. Has resources to spare i.e. the observability value add of this context outweighs the potential resource usage of this instrumentation.

Describe alternatives you've considered
Writing a custom instrumentation to handle this i.e. wrapping something like what P6Spy does

Additional context
Similar features exist in other APM agents i.e. AppDynamics and New Relic

@freshgeek
Copy link

can i use extensions to support? when replace sky agent to open-telemetry agent , I also have to face this problem

@mateuszrzeszutek
Copy link
Member

Hey @freshgeek ,
Theoretically you could implement an extension to ad support for this; it would require writing your own custom JDBC instrumentation though.

@trask
Copy link
Member

trask commented Mar 31, 2023

@freshgeek I think this could be added to the existing jdbc instrumentation under an experimental flag.

There have been discussions of adding this eventually to the specification (as an opt-in feature due to PII concerns): open-telemetry/opentelemetry-specification#3092

@freshgeek
Copy link

Thank you for your reply. Can you provide a simple idea or step? Thank you

Hey @freshgeek , Theoretically you could implement an extension to ad support for this; it would require writing your own custom JDBC instrumentation though.

Thank you for your reply. Can you provide a simple idea or step? Thank you

@freshgeek
Copy link

freshgeek commented Apr 2, 2023

this is a hack idea for Urgent need' devlepmenter:
io.opentelemetry.instrumentation.jdbc.internal.DbRequest#create(java.sql.PreparedStatement)
old code :

 @Nullable
  public static DbRequest create(PreparedStatement statement) {
    return create(statement, JdbcData.preparedStatement.get(statement));
  }

hack code in mysql jdbc 8 :


@Nullable
  public static DbRequest create(PreparedStatement statement) {
    String statementString = statement.toString();
    int i = 0;
    if ((i=statementString.indexOf(":"))>0){
      statementString = statementString.substring(i+1);
    }
    return create(statement, statementString);
  }

then , build agent , replace

@sysbes
Copy link

sysbes commented Jun 19, 2023

Any updates?

@mateuszrzeszutek
Copy link
Member

Hey @e-balashov ,
No, we don't have any new updates on that. AFAIK nobody's been working on this.

@manavgakhar
Copy link

this is a hack idea for Urgent need' devlepmenter: io.opentelemetry.instrumentation.jdbc.internal.DbRequest#create(java.sql.PreparedStatement) old code :

 @Nullable
  public static DbRequest create(PreparedStatement statement) {
    return create(statement, JdbcData.preparedStatement.get(statement));
  }

hack code in mysql jdbc 8 :


@Nullable
  public static DbRequest create(PreparedStatement statement) {
    String statementString = statement.toString();
    int i = 0;
    if ((i=statementString.indexOf(":"))>0){
      statementString = statementString.substring(i+1);
    }
    return create(statement, statementString);
  }

then , build agent , replace

@freshgeek can you tell me if this worked? i.e if the bind parameters were shown in query?

@trask trask added the contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome label Aug 15, 2023
@easayliu
Copy link

这是紧急需求开发者的一个黑客想法: io.opentelemetry.instrumentation.jdbc.internal.DbRequest#create(java.sql.PreparedStatement) 旧代码:

 @Nullable
  public static DbRequest create(PreparedStatement statement) {
    return create(statement, JdbcData.preparedStatement.get(statement));
  }

mysql jdbc 8 中的黑客代码:


@Nullable
  public static DbRequest create(PreparedStatement statement) {
    String statementString = statement.toString();
    int i = 0;
    if ((i=statementString.indexOf(":"))>0){
      statementString = statementString.substring(i+1);
    }
    return create(statement, statementString);
  }

然后,构建代理,替换

i try it at 1.30.0,it does not work.
this value looks like

com.alibaba.druid.proxy.jdbc.PreparedStatementProxyImpl@?dd8fbb

@Jamel-jun
Copy link

Is it possible to support this.
Provide the user with a switch to turn this feature off or on.

@freshgeek
Copy link

this is a hack idea for Urgent need' devlepmenter: io.opentelemetry.instrumentation.jdbc.internal.DbRequest#create(java.sql.PreparedStatement) old code :

 @Nullable
  public static DbRequest create(PreparedStatement statement) {
    return create(statement, JdbcData.preparedStatement.get(statement));
  }

hack code in mysql jdbc 8 :


@Nullable
  public static DbRequest create(PreparedStatement statement) {
    String statementString = statement.toString();
    int i = 0;
    if ((i=statementString.indexOf(":"))>0){
      statementString = statementString.substring(i+1);
    }
    return create(statement, statementString);
  }

then , build agent , replace

@freshgeek can you tell me if this worked? i.e if the bind parameters were shown in query?

Each project environment is different, and it cannot be guaranteed that all are valid. If the official does not fix it, I can only provide my feasible example reference. If you need an agent jar, can email it

@sstglobal
Copy link

sstglobal commented Jul 18, 2024

@freshgeek We got the same error when using Oracle database with Hibernate. When I used the solution that you provided, it did not work. Could you give me some suggestions or ideas to resolve this issue?

  • Spring boot 2.4.3
  • ojdbc8 12.2.0.1
  • WebLogic server 12c

@freshgeek
Copy link

@freshgeek We got the same error when using Oracle database with Hibernate. When I used the solution that you provided, it did not work. Could you give me some suggestions or ideas to resolve this issue?

  • Spring boot 2.4.3
  • ojdbc8 12.2.0.1
  • WebLogic server 12c

I don't have the equipment or experience to use Oracle, so the logic may be similar. Find the corresponding statement to extract the code, and then replace statementString for temporary support. Waiting for version updates may be slower

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants