Skip to content

Commit 11e7b49

Browse files
trasktrask
committed
Fix span name fallback when SQL operation cannot be parsed
When SQL query text cannot be parsed (e.g., invalid SQL or non-SQL text), the span name was incorrectly falling back to server.address instead of the default 'DB Query' name. According to semconv, server.address should only be used as part of the target when combined with an operation (e.g., 'SELECT localhost'). When there is no operation, the fallback should be db.system.name or the default span name. This change ensures that server.address is only used when we have a valid operation to combine it with, preventing span names like 'localhost' when the SQL cannot be parsed. Fixes failing tests in: - :instrumentation:jdbc:javaagent:testStableSemconv - :instrumentation:jdbc:library:testStableSemconv - :instrumentation:cassandra:cassandra-3.0:javaagent:testStableSemconv
1 parent 2e1a78a commit 11e7b49

File tree

5 files changed

+490
-2
lines changed

5 files changed

+490
-2
lines changed

‎CI-PLAN.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# CI Failure Analysis Plan
2+
3+
## Failed Jobs Summary
4+
- Job 1: common / test1 (multiple Java versions) (job IDs: 57490440498, 57490440505, 57490440450, 57490440447, 57490440473, etc.)
5+
- Job 2: common / test2 (multiple Java versions) (job IDs: 57490440520, 57490440521, 57490440540, 57490440566, etc.)
6+
- Job 3: test-latest-deps / testLatestDeps1 (job ID: 57490440183)
7+
- Job 4: test-latest-deps / testLatestDeps2 (job ID: 57490440203)
8+
9+
Note: All jobs with parameters in parentheses (e.g., Java version, VM, indy settings) are variations of the same base jobs.
10+
11+
## Unique Failed Gradle Tasks
12+
**Note**: Spotless tasks are excluded from this analysis as they are formatting-only checks.
13+
14+
- [x] Task: `:instrumentation:jdbc:javaagent:testStableSemconv`
15+
- Seen in: test1 (all Java versions), testLatestDeps1
16+
- Log files: /tmp/test1-java17-indy-false.log, /tmp/testLatestDeps1.log
17+
- Error: `Expected span to have name <DB Query> but was <localhost>`
18+
- Test: `JdbcInstrumentationTest > test getClientInfo exception`
19+
- Location: AbstractJdbcInstrumentationTest.java:1223
20+
- Fix: Updated DbClientSpanNameExtractor to not use server address alone as span name
21+
22+
- [x] Task: `:instrumentation:jdbc:library:testStableSemconv`
23+
- Seen in: test2 (all Java versions)
24+
- Log files: /tmp/test2-java25-indy-false.log
25+
- Error: `Expected span to have name <DB Query> but was <localhost>`
26+
- Test: `JdbcInstrumentationTest > test getClientInfo exception`
27+
- Location: AbstractJdbcInstrumentationTest.java:1223
28+
- Fix: Same as above
29+
30+
- [x] Task: `:instrumentation:cassandra:cassandra-3.0:javaagent:testStableSemconv`
31+
- Seen in: test1 (all Java versions), testLatestDeps1
32+
- Log files: /tmp/test1-java17-indy-false.log, /tmp/testLatestDeps1.log
33+
- Fix: Same as above
34+
35+
## Notes
36+
The failures are all related to the db.query.summary implementation. The span name is being set to "localhost" instead of "DB Query". This is happening in test cases that involve getClientInfo exceptions.
37+
38+
The issue appears to be that when there's no actual SQL statement (e.g., during getClientInfo operations that fail), the query summary logic is returning the server name ("localhost") as the span name instead of falling back to "DB Query".
39+
40+
Root cause: The implementation needs to handle cases where there is no SQL statement and fall back to an appropriate default span name.

‎DB_QUERY_SUMMARY_PROGRESS.md‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# db.query.summary Implementation Progress
2+
3+
## Overview
4+
Implementing `db.query.summary` attribute from OpenTelemetry semantic conventions.
5+
- Format: `{operation} {target}` (e.g., "SELECT users", "INSERT orders")
6+
- Max length: 255 characters
7+
- Span name in stable semconv should be `db.query.summary`
8+
9+
## Completed Work
10+
11+
### 1. SqlStatementInfo.java
12+
**File:** `instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementInfo.java`
13+
14+
Added:
15+
- `querySummary` field to AutoValue class
16+
- `getQuerySummary()` method
17+
- `truncateQuerySummary()` helper (max 255 chars, truncates at word boundary)
18+
- Updated `create()` factory method signature
19+
20+
### 2. SqlSanitizer.jflex
21+
**File:** `instrumentation-api-incubator/src/jflex/SqlSanitizer.jflex`
22+
23+
Added:
24+
- `querySummaryBuilder` StringBuilder field
25+
- `appendOperationToSummary()` - appends operation (SELECT, INSERT, etc.)
26+
- `appendTargetToSummary()` - appends table name after operation
27+
- Integration in lexer rules to build query summary during parsing
28+
29+
**Note:** After editing, regenerate with: `./gradlew :instrumentation-api-incubator:generateJflex`
30+
31+
### 3. SqlClientAttributesExtractor.java
32+
**File:** `instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java`
33+
34+
Added:
35+
- Import for `DB_QUERY_SUMMARY`
36+
- Sets `db.query.summary` attribute from `SqlStatementInfo.getQuerySummary()`
37+
38+
### 4. MultiQuery.java
39+
**File:** `instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java`
40+
41+
Added:
42+
- `querySummary` tracking for multi-statement queries
43+
- `getQuerySummary()` method
44+
45+
### 5. DbClientSpanNameExtractor.java
46+
**File:** `instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java`
47+
48+
Updated `SqlClientSpanNameExtractor.extract()`:
49+
- In stable semconv mode: returns `querySummary` directly as span name
50+
- In old semconv mode: uses existing `computeSpanName(namespace, operation, mainIdentifier)`
51+
52+
### 6. AbstractJdbcInstrumentationTest.java
53+
**File:** `instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java`
54+
55+
Added:
56+
- Import: `import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_SUMMARY;`
57+
- Assertion for `DB_QUERY_SUMMARY` attribute in `testBasicStatement`
58+
- Helper method: `querySummary(String table)` - returns `"SELECT"` or `"SELECT " + table`
59+
- Updated span name assertion for stable semconv in `testBasicStatement`
60+
61+
## Test Status
62+
63+
### Passing
64+
- ✅ `./gradlew :instrumentation:jdbc:javaagent:test` (old semconv) - BUILD SUCCESSFUL
65+
- ✅ `./gradlew :instrumentation:jdbc:javaagent:testStableSemconv` (stable semconv) - BUILD SUCCESSFUL (107 tests)
66+
67+
All JDBC tests now pass for both old and stable semconv modes.
68+
69+
## Completed Test Fixes
70+
71+
Tests updated to use conditional span names and `DB_QUERY_SUMMARY` assertions:
72+
73+
1. `testBasicStatement` - Initial implementation
74+
2. `testConnectionConstructorThrowing`
75+
3. `testStatementUpdate`
76+
4. `testPreparedStatementQuery`
77+
5. `testProxyStatement`
78+
6. `testProxyPreparedStatement`
79+
7. `testCommitTransaction`
80+
8. `testPreparedStatementWrapper`
81+
9. `testProduceProperSpanName`
82+
10. `testPreparedStatementExecute`
83+
84+
**Pattern applied:**
85+
```java
86+
// Span name conditional
87+
span.hasName(emitStableDatabaseSemconv() ? querySummary(operation, table) : oldSpanName)
88+
89+
// DB_QUERY_SUMMARY attribute assertion
90+
equalTo(DB_QUERY_SUMMARY, emitStableDatabaseSemconv() ? querySummary(operation, table) : null)
91+
```
92+
93+
## Previous Issue (Resolved)
94+
95+
Tests were failing because span names in stable semconv mode should match `db.query.summary` format:
96+
- Old format: `{operation} {namespace}.{table}` (e.g., "SELECT jdbcunittest")
97+
- New format: `{operation} {table}` or just `{operation}` (e.g., "SELECT" or "SELECT users")
98+
99+
## Next Steps
100+
101+
1. **Run other instrumentation testStableSemconv tests** - Check if similar fixes are needed for:
102+
- R2DBC instrumentation
103+
- Other database instrumentations
104+
105+
## Key Files Reference
106+
107+
| File | Purpose |
108+
|------|---------|
109+
| `instrumentation-api-incubator/src/jflex/SqlSanitizer.jflex` | JFlex lexer source |
110+
| `instrumentation-api-incubator/build/generated/sources/jflex/.../AutoSqlSanitizer.java` | Generated lexer |
111+
| `instrumentation-api-incubator/.../db/SqlStatementInfo.java` | Parsed SQL data class |
112+
| `instrumentation-api-incubator/.../db/SqlClientAttributesExtractor.java` | Sets DB attributes |
113+
| `instrumentation-api-incubator/.../db/DbClientSpanNameExtractor.java` | Generates span names |
114+
| `instrumentation/jdbc/testing/.../AbstractJdbcInstrumentationTest.java` | JDBC tests |
115+
116+
## Commands
117+
118+
```bash
119+
# Regenerate JFlex lexer
120+
./gradlew :instrumentation-api-incubator:generateJflex
121+
122+
# Run old semconv tests
123+
./gradlew :instrumentation:jdbc:javaagent:test
124+
125+
# Run stable semconv tests
126+
./gradlew :instrumentation:jdbc:javaagent:testStableSemconv
127+
128+
# Run specific test
129+
./gradlew :instrumentation:jdbc:javaagent:testStableSemconv --tests "*.testBasicStatement"
130+
```
131+
132+
## Semantic Convention Reference
133+
- `db.query.summary`: A low-cardinality string describing the performed operation
134+
- Format: `{operation}` or `{operation} {target}`
135+
- Max length: 255 characters
136+
- Used as span name in stable semconv mode

0 commit comments

Comments
 (0)