wy471x commented on issue #7566:
URL: https://github.com/apache/shenyu/issues/7566#issuecomment-6127974437
## Implementation plan (progress tracking)
Sharing the plan I am working from so progress on this issue stays visible.
Feedback welcome, especially on the design decisions in section 1, before I
open the skeleton PR.
### 1. Design decisions to confirm first
| # | Decision | Options considered | Proposal |
|---|---|---|---|
| D1 | Plugin shape | (a) Global/local-config plugin: no DB row, no
selectors/rules (same as `metrics` and `global`, which have no record in the
`plugin` table); (b) admin-managed plugin: `plugin` + `plugin_handle` rows and
`AbstractShenyuPlugin` selector/rule matching | (a) for v1. Enabling tracing
should not depend on data sync, and it avoids touching DB init scripts for 5
dialects. (b) can follow if dashboard control is wanted |
| D2 | Dependency boundary | core module depends on `opentelemetry-api`
only, SDK/exporter live in the starter or a separate module | separate them.
Caveat: `grpc-netty` is already on the gateway classpath
(`shenyu-plugin-grpc/pom.xml`), so an OTLP-over-gRPC exporter risks dependency
conflicts. Default to the `otlp-http` exporter and verify convergence with `mvn
dependency:tree` |
| D3 | Coexistence with a Java agent | create the span by default;
`shenyu.tracing.create-span=false`; or reuse an already valid context instead
of starting a new one | provide the flag and the reuse behaviour,
`create-span=true` by default |
| D4 | Default state | disabled by default at the property level
(`shenyu.tracing.enabled=false`, gated with `@ConditionalOnProperty`, same as
`MetricsPluginConfiguration`), rather than a disabled DB row. A plugin with no
DB row never receives a disable event and would keep executing | property-level
opt-in |
### 2. Module layout
```
shenyu-plugin/shenyu-plugin-tracing/ # new: API + logic
only, no exporter implementation
TracingPlugin.java # implements
ShenyuPlugin, wraps chain.execute()
TracingConstants.java
extract/TraceContextExtractor.java # W3C
traceparent/tracestate + baggage
extract/SpanAttributesBuilder.java # normalized
route/plugin/rpcType/upstream/status
context/ReactorTracingContext.java # current span held
in Reactor Context (not ThreadLocal)
spi/TracingPropagator.java # @SPI outbound
injection extension point
shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/
shenyu-spring-boot-starter-plugin-tracing/
TracingPluginConfiguration.java #
@ConditionalOnProperty("shenyu.tracing.enabled")
```
Plus the in-place changes:
- `PluginEnum.TRACING` - proposed order `0`, i.e. after `GLOBAL(-1)` and
before `CACHE(10)` / `METRICS(15)`, so the span covers the whole observable
chain.
- Module registration in `shenyu-plugin/pom.xml`,
`shenyu-spring-boot-starter-plugin/pom.xml`, and a starter dependency in
`shenyu-bootstrap/pom.xml`.
- `shenyu-common/.../config/ShenyuConfig.java` - new `TracingConfig`,
following the existing `MetricsConfig` pattern.
- `shenyu-plugin-httpclient` (`WebClientPlugin`, `NettyHttpClientPlugin`)
for outbound injection.
- `ShenyuPlugin.after()` and `ShenyuWebHandler.after()` to use the span
trace id instead of `exchange.getLogPrefix()`.
Two implementation notes worth recording:
- The span must be carried in the Reactor Context: `ShenyuWebHandler.handle`
may call `subscribeOn(scheduler)` and switch threads, so a `ThreadLocal` would
lose it.
- `ShenyuWebHandler.sortPlugins` overrides `getOrder()` with the
admin-supplied `PluginData.getSort()`, so option D1 (b) would also require
consistent `sort` values in the DB rows.
### 3. PR breakdown
Each PR is independently reviewable and revertible, and passes `mvn -pl
<module> -am test` plus `validate` (checkstyle + RAT).
**PR1 - skeleton and inbound span (HTTP, disabled by default, noop/console
exporter)**
Scope: new module + starter + `PluginEnum.TRACING` + pom registration; W3C
`traceparent`/`tracestate` and baggage extraction; invalid or absent header
starts a new trace instead of failing; server span created around
`chain.execute()`; write the trace id back to the `shenyu-agent-trace-id`
exchange attribute so logging plugins correlate access logs.
Tests: `TracingPluginTest` (valid context -> same trace and correct parent;
no context -> new trace; malformed context -> new trace and no exception;
disabled -> unchanged behaviour), `TraceContextExtractorTest` (table-driven
header edge cases), `TracingPluginConfigurationTest` (property toggling).
AC: 1, 2, 6 (partial), 7.
**PR2 - span lifecycle and gateway span attributes**
Scope: correct `end()` and span status on success, HTTP error, thrown
exception, timeout and cancellation, with idempotent termination; normalized
attributes (rpc type, HTTP method/status, module/method, upstream target,
selector/rule ids where available) with no high-cardinality or sensitive data
by default; replace `getLogPrefix()` in the plugin and chain timing logs.
Tests: `TracingSpanLifecycleTest` (parameterized over the five paths, span
ended exactly once), `TracingReactorContextTest` (same trace id survives
`subscribeOn(scheduler)`), `SpanAttributesTest` (no query string, no raw path),
`ShenyuPluginAfterTest` / `ShenyuWebHandlerTest` (log correlation, graceful
fallback without a span).
AC: 4, 5, and the rest of 6.
**PR3 - outbound HTTP injection and protocol extension point**
Scope: inject the active context in `WebClientPlugin` and
`NettyHttpClientPlugin`; remove the client's original `traceparent` before
injecting, otherwise `headers.addAll(exchange.getRequest().getHeaders())`
forwards the original header and the upstream receives duplicates; define the
`TracingPropagator` SPI (HTTP implementation only in v1) as the extension point
for Spring Cloud, gRPC and Dubbo.
Tests: `HttpTracePropagationTest` (single `traceparent`, parent span id
equals the gateway span id, trace id continuity with and without an inbound
context), `TracingPropagatorSpiTest` (extension loading).
AC: 3.
**PR4 - OTLP export, configuration surface, sampling, failure isolation,
agent coexistence**
Scope: `shenyu.tracing.*` properties (enablement, service name, resource
attributes, sampler, propagators, OTLP endpoint/protocol/batch, `create-span`);
batch export; all exporter and sampler failures caught and downgraded so
request forwarding is never affected, including exporter construction failure
at startup.
Tests: `TracingExporterFailureTest` (unreachable endpoint, throwing
exporter, throwing sampler -> chain still completes normally),
`TracingPropertiesTest` (defaults and overrides), sampler behaviour (always
on/off, ratio, parent-based), `AgentCoexistenceTest`.
AC: 7, 8.
**PR5 - control-plane registration (only if D1 (b) is chosen)**
Scope: `plugin` and `plugin_handle` rows in the DB init scripts for all 5
dialects plus the upgrade scripts, with `sort` consistent with
`PluginEnum.TRACING`.
Tests: plugin appears in the admin plugin list, enable/disable propagates to
the gateway, scripts executable per dialect.
AC: none directly.
**PR6 - integration test and documentation**
Scope: `shenyu-integrated-test/shenyu-integrated-test-tracing` modelled on
the existing HTTP integration test, asserting trace continuity and parent/child
relationships across `test client -> ShenYu -> mock upstream`, plus access-log
correlation and baggage/tracestate passthrough. Documentation covers
configuration, supported propagators/protocols, backend examples and
performance considerations.
Tests: the three main chains, plus "sampling disabled does not affect
forwarding" and "exporter unavailable does not affect forwarding".
AC: 9, 10.
I will post per-PR notes here as each one lands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
