Skip to content

Commit 9c50ffd

Browse files
trasktrask
committed
Address review comment from Copilot: align mixed batch network type semconv
1 parent 49e75eb commit 9c50ffd

7 files changed

Lines changed: 98 additions & 82 deletions

File tree

‎instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static class Builder {
5656
private final Set<String> uniqueQueryTexts = new LinkedHashSet<>();
5757
private final UniqueValue uniqueQuerySummary = new UniqueValue();
5858

59-
void add(SqlQuery analyzedQuery, String queryText) {
59+
void add(SqlQuery analyzedQuery, @Nullable String queryText) {
6060
uniqueStoredProcedureName.set(analyzedQuery.getStoredProcedureName());
6161
uniqueQueryTexts.add(queryText);
6262
uniqueQuerySummary.set(analyzedQuery.getQuerySummary());

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ void shouldExtractMixedParameterizedMultiQueryBatchAttributes() {
369369
request.put("db.namespace", "potatoes");
370370
request.put(
371371
"db.query.texts",
372-
asList("INSERT INTO potato VALUES(?)", "UPDATE potato SET name='bob' WHERE id=1"));
372+
asList("INSERT INTO potato VALUES('alice', ?)", "UPDATE potato SET name='bob' WHERE id=1"));
373373
request.put("db.query.parameterized", asList(true, false));
374374
request.put(DB_OPERATION_BATCH_SIZE.getKey(), 2L);
375375

@@ -393,7 +393,7 @@ void shouldExtractMixedParameterizedMultiQueryBatchAttributes() {
393393
entry(DB_NAMESPACE, "potatoes"),
394394
entry(
395395
DB_QUERY_TEXT,
396-
"INSERT INTO potato VALUES(?); UPDATE potato SET name=? WHERE id=?"),
396+
"INSERT INTO potato VALUES('alice', ?); UPDATE potato SET name=? WHERE id=?"),
397397
entry(DB_QUERY_SUMMARY, "BATCH"),
398398
entry(DB_OPERATION_BATCH_SIZE, 2L));
399399
} else if (emitOldDatabaseSemconv()) {
@@ -404,7 +404,7 @@ void shouldExtractMixedParameterizedMultiQueryBatchAttributes() {
404404
entry(DB_NAMESPACE, "potatoes"),
405405
entry(
406406
DB_QUERY_TEXT,
407-
"INSERT INTO potato VALUES(?); UPDATE potato SET name=? WHERE id=?"),
407+
"INSERT INTO potato VALUES('alice', ?); UPDATE potato SET name=? WHERE id=?"),
408408
entry(DB_QUERY_SUMMARY, "BATCH"),
409409
entry(DB_OPERATION_BATCH_SIZE, 2L));
410410
}

‎instrumentation/cassandra/cassandra-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v3_0/CassandraRequest.java‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,49 +39,50 @@ static CassandraRequest create(Session session, Statement statement) {
3939

4040
private static CassandraRequest create(Session session, BatchStatement batchStatement) {
4141
List<String> queryTexts = new ArrayList<>();
42-
List<Boolean> parameterizedQueries = null;
43-
boolean allParameterized = true;
42+
List<Boolean> mixedParameterizedQueries = null;
43+
boolean allQueriesParameterized = true;
4444
Boolean firstParameterizedQuery = null;
4545
int queryIndex = 0;
4646
for (Statement batchEntry : batchStatement.getStatements()) {
4747
queryTexts.add(getQuery(batchEntry));
4848
boolean parameterizedQuery = batchEntry instanceof BoundStatement;
4949
if (!parameterizedQuery) {
50-
allParameterized = false;
50+
allQueriesParameterized = false;
5151
}
5252
if (firstParameterizedQuery == null) {
5353
firstParameterizedQuery = parameterizedQuery;
54-
} else if (parameterizedQuery != firstParameterizedQuery && parameterizedQueries == null) {
55-
parameterizedQueries = new ArrayList<>(batchStatement.size());
54+
} else if (parameterizedQuery != firstParameterizedQuery
55+
&& mixedParameterizedQueries == null) {
56+
mixedParameterizedQueries = new ArrayList<>(batchStatement.size());
5657
for (int previousQueryIndex = 0; previousQueryIndex < queryIndex; previousQueryIndex++) {
57-
parameterizedQueries.add(firstParameterizedQuery);
58+
mixedParameterizedQueries.add(firstParameterizedQuery);
5859
}
5960
}
60-
if (parameterizedQueries != null) {
61-
parameterizedQueries.add(parameterizedQuery);
61+
if (mixedParameterizedQueries != null) {
62+
mixedParameterizedQueries.add(parameterizedQuery);
6263
}
6364
queryIndex++;
6465
}
65-
boolean parameterizedQuery = allParameterized;
66-
if (parameterizedQueries == null && firstParameterizedQuery != null) {
67-
parameterizedQuery = firstParameterizedQuery;
66+
boolean allQueriesParameterizedResult = allQueriesParameterized;
67+
if (mixedParameterizedQueries == null && firstParameterizedQuery != null) {
68+
allQueriesParameterizedResult = firstParameterizedQuery;
6869
}
6970
return create(
7071
session,
7172
queryTexts,
72-
parameterizedQuery,
73-
parameterizedQueries,
73+
allQueriesParameterizedResult,
74+
mixedParameterizedQueries,
7475
Long.valueOf(batchStatement.size()));
7576
}
7677

7778
private static CassandraRequest create(
7879
Session session,
7980
Collection<String> queryTexts,
80-
boolean parameterizedQuery,
81-
@Nullable List<Boolean> parameterizedQueries,
81+
boolean allQueriesParameterized,
82+
@Nullable List<Boolean> mixedParameterizedQueries,
8283
@Nullable Long batchSize) {
8384
return new AutoValue_CassandraRequest(
84-
session, queryTexts, parameterizedQuery, parameterizedQueries, batchSize);
85+
session, queryTexts, allQueriesParameterized, mixedParameterizedQueries, batchSize);
8586
}
8687

8788
private static String getQuery(Statement statement) {
@@ -99,16 +100,16 @@ private static String getQuery(Statement statement) {
99100

100101
abstract Collection<String> getQueryTexts();
101102

102-
abstract boolean parameterizedQuery();
103+
abstract boolean allQueriesParameterized();
103104

104105
@Nullable
105-
abstract List<Boolean> getParameterizedQueries();
106+
abstract List<Boolean> mixedParameterizedQueries();
106107

107108
boolean isParameterizedQuery(int queryIndex) {
108-
List<Boolean> parameterizedQueries = getParameterizedQueries();
109-
return parameterizedQueries == null
110-
? parameterizedQuery()
111-
: parameterizedQueries.get(queryIndex);
109+
List<Boolean> mixedParameterizedQueries = mixedParameterizedQueries();
110+
return mixedParameterizedQueries == null
111+
? allQueriesParameterized()
112+
: mixedParameterizedQueries.get(queryIndex);
112113
}
113114

114115
@Nullable

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -355,12 +355,12 @@ void batchStatementWithDifferentQueries() {
355355
"CREATE KEYSPACE batch_mixed_test WITH REPLICATION = {'class':'SimpleStrategy', 'replication_factor':1}");
356356
session.execute("CREATE TABLE batch_mixed_test.users ( name text PRIMARY KEY, age int )");
357357
PreparedStatement insertStatement =
358-
session.prepare("INSERT INTO batch_mixed_test.users (name, age) values (?, ?)");
358+
session.prepare("INSERT INTO batch_mixed_test.users (name, age) values ('alice', ?)");
359359
testing.clearData();
360360

361361
BatchStatement batchStatement =
362362
new BatchStatement()
363-
.add(insertStatement.bind("alice", 1))
363+
.add(insertStatement.bind(1))
364364
.add(
365365
new SimpleStatement(
366366
"UPDATE batch_mixed_test.users SET age = 2 WHERE name = 'alice'"));
@@ -374,7 +374,7 @@ void batchStatementWithDifferentQueries() {
374374
.hasKind(SpanKind.CLIENT)
375375
.hasNoParent()
376376
.hasAttributesSatisfyingExactly(
377-
equalTo(NETWORK_TYPE, "ipv4"),
377+
equalTo(NETWORK_TYPE, emitStableDatabaseSemconv() ? null : "ipv4"),
378378
equalTo(SERVER_ADDRESS, cassandraHost),
379379
equalTo(SERVER_PORT, cassandraPort),
380380
equalTo(NETWORK_PEER_ADDRESS, cassandraIp),
@@ -383,7 +383,7 @@ void batchStatementWithDifferentQueries() {
383383
equalTo(
384384
maybeStable(DB_STATEMENT),
385385
emitStableDatabaseSemconv()
386-
? "INSERT INTO batch_mixed_test.users (name, age) values (?, ?); UPDATE batch_mixed_test.users SET age = ? WHERE name = ?"
386+
? "INSERT INTO batch_mixed_test.users (name, age) values ('alice', ?); UPDATE batch_mixed_test.users SET age = ? WHERE name = ?"
387387
: null),
388388
equalTo(
389389
DB_OPERATION_BATCH_SIZE, emitStableDatabaseSemconv() ? 2L : null),

‎instrumentation/cassandra/cassandra-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraRequest.java‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,49 +36,50 @@ static CassandraRequest create(Session session, Statement<?> statement) {
3636

3737
private static CassandraRequest create(Session session, BatchStatement batchStatement) {
3838
List<String> queryTexts = new ArrayList<>();
39-
List<Boolean> parameterizedQueries = null;
40-
boolean allParameterized = true;
39+
List<Boolean> mixedParameterizedQueries = null;
40+
boolean allQueriesParameterized = true;
4141
Boolean firstParameterizedQuery = null;
4242
int queryIndex = 0;
4343
for (BatchableStatement<?> batchEntry : batchStatement) {
4444
queryTexts.add(getQuery(batchEntry));
4545
boolean parameterizedQuery = batchEntry instanceof BoundStatement;
4646
if (!parameterizedQuery) {
47-
allParameterized = false;
47+
allQueriesParameterized = false;
4848
}
4949
if (firstParameterizedQuery == null) {
5050
firstParameterizedQuery = parameterizedQuery;
51-
} else if (parameterizedQuery != firstParameterizedQuery && parameterizedQueries == null) {
52-
parameterizedQueries = new ArrayList<>(batchStatement.size());
51+
} else if (parameterizedQuery != firstParameterizedQuery
52+
&& mixedParameterizedQueries == null) {
53+
mixedParameterizedQueries = new ArrayList<>(batchStatement.size());
5354
for (int previousQueryIndex = 0; previousQueryIndex < queryIndex; previousQueryIndex++) {
54-
parameterizedQueries.add(firstParameterizedQuery);
55+
mixedParameterizedQueries.add(firstParameterizedQuery);
5556
}
5657
}
57-
if (parameterizedQueries != null) {
58-
parameterizedQueries.add(parameterizedQuery);
58+
if (mixedParameterizedQueries != null) {
59+
mixedParameterizedQueries.add(parameterizedQuery);
5960
}
6061
queryIndex++;
6162
}
62-
boolean parameterizedQuery = allParameterized;
63-
if (parameterizedQueries == null && firstParameterizedQuery != null) {
64-
parameterizedQuery = firstParameterizedQuery;
63+
boolean allQueriesParameterizedResult = allQueriesParameterized;
64+
if (mixedParameterizedQueries == null && firstParameterizedQuery != null) {
65+
allQueriesParameterizedResult = firstParameterizedQuery;
6566
}
6667
return create(
6768
session,
6869
queryTexts,
69-
parameterizedQuery,
70-
parameterizedQueries,
70+
allQueriesParameterizedResult,
71+
mixedParameterizedQueries,
7172
Long.valueOf(batchStatement.size()));
7273
}
7374

7475
private static CassandraRequest create(
7576
Session session,
7677
Collection<String> queryTexts,
77-
boolean parameterizedQuery,
78-
@Nullable List<Boolean> parameterizedQueries,
78+
boolean allQueriesParameterized,
79+
@Nullable List<Boolean> mixedParameterizedQueries,
7980
@Nullable Long batchSize) {
8081
return new AutoValue_CassandraRequest(
81-
session, queryTexts, parameterizedQuery, parameterizedQueries, batchSize);
82+
session, queryTexts, allQueriesParameterized, mixedParameterizedQueries, batchSize);
8283
}
8384

8485
private static String getQuery(Statement<?> statement) {
@@ -96,16 +97,16 @@ private static String getQuery(Statement<?> statement) {
9697

9798
abstract Collection<String> getQueryTexts();
9899

99-
abstract boolean parameterizedQuery();
100+
abstract boolean allQueriesParameterized();
100101

101102
@Nullable
102-
abstract List<Boolean> getParameterizedQueries();
103+
abstract List<Boolean> mixedParameterizedQueries();
103104

104105
boolean isParameterizedQuery(int queryIndex) {
105-
List<Boolean> parameterizedQueries = getParameterizedQueries();
106-
return parameterizedQueries == null
107-
? parameterizedQuery()
108-
: parameterizedQueries.get(queryIndex);
106+
List<Boolean> mixedParameterizedQueries = mixedParameterizedQueries();
107+
return mixedParameterizedQueries == null
108+
? allQueriesParameterized()
109+
: mixedParameterizedQueries.get(queryIndex);
109110
}
110111

111112
@Nullable

‎instrumentation/cassandra/cassandra-4.4/library/src/main/java/io/opentelemetry/instrumentation/cassandra/v4_4/CassandraRequest.java‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,54 +36,56 @@ static CassandraRequest create(Session session, Statement<?> statement) {
3636
if (statement instanceof BatchStatement) {
3737
return create(session, (BatchStatement) statement);
3838
}
39-
return create(session, singleton(getQuery(statement)), statement instanceof BoundStatement, null, null);
39+
return create(
40+
session, singleton(getQuery(statement)), statement instanceof BoundStatement, null, null);
4041
}
4142

4243
private static CassandraRequest create(Session session, BatchStatement batchStatement) {
4344
List<String> queryTexts = new ArrayList<>();
44-
List<Boolean> parameterizedQueries = null;
45-
boolean allParameterized = true;
45+
List<Boolean> mixedParameterizedQueries = null;
46+
boolean allQueriesParameterized = true;
4647
Boolean firstParameterizedQuery = null;
4748
int queryIndex = 0;
4849
for (BatchableStatement<?> batchEntry : batchStatement) {
4950
queryTexts.add(getQuery(batchEntry));
5051
boolean parameterizedQuery = batchEntry instanceof BoundStatement;
5152
if (!parameterizedQuery) {
52-
allParameterized = false;
53+
allQueriesParameterized = false;
5354
}
5455
if (firstParameterizedQuery == null) {
5556
firstParameterizedQuery = parameterizedQuery;
56-
} else if (parameterizedQuery != firstParameterizedQuery && parameterizedQueries == null) {
57-
parameterizedQueries = new ArrayList<>(batchStatement.size());
57+
} else if (parameterizedQuery != firstParameterizedQuery
58+
&& mixedParameterizedQueries == null) {
59+
mixedParameterizedQueries = new ArrayList<>(batchStatement.size());
5860
for (int previousQueryIndex = 0; previousQueryIndex < queryIndex; previousQueryIndex++) {
59-
parameterizedQueries.add(firstParameterizedQuery);
61+
mixedParameterizedQueries.add(firstParameterizedQuery);
6062
}
6163
}
62-
if (parameterizedQueries != null) {
63-
parameterizedQueries.add(parameterizedQuery);
64+
if (mixedParameterizedQueries != null) {
65+
mixedParameterizedQueries.add(parameterizedQuery);
6466
}
6567
queryIndex++;
6668
}
67-
boolean parameterizedQuery = allParameterized;
68-
if (parameterizedQueries == null && firstParameterizedQuery != null) {
69-
parameterizedQuery = firstParameterizedQuery;
69+
boolean allQueriesParameterizedResult = allQueriesParameterized;
70+
if (mixedParameterizedQueries == null && firstParameterizedQuery != null) {
71+
allQueriesParameterizedResult = firstParameterizedQuery;
7072
}
7173
return create(
7274
session,
7375
queryTexts,
74-
parameterizedQuery,
75-
parameterizedQueries,
76+
allQueriesParameterizedResult,
77+
mixedParameterizedQueries,
7678
Long.valueOf(batchStatement.size()));
7779
}
7880

7981
private static CassandraRequest create(
8082
Session session,
8183
Collection<String> queryTexts,
82-
boolean parameterizedQuery,
83-
@Nullable List<Boolean> parameterizedQueries,
84+
boolean allQueriesParameterized,
85+
@Nullable List<Boolean> mixedParameterizedQueries,
8486
@Nullable Long batchSize) {
8587
return new AutoValue_CassandraRequest(
86-
session, queryTexts, parameterizedQuery, parameterizedQueries, batchSize);
88+
session, queryTexts, allQueriesParameterized, mixedParameterizedQueries, batchSize);
8789
}
8890

8991
private static String getQuery(Statement<?> statement) {
@@ -112,13 +114,17 @@ private static String getQuery(Statement<?> statement) {
112114
*/
113115
@Deprecated
114116
public String getQueryText() {
115-
return getQueryTexts().isEmpty() ? "" : getQueryTexts().iterator().next();
117+
if (getBatchSize() != null) {
118+
// Preserve previous public API behavior: BatchStatement query text was not captured.
119+
return "";
120+
}
121+
return getQueryTexts().iterator().next();
116122
}
117123

118-
abstract boolean parameterizedQuery();
124+
abstract boolean allQueriesParameterized();
119125

120126
@Nullable
121-
abstract List<Boolean> getParameterizedQueries();
127+
abstract List<Boolean> mixedParameterizedQueries();
122128

123129
/**
124130
* Returns whether all queries in this request are parameterized.
@@ -127,14 +133,14 @@ public String getQueryText() {
127133
*/
128134
@Deprecated
129135
public boolean isParameterizedQuery() {
130-
return parameterizedQuery();
136+
return allQueriesParameterized();
131137
}
132138

133139
boolean isParameterizedQuery(int queryIndex) {
134-
List<Boolean> parameterizedQueries = getParameterizedQueries();
135-
return parameterizedQueries == null
136-
? parameterizedQuery()
137-
: parameterizedQueries.get(queryIndex);
140+
List<Boolean> mixedParameterizedQueries = mixedParameterizedQueries();
141+
return mixedParameterizedQueries == null
142+
? allQueriesParameterized()
143+
: mixedParameterizedQueries.get(queryIndex);
138144
}
139145

140146
@Nullable

0 commit comments

Comments
 (0)