Skip to content

further SQL parsing improvement #676

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

Closed
wolframhaussig opened this issue Jun 18, 2019 · 11 comments
Closed

further SQL parsing improvement #676

wolframhaussig opened this issue Jun 18, 2019 · 11 comments
Assignees

Comments

@wolframhaussig
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We would like to further improve the Span naming for JDBC spans

  • Calling DB procedures
    • SQL "{call demoSp(?, ?)}" gets the name "{call"
    • SQL "{ call demoSp(?, ?)}" gets the name "{"
  • SQL merge
    • SQL "Merge into testTable using(select * from table2) src ..." gets the name "Merge"

Describe the solution you'd like
For the "{call demoSp(?, ?)}" I would like to get "call demoSp" as Span name.
For "Merge into testTable ..." I would like to get at least "Merge testTable", maybe it is even possible to get "Merge testTable with table2" as span name?

@eyalkoren
Copy link
Contributor

Implemented in version 1.7.0.
Did you try it out?

@wolframhaussig
Copy link
Contributor Author

Oh, sorry that was a fact I wanted to add: I am already using version 1.7.0, sorry.

@eyalkoren
Copy link
Contributor

Ahh, thanks. Will look into it then.

@eyalkoren
Copy link
Contributor

Are those all the cases you found out not working as expected, or are the additional ones?
Please try to provide the most exhaustive list of examples, this can help us out.

@eyalkoren eyalkoren reopened this Jun 18, 2019
@eyalkoren
Copy link
Contributor

🤦‍♂ Wrong button..

@wolframhaussig
Copy link
Contributor Author

We have an Oracle 12c DB running and Elastic Stack running on version 6.5.4. I upgraded today to apm-agent 1.7.0 and the following SQL namings works fine: UPDATE, SELECT, DELETE.

The following SQLs are not parsed correctly:
merge into APPLICATION_VERSION target using (select APPLICATION_ID, ? VERSION from APPLICATION where APPLICATION_NAME=? ) source on (target.APPLICATION_ID=source.APPLICATION_ID and target.VERSION=source.VERSION) when matched then update set STARTUP=sysdate, LAST_KEEP_ALIVE_SIGNAL=sysdate when not matched then insert(APPLICATION_ID,VERSION,STARTUP,FIRST_STARTUP,LAST_KEEP_ALIVE_SIGNAL) values(source.APPLICATION_ID, source.VERSION,sysdate,sysdate,sysdate)
This gets "merge" as Span name.

{ call TRACE(CONTEXT=>?, WHO=>?, WHAT=>?, TARGET=>?, TEXT=>?) }
This gets only "{" as Span name.

@felixbarny
Copy link
Member

/cc @axw

@axw
Copy link
Member

axw commented Jun 18, 2019

Thanks @wolframhaussig. I'm not personally familiar with Oracle's dialect, having { in the query is foreign to me. If you happen to know where I can find the syntax definition, that would be a great help.

MERGE doesn't look too difficult to handle.

@wolframhaussig
Copy link
Contributor Author

@wolframhaussig
Copy link
Contributor Author

The curly braces seem to be not oracle specific but JDBC specific: https://db.apache.org/derby/docs/10.0/manuals/reference/sqlj230.html
"JDBC provides a way of smoothing out some of the differences in the way different DBMS vendors implement SQL. This is called escape syntax. Escape syntax signals that the JDBC driver, which is provided by a particular vendor, scans for any escape syntax and converts it into the code that the particular database understands. This makes escape syntax DBMS-independent.

A JDBC escape clause begins and ends with curly braces. A keyword always follows the opening curly brace:

{keyword }"

@axw
Copy link
Member

axw commented Jun 19, 2019

Thanks for the pointers @wolframhaussig!

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

4 participants