Skip to content

Commit 117512b

Browse files
committed
fix missing db.operation for create/drop/alter statement
1 parent 89514b8 commit 117512b

File tree

7 files changed

+204
-70
lines changed

7 files changed

+204
-70
lines changed

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

+91-2
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) {
@@ -357,11 +430,27 @@ WHITESPACE = [ \t\r\n]+
357430
}
358431
"NEXT" {
359432
if (!insideComment && !extractionDone) {
360-
extractionDone = operation.handleNext();
433+
extractionDone = operation.handleNext();
434+
}
435+
appendCurrentFragment();
436+
if (isOverLimit()) return YYEOF;
437+
}
438+
"IF" | "NOT" | "EXISTS" {
439+
appendCurrentFragment();
440+
if (isOverLimit()) return YYEOF;
441+
}
442+
"TABLE" | "INDEX" | "DATABASE" | "PROCEDURE" | "VIEW" {
443+
if (!insideComment && !extractionDone) {
444+
if (operation.expectingOperationTarget()) {
445+
extractionDone = operation.handleOperationTarget(yytext());
446+
} else {
447+
extractionDone = operation.handleIdentifier();
361448
}
449+
}
362450
appendCurrentFragment();
363451
if (isOverLimit()) return YYEOF;
364452
}
453+
365454
{COMMA} {
366455
if (!insideComment && !extractionDone) {
367456
extractionDone = operation.handleComma();

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

+41
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,17 @@ void veryLongSelectStatementsAreOk() {
6060
assertThat(result).isEqualTo(expected);
6161
}
6262

63+
@ParameterizedTest
64+
@ArgumentsSource(DdlArgs.class)
65+
void checkDdlOperationStatementsAreOk(
66+
String actual, Function<String, SqlStatementInfo> expectFunc) {
67+
SqlStatementInfo result = SqlStatementSanitizer.create(true).sanitize(actual);
68+
SqlStatementInfo expected = expectFunc.apply(actual);
69+
assertThat(result.getFullStatement()).isEqualTo(expected.getFullStatement());
70+
assertThat(result.getOperation()).isEqualTo(expected.getOperation());
71+
assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier());
72+
}
73+
6374
@Test
6475
void lotsOfTicksDontCauseStackOverflowOrLongRuntimes() {
6576
String s = "'";
@@ -331,4 +342,34 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) th
331342
Arguments.of(null, expect(null, null)));
332343
}
333344
}
345+
346+
static class DdlArgs implements ArgumentsProvider {
347+
348+
static Function<String, SqlStatementInfo> expect(String operation, String identifier) {
349+
return sql -> SqlStatementInfo.create(sql, operation, identifier);
350+
}
351+
352+
static Function<String, SqlStatementInfo> expect(
353+
String sql, String operation, String identifier) {
354+
return ignored -> SqlStatementInfo.create(sql, operation, identifier);
355+
}
356+
357+
@Override
358+
public Stream<? extends Arguments> provideArguments(ExtensionContext context) throws Exception {
359+
return Stream.of(
360+
Arguments.of("CREATE TABLE `table`", expect("CREATE TABLE", "table")),
361+
Arguments.of("CREATE TABLE IF NOT EXISTS table", expect("CREATE TABLE", "table")),
362+
Arguments.of("DROP TABLE `if`", expect("DROP TABLE", "if")),
363+
Arguments.of(
364+
"ALTER TABLE table ADD CONSTRAINT c FOREIGN KEY (foreign_id) REFERENCES ref (id)",
365+
expect("ALTER TABLE", "table")),
366+
Arguments.of("CREATE INDEX types_name ON types (name)", expect("CREATE INDEX", null)),
367+
Arguments.of("DROP INDEX types_name ON types (name)", expect("DROP INDEX", null)),
368+
Arguments.of(
369+
"CREATE VIEW tmp AS SELECT type FROM table WHERE id = ?",
370+
expect("CREATE VIEW", null)),
371+
Arguments.of(
372+
"CREATE PROCEDURE p AS SELECT * FROM table GO", expect("CREATE PROCEDURE", null)));
373+
}
374+
}
334375
}

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

+14-14
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,8 @@ private static Stream<Arguments> provideSyncParameters() {
225225
null,
226226
"DROP KEYSPACE IF EXISTS sync_test",
227227
"DROP KEYSPACE IF EXISTS sync_test",
228-
"DB Query",
229-
null,
228+
"DROP",
229+
"DROP",
230230
null))),
231231
Arguments.of(
232232
named(
@@ -235,8 +235,8 @@ private static Stream<Arguments> provideSyncParameters() {
235235
null,
236236
"CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
237237
"CREATE KEYSPACE sync_test WITH REPLICATION = {?:?, ?:?}",
238-
"DB Query",
239-
null,
238+
"CREATE",
239+
"CREATE",
240240
null))),
241241
Arguments.of(
242242
named(
@@ -245,9 +245,9 @@ private static Stream<Arguments> provideSyncParameters() {
245245
"sync_test",
246246
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
247247
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
248-
"sync_test",
249-
null,
250-
null))),
248+
"CREATE TABLE sync_test.users",
249+
"CREATE TABLE",
250+
"sync_test.users"))),
251251
Arguments.of(
252252
named(
253253
"Insert data",
@@ -279,8 +279,8 @@ private static Stream<Arguments> provideAsyncParameters() {
279279
null,
280280
"DROP KEYSPACE IF EXISTS async_test",
281281
"DROP KEYSPACE IF EXISTS async_test",
282-
"DB Query",
283-
null,
282+
"DROP",
283+
"DROP",
284284
null))),
285285
Arguments.of(
286286
named(
@@ -289,8 +289,8 @@ private static Stream<Arguments> provideAsyncParameters() {
289289
null,
290290
"CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
291291
"CREATE KEYSPACE async_test WITH REPLICATION = {?:?, ?:?}",
292-
"DB Query",
293-
null,
292+
"CREATE",
293+
"CREATE",
294294
null))),
295295
Arguments.of(
296296
named(
@@ -299,9 +299,9 @@ private static Stream<Arguments> provideAsyncParameters() {
299299
"async_test",
300300
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
301301
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
302-
"async_test",
303-
null,
304-
null))),
302+
"CREATE TABLE async_test.users",
303+
"CREATE TABLE",
304+
"async_test.users"))),
305305
Arguments.of(
306306
named(
307307
"Insert data",

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

+14-14
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ private static Stream<Arguments> provideSyncParameters() {
200200
null,
201201
"DROP KEYSPACE IF EXISTS sync_test",
202202
"DROP KEYSPACE IF EXISTS sync_test",
203-
"DB Query",
204-
null,
203+
"DROP",
204+
"DROP",
205205
null))),
206206
Arguments.of(
207207
named(
@@ -210,8 +210,8 @@ private static Stream<Arguments> provideSyncParameters() {
210210
null,
211211
"CREATE KEYSPACE sync_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
212212
"CREATE KEYSPACE sync_test WITH REPLICATION = {?:?, ?:?}",
213-
"DB Query",
214-
null,
213+
"CREATE",
214+
"CREATE",
215215
null))),
216216
Arguments.of(
217217
named(
@@ -220,9 +220,9 @@ private static Stream<Arguments> provideSyncParameters() {
220220
"sync_test",
221221
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
222222
"CREATE TABLE sync_test.users ( id UUID PRIMARY KEY, name text )",
223-
"sync_test",
224-
null,
225-
null))),
223+
"CREATE TABLE sync_test.users",
224+
"CREATE TABLE",
225+
"sync_test.users"))),
226226
Arguments.of(
227227
named(
228228
"Insert data",
@@ -254,8 +254,8 @@ private static Stream<Arguments> provideAsyncParameters() {
254254
null,
255255
"DROP KEYSPACE IF EXISTS async_test",
256256
"DROP KEYSPACE IF EXISTS async_test",
257-
"DB Query",
258-
null,
257+
"DROP",
258+
"DROP",
259259
null))),
260260
Arguments.of(
261261
named(
@@ -264,8 +264,8 @@ private static Stream<Arguments> provideAsyncParameters() {
264264
null,
265265
"CREATE KEYSPACE async_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
266266
"CREATE KEYSPACE async_test WITH REPLICATION = {?:?, ?:?}",
267-
"DB Query",
268-
null,
267+
"CREATE",
268+
"CREATE",
269269
null))),
270270
Arguments.of(
271271
named(
@@ -274,9 +274,9 @@ private static Stream<Arguments> provideAsyncParameters() {
274274
"async_test",
275275
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
276276
"CREATE TABLE async_test.users ( id UUID PRIMARY KEY, name text )",
277-
"async_test",
278-
null,
279-
null))),
277+
"CREATE TABLE async_test.users",
278+
"CREATE TABLE",
279+
"async_test.users"))),
280280
Arguments.of(
281281
named(
282282
"Insert data",

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

+7-7
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ private static Stream<Arguments> provideReactiveParameters() {
106106
null,
107107
"DROP KEYSPACE IF EXISTS reactive_test",
108108
"DROP KEYSPACE IF EXISTS reactive_test",
109-
"DB Query",
110-
null,
109+
"DROP",
110+
"DROP",
111111
null))),
112112
Arguments.of(
113113
named(
@@ -116,8 +116,8 @@ private static Stream<Arguments> provideReactiveParameters() {
116116
null,
117117
"CREATE KEYSPACE reactive_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':3}",
118118
"CREATE KEYSPACE reactive_test WITH REPLICATION = {?:?, ?:?}",
119-
"DB Query",
120-
null,
119+
"CREATE",
120+
"CREATE",
121121
null))),
122122
Arguments.of(
123123
named(
@@ -126,9 +126,9 @@ private static Stream<Arguments> provideReactiveParameters() {
126126
"reactive_test",
127127
"CREATE TABLE reactive_test.users ( id UUID PRIMARY KEY, name text )",
128128
"CREATE TABLE reactive_test.users ( id UUID PRIMARY KEY, name text )",
129-
"reactive_test",
130-
null,
131-
null))),
129+
"CREATE TABLE reactive_test.users",
130+
"CREATE TABLE",
131+
"reactive_test.users"))),
132132
Arguments.of(
133133
named(
134134
"Insert data",

0 commit comments

Comments
 (0)