Update SQL span name for procedures #7557
Conversation
|
|
|
Hey @akats7 , looks like your GitHud ID is now being properly recognized. Can you sign the CLA? |
akats7
force-pushed
the
main
branch
4 times, most recently
from
8b92ae2
to
96bad86
Compare
Hi @mateuszrzeszutek, I added myself to my orgs CLA and was approved but for some reason it doesn't seem to be linking it, I opened a support ticket |
|
/easycla |
...tion-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementInfo.java
Outdated
Show resolved
Hide resolved
| // Call | ||
| Arguments.of("call test_proc()", expect("CALL", "test_proc")), | ||
| Arguments.of("call test_proc", expect("CALL", "test_proc")), | ||
| Arguments.of("call next value in hibernate_sequence", expect("CALL", null)), |
There was a problem hiding this comment.
(not sure what happens, but may be a good test also)
| Arguments.of("call next value in hibernate_sequence", expect("CALL", null)), | |
| Arguments.of("call next value in hibernate_sequence", expect("CALL", null)), | |
| Arguments.of("select next value in hibernate_sequence", expect("SELECT", null)), |
There was a problem hiding this comment.
@akats7 sorry it looks like this comment got resolved accidentally, does it make sense to add this?
There was a problem hiding this comment.
@trask Yep I did add it, its in the //Select section of the test cases
...tion-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementInfo.java
Outdated
Show resolved
Hide resolved
...tion-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementInfo.java
Outdated
Show resolved
Hide resolved
...ain/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbClientSpanNameExtractor.java
Outdated
Show resolved
Hide resolved
...tion-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/db/SqlStatementInfo.java
Outdated
Show resolved
Hide resolved
|
Hey @trask @mateuszrzeszutek, I addressed your comments, please let me know if everything looks okay and I can squash the commit |
| @@ -43,6 +43,7 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R | |||
| return new SqlClientAttributesExtractorBuilder<>(getter); | |||
| } | |||
|
|
|||
| private static final String SQL_CALL = "CALL"; | |||
There was a problem hiding this comment.
Nit: just a blank line to separate constants from fields:
| private static final String SQL_CALL = "CALL"; | |
| private static final String SQL_CALL = "CALL"; | |
.../java/io/opentelemetry/instrumentation/api/instrumenter/db/SqlClientAttributesExtractor.java
Outdated
Show resolved
Hide resolved
|
Ignore the close, I somehow accidentally clicked close PR 😄 |
| @@ -40,24 +41,24 @@ public static <REQUEST> SpanNameExtractor<REQUEST> create( | |||
|
|
|||
| private DbClientSpanNameExtractor() {} | |||
|
|
|||
| protected String computeSpanName(String dbName, String operation, String table) { | |||
| protected String computeSpanName(String dbName, String operation, String identifier) { | |||
There was a problem hiding this comment.
can you rename identifier -> mainIdentifier?
|
thx @akats7! |
This PR includes updates to the SQLSanitizer, DbClientSpanNameExtractor and SqlStatementInfo to name spans according to procedure name for CALL statements. The updates to the naming logic are in the SqlSanitizer and table has been renamed to identifier as using the table variable for the procedure name would not be idiomatic. SqlStatementInfo has been updated so that the db.sql.table attribute is not included for procedures.