apache / apache/shardingsphere

Agent OpenTelemetry loses execute spans when JDBC callbacks overlap

Open
#39,765 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
20.8k
Forks
6.9k
Avg merge
11h 38m
Merged PRs (30d)
326

Description

## Bug Report

### Which version of ShardingSphere did you use?

Current `master`, commit `bd48f5204c8bbfcb87f7e52fd8d39f8457946520` (`5.5.4-SNAPSHOT`). The deterministic regression uses OpenTelemetry SDK 1.58.0 and Java 17.0.15.

### Which project did you use? ShardingSphere-JDBC or ShardingSphere-Proxy?

The ShardingSphere Agent OpenTelemetry plugin's JDBC executor callback advice. I investigated this while checking physical SQL execution visibility for a ShardingSphere-JDBC routing-contract project. The current-master reproduction is an advice-level unit test with mocked JDBC metadata; it does not require a database or collector.

### Expected behavior

Two overlapping per-unit `execute` invocations on the same JDBC executor callback should produce two distinct completed `/ShardingSphere/executeSQL/` spans. Each span should preserve its own SQL attributes and success/error outcome. An exception in one invocation must not change or disappear into the other invocation's span.

### Actual behavior

A deterministic two-thread test starts invocation A, starts invocation B, completes A successfully, then completes B with an exception. Only one span is exported: B's span is ended by A's completion hook. A's span is never ended, and B's error is not recorded because its span has already ended.

The test fails on unchanged current-master production code with:

```text
OpenTelemetryJDBCExecutorCallbackAdviceTest.assertConcurrentInvocations
Expected: is <2>
but: was <1>
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
```

### Reason analyze (If you can)

`OpenTelemetryJDBCExecutorCallbackAdvice.recordExecuteInfo` stores each started span in `TargetAdviceObject.setAttachment`. That attachment is a single field on the callback object. `afterMethod` and `onThrowing` read the same field, so overlapping invocations overwrite each other's span.

The invocation sequence is:

```text
A.before: attachment = spanA
B.before: attachment = spanB
A.after: end(spanB) with OK
B.throw: record error on already-ended spanB
B.after: end already-ended spanB again
```

`InstanceMethodAdviceExecutor` also invokes `afterMethod` in `finally` after `onThrowing`; a fix needs to handle this lifecycle without completing an outer nested invocation. The executor passes the same `Object[] args` reference through all hooks for one invocation, providing an existing invocation identity.

### Steps to reproduce the behavior, such as: SQL to execute, sharding rule configuration, when exception occur etc.

1. Create one `OpenTelemetryJDBCExecutorCallbackAdvice` and one callback fixture implementing `TargetAdviceObject`.
2. Use an `InMemorySpanExporter` and `SimpleSpanProcessor` with OpenTelemetry SDK.
3. Prepare two argument arrays for `SELECT 1` and `SELECT 2`.
4. Use two executor threads and three bounded `CountDownLatch` instances to enforce the exact sequence above. Use the same argument array for all hooks belonging to one invocation, matching `InstanceMethodAdviceExecutor`.
5. Assert two completed span IDs, A's `SELECT 1`/OK/no-exception attributes, and B's `SELECT 2`/ERROR/one-exception attributes.

### Example codes for reproduce this issue (such as a github link).

The [test-only reproducer commit](https://github.com/ym0506/shardingsphere/commit/92571c88362bf71b2b3e49d1ec94834fd67854a7) keeps production code at the base above. Its `OpenTelemetryJDBCExecutorCallbackAdviceTest.assertConcurrentInvocations` compiles and deterministically fails with expected 2 / actual 1. Run:

```sh
./mvnw -B -pl agent/plugins/tracing/type/opentelemetry -am \
-Dtest=OpenTelemetryJDBCExecutorCallbackAdviceTest#assertConcurrentInvocations \
-Dsurefire.failIfNoSpecifiedTests=false test
```

### Separate 5.5.3/MySQL corroboration

I also reran the [runtime-absent fixture at da1240b](https://github.com/ym0506/routecontract/blob/da1240b46765d91695922c28a7d79dc2eb66e8fc/docs/agent-runtime-absent-reproducer.md) on 2026-09-08 with the SHA-512-verified official Agent 5.5.3 and MySQL 8.4.11. RouteContract API/hook/SPI/artifact are absent from the forked runtime; datasource-proxy is the backing-callback count oracle. Across 20 controls and 20 forced two-way fan-outs, it reports 60 successful backing callbacks but only 40 completed execute spans (20 control, 20 fan-out). This is an exact forced-overlap fixture, not a production loss rate or current-master database test.

### Fix direction to discuss

An invocation-keyed local candidate passes concurrency, nesting, failed-before and exception-then-finally tests. Applying only the two corresponding advice classes to an explicitly modified 5.5.3 Agent also gives 60 completed spans for the same 60-callback MySQL fixture, including two distinct-target children for every fan-out root. This is a local experimental build, not an official release.

I would appreciate guidance on the invocation-state owner before proposing a production PR. A per-thread weak-key map avoids strongly retaining abandoned argument arrays, but stale Span values need a later map access after GC. Dynamic `agent-plugins-enabled` changes can skip completion hooks. The candidate also adds about 40 B and 62 ns per invocation in a local advice/SDK-only loop, excluding database and exporter work. I am not treating that prototype as performance-neutral or ready to merge.

AI assistance: OpenAI Codex assisted with investigation, reproductions, tests and the experimental candidate.

Contributor guide

Open the contributing guide

Research direction

Run the named Maven command for OpenTelemetryJDBCExecutorCallbackAdviceTest#assertConcurrentInvocations and read OpenTelemetryJDBCExecutorCallbackAdvice, TargetAdviceObject, and InstanceMethodAdviceExecutor. Trace how args and attachments move through before, afterMethod, and onThrowing, including finally and nested calls. Done means the concurrency, nesting, failed-before, and exception-then-finally tests preserve two spans with their own attributes and outcomes.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
databases, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.