Skip to content

Commit c0f9cdf

Browse files
bjrarabjrara
committed
fix missing db.operation for create/drop/alter statement
1 parent 634332b commit c0f9cdf

File tree

7 files changed

+241
-69
lines changed

7 files changed

+241
-69
lines changed

‎instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,56 @@ WHITESPACE = [ \t\r\n]+
122122
/** @return true if all statement info is gathered */
123123
boolean handleNext() {
124124
return false;
125-
}
125+
}
126+
127+
/** @return true if all statement info is gathered */
128+
boolean handleOperationTarget(String target) {
129+
return false;
130+
}
131+
132+
boolean expectingOperationTarget() {
133+
return false;
134+
}
126135

127136
SqlStatementInfo getResult(String fullStatement) {
128137
return SqlStatementInfo.create(fullStatement, getClass().getSimpleName().toUpperCase(java.util.Locale.ROOT), mainIdentifier);
129138
}
130139
}
131140

141+
private abstract class DdlOperation extends Operation {
142+
private String operationTarget = "";
143+
private boolean expectingOperationTarget = true;
144+
145+
boolean expectingOperationTarget() {
146+
return expectingOperationTarget;
147+
}
148+
149+
boolean handleOperationTarget(String target) {
150+
operationTarget = target;
151+
expectingOperationTarget = false;
152+
return false;
153+
}
154+
155+
boolean shouldHandleIdentifier() {
156+
// Return true only if the provided value corresponds to a table, as it will be used to set the attribute `db.sql.table`.
157+
return "TABLE".equals(operationTarget);
158+
}
159+
160+
boolean handleIdentifier() {
161+
if (shouldHandleIdentifier()) {
162+
mainIdentifier = readIdentifierName();
163+
}
164+
return true;
165+
}
166+
167+
SqlStatementInfo getResult(String fullStatement) {
168+
if (!"".equals(operationTarget)) {
169+
return SqlStatementInfo.create(fullStatement, getClass().getSimpleName().toUpperCase(java.util.Locale.ROOT) + " " + operationTarget, mainIdentifier);
170+
}
171+
return super.getResult(fullStatement);
172+
}
173+
}
174+
132175
private static class NoOp extends Operation {
133176
static final Operation INSTANCE = new NoOp();
134177

@@ -273,6 +316,15 @@ WHITESPACE = [ \t\r\n]+
273316
}
274317
}
275318

319+
private class Create extends DdlOperation {
320+
}
321+
322+
private class Drop extends DdlOperation {
323+
}
324+
325+
private class Alter extends DdlOperation {
326+
}
327+
276328
private SqlStatementInfo getResult() {
277329
if (builder.length() > LIMIT) {
278330
builder.delete(LIMIT, builder.length());
@@ -329,6 +381,27 @@ WHITESPACE = [ \t\r\n]+
329381
appendCurrentFragment();
330382
if (isOverLimit()) return YYEOF;
331383
}
384+
"CREATE" {
385+
if (!insideComment) {
386+
setOperation(new Create());
387+
}
388+
appendCurrentFragment();
389+
if (isOverLimit()) return YYEOF;
390+
}
391+
"DROP" {
392+
if (!insideComment) {
393+
setOperation(new Drop());
394+
}
395+
appendCurrentFragment();
396+
if (isOverLimit()) return YYEOF;
397+
}
398+
"ALTER" {
399+
if (!insideComment) {
400+
setOperation(new Alter());
401+
}
402+
appendCurrentFragment();
403+
if (isOverLimit()) return YYEOF;
404+
}
332405
"FROM" {
333406
if (!insideComment && !extractionDone) {
334407
if (operation == NoOp.INSTANCE) {
@@ -358,7 +431,62 @@ WHITESPACE = [ \t\r\n]+
358431
"NEXT" {
359432
if (!insideComment && !extractionDone) {
360433
extractionDone = operation.handleNext();
434+
}
435+
appendCurrentFragment();
436+
if (isOverLimit()) return YYEOF;
437+
}
438+
"TABLE" {
439+
if (!insideComment && !extractionDone) {
440+
if (operation.expectingOperationTarget()) {
441+
extractionDone = operation.handleOperationTarget("TABLE");
442+
} else {
443+
extractionDone = operation.handleIdentifier();
444+
}
445+
}
446+
appendCurrentFragment();
447+
if (isOverLimit()) return YYEOF;
448+
}
449+
"INDEX" {
450+
if (!insideComment && !extractionDone) {
451+
if (operation.expectingOperationTarget()) {
452+
extractionDone = operation.handleOperationTarget("INDEX");
453+
} else {
454+
extractionDone = operation.handleIdentifier();
455+
}
456+
}
457+
appendCurrentFragment();
458+
if (isOverLimit()) return YYEOF;
459+
}
460+
"DATABASE" {
461+
if (!insideComment && !extractionDone) {
462+
if (operation.expectingOperationTarget()) {
463+
extractionDone = operation.handleOperationTarget("DATABASE");
464+
} else {
465+
extractionDone = operation.handleIdentifier();
466+
}
467+
}
468+
appendCurrentFragment();
469+
if (isOverLimit()) return YYEOF;
470+
}
471+
"PROCEDURE" {
472+
if (!insideComment && !extractionDone) {
473+
if (operation.expectingOperationTarget()) {
474+
extractionDone = operation.handleOperationTarget("PROCEDURE");
475+
} else {
476+
extractionDone = operation.handleIdentifier();
477+
}
478+
}
479+
appendCurrentFragment();
480+
if (isOverLimit()) return YYEOF;
481+
}
482+
"VIEW" {
483+
if (!insideComment && !extractionDone) {
484+
if (operation.expectingOperationTarget()) {
485+
extractionDone = operation.handleOperationTarget("VIEW");
486+
} else {
487+
extractionDone = operation.handleIdentifier();
361488
}
489+
}
362490
appendCurrentFragment();
363491
if (isOverLimit()) return YYEOF;
364492
}

‎instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,15 @@ void veryLongSelectStatementsAreOk() {
5858
assertThat(result).isEqualTo(expected);
5959
}
6060

61+
@ParameterizedTest
62+
@ArgumentsSource(DdlArgs.class)
63+
void checkDdlOperationStatementsAreOk(
64+
String actual, Function<String, SqlStatementInfo> expectFunc) {
65+
SqlStatementInfo result = SqlStatementSanitizer.create(true).sanitize(actual);
66+
String expected = expectFunc.apply(actual).getFullStatement();
67+
assertThat(result.getFullStatement()).isEqualTo(expected);
68+
}
69+
6170
@Test
6271
void lotsOfTicksDontCauseStackOverflowOrLongRuntimes() {
6372
String s = "'";
@@ -329,4 +338,35 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
329338
Arguments.of(null, expect(null, null)));
330339
}
331340
}
341+
342+
static class DdlArgs implements ArgumentsProvider {
343+
344+
static Function<String, SqlStatementInfo> expect(String operation, String identifier) {
345+
return sql -> SqlStatementInfo.create(sql, operation, identifier);
346+
}
347+
348+
static Function<String, SqlStatementInfo> expect(
349+
String sql, String operation, String identifier) {
350+
return ignored -> SqlStatementInfo.create(sql, operation, identifier);
351+
}
352+
353+
@Override
354+
public Stream<? extends Arguments> provideArguments(ExtensionContext context) throws Exception {
355+
return Stream.of(
356+
// Select
357+
Arguments.of("CREATE TABLE `table`", expect("CREATE TABLE", "table")),
358+
Arguments.of("CREATE TABLE IF NOT EXISTS table", expect("CREATE TABLE", "table")),
359+
Arguments.of("DROP TABLE anotherTable", expect("DROP TABLE", "anotherTable")),
360+
Arguments.of(
361+
"ALTER TABLE table ADD CONSTRAINT c FOREIGN KEY (foreign_id) REFERENCES ref (id)",
362+
expect("ALTER TABLE", "table")),
363+
Arguments.of(
364+
"CREATE UNIQUE INDEX types_name ON types (name)", expect("CREATE INDEX", null)),
365+
Arguments.of("DROP INDEX types_name ON types (name)", expect("DROP INDEX", null)),
366+
Arguments.of(
367+
"CREATE VIEW tmp AS SELECT type FROM table WHERE id = ?", expect("DROP INDEX", null)),
368+
Arguments.of(
369+
"CREATE PROCEDURE p AS SELECT * FROM table GO", expect("CREATE PROCEDURE", null)));
370+
}
371+
}
332372
}

‎instrumentation/cassandra/cassandra-3.0/javaagent/src/test/java/CassandraClientTest.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,8 @@ private static Stream<Arguments> provideSyncParameters() {
213213
null,
214214
"DROP KEYSPACE IF EXISTS sync_test",
215215
"DROP KEYSPACE IF EXISTS sync_test",
216-
"DB Query",
217-
null,
216+
"DROP",
217+
"DROP",
218218
null))),
219219
Arguments.of(
220220
named(
@@ -223,8 +223,8 @@ private static Stream<Arguments> provideSyncParameters() {
223223
null,
224224
"CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
225225
"CREATE KEYSPACE sync_test WITH REPLICATION = {?:?, ?:?}",
226-
"DB Query",
227-
null,
226+
"CREATE",
227+
"CREATE",
228228
null))),
229229
Arguments.of(
230230
named(
@@ -233,9 +233,9 @@ private static Stream<Arguments> provideSyncParameters() {
233233
"sync_test",
234234
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
235235
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
236-
"sync_test",
237-
null,
238-
null))),
236+
"CREATE TABLE sync_test.users",
237+
"CREATE TABLE",
238+
"sync_test.users"))),
239239
Arguments.of(
240240
named(
241241
"Insert data",
@@ -267,8 +267,8 @@ private static Stream<Arguments> provideAsyncParameters() {
267267
null,
268268
"DROP KEYSPACE IF EXISTS async_test",
269269
"DROP KEYSPACE IF EXISTS async_test",
270-
"DB Query",
271-
null,
270+
"DROP",
271+
"DROP",
272272
null))),
273273
Arguments.of(
274274
named(
@@ -277,8 +277,8 @@ private static Stream<Arguments> provideAsyncParameters() {
277277
null,
278278
"CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
279279
"CREATE KEYSPACE async_test WITH REPLICATION = {?:?, ?:?}",
280-
"DB Query",
281-
null,
280+
"CREATE",
281+
"CREATE",
282282
null))),
283283
Arguments.of(
284284
named(
@@ -287,9 +287,9 @@ private static Stream<Arguments> provideAsyncParameters() {
287287
"async_test",
288288
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
289289
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
290-
"async_test",
291-
null,
292-
null))),
290+
"CREATE TABLE async_test.users",
291+
"CREATE TABLE",
292+
"async_test.users"))),
293293
Arguments.of(
294294
named(
295295
"Insert data",

‎instrumentation/cassandra/cassandra-4-common/testing/src/main/java/io/opentelemetry/cassandra/v4/common/AbstractCassandraTest.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ private static Stream<Arguments> provideSyncParameters() {
172172
null,
173173
"DROP KEYSPACE IF EXISTS sync_test",
174174
"DROP KEYSPACE IF EXISTS sync_test",
175-
"DB Query",
176-
null,
175+
"DROP",
176+
"DROP",
177177
null))),
178178
Arguments.of(
179179
named(
@@ -182,8 +182,8 @@ private static Stream<Arguments> provideSyncParameters() {
182182
null,
183183
"CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
184184
"CREATE KEYSPACE sync_test WITH REPLICATION = {?:?, ?:?}",
185-
"DB Query",
186-
null,
185+
"CREATE",
186+
"CREATE",
187187
null))),
188188
Arguments.of(
189189
named(
@@ -192,9 +192,9 @@ private static Stream<Arguments> provideSyncParameters() {
192192
"sync_test",
193193
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
194194
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
195-
"sync_test",
196-
null,
197-
null))),
195+
"CREATE TABLE sync_test.users",
196+
"CREATE TABLE",
197+
"sync_test.users"))),
198198
Arguments.of(
199199
named(
200200
"Insert data",
@@ -226,8 +226,8 @@ private static Stream<Arguments> provideAsyncParameters() {
226226
null,
227227
"DROP KEYSPACE IF EXISTS async_test",
228228
"DROP KEYSPACE IF EXISTS async_test",
229-
"DB Query",
230-
null,
229+
"DROP",
230+
"DROP",
231231
null))),
232232
Arguments.of(
233233
named(
@@ -236,8 +236,8 @@ private static Stream<Arguments> provideAsyncParameters() {
236236
null,
237237
"CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
238238
"CREATE KEYSPACE async_test WITH REPLICATION = {?:?, ?:?}",
239-
"DB Query",
240-
null,
239+
"CREATE",
240+
"CREATE",
241241
null))),
242242
Arguments.of(
243243
named(
@@ -246,9 +246,9 @@ private static Stream<Arguments> provideAsyncParameters() {
246246
"async_test",
247247
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
248248
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
249-
"async_test",
250-
null,
251-
null))),
249+
"CREATE TABLE async_test.users",
250+
"CREATE TABLE",
251+
"async_test.users"))),
252252
Arguments.of(
253253
named(
254254
"Insert data",

‎instrumentation/cassandra/cassandra-4.4/testing/src/main/java/io/opentelemetry/testing/cassandra/v4_4/AbstractCassandra44Test.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ private static Stream<Arguments> provideReactiveParameters() {
9191
null,
9292
"DROP KEYSPACE IF EXISTS reactive_test",
9393
"DROP KEYSPACE IF EXISTS reactive_test",
94-
"DB Query",
95-
null,
94+
"DROP",
95+
"DROP",
9696
null))),
9797
Arguments.of(
9898
named(
@@ -101,8 +101,8 @@ private static Stream<Arguments> provideReactiveParameters() {
101101
null,
102102
"CREATE KEYSPACE reactive_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
103103
"CREATE KEYSPACE reactive_test WITH REPLICATION = {?:?, ?:?}",
104-
"DB Query",
105-
null,
104+
"CREATE",
105+
"CREATE",
106106
null))),
107107
Arguments.of(
108108
named(
@@ -111,9 +111,9 @@ private static Stream<Arguments> provideReactiveParameters() {
111111
"reactive_test",
112112
"CREATE TABLE reactive_test.users ( id UUID PRIMARY KEY, name text )",
113113
"CREATE TABLE reactive_test.users ( id UUID PRIMARY KEY, name text )",
114-
"reactive_test",
115-
null,
116-
null))),
114+
"CREATE TABLE reactive_test.users",
115+
"CREATE TABLE",
116+
"reactive_test.users"))),
117117
Arguments.of(
118118
named(
119119
"Insert data",

0 commit comments

Comments
 (0)