Copilot commented on code in PR #7705:
URL: https://github.com/apache/shenyu/pull/7705#discussion_r4301683170
##########
shenyu-plugin/shenyu-plugin-tracing/src/main/java/org/apache/shenyu/plugin/tracing/TracingPlugin.java:
##########
@@ -103,6 +123,15 @@ public String named() {
*/
private Mono<Void> withSpan(final ServerWebExchange exchange, final
Mono<Void> chainResult) {
final Context parent = extractor.extract(exchange);
+ if (!createSpan) {
+ // the deployment instruments the gateway already, so the request
is propagated without
+ // creating a second span for it: the incoming context is the
current context, which the
+ // access log and the outgoing call see as well.
+ ReactorTracingContext.store(exchange, parent);
+ exposeTraceId(exchange, Span.fromContext(parent));
+ return chainResult
+ .contextWrite(context ->
context.put(TracingConstants.REACTOR_CONTEXT_KEY, parent));
Review Comment:
With `createSpan=false`, this stores the inbound client's context rather
than the agent's active server span. `TraceContextExtractor` extracts from
`Context.root()`: without `traceparent`, this plugin exposes no access-log
trace ID and supplies no valid context to its outbound propagator; with
headers, it propagates the remote parent instead of the agent's gateway span.
Prefer the active instrumentation context, preserving it across Reactor
subscriptions, and fall back to header extraction only when no valid active
span exists. Add coexistence tests with an active span, both with and without
incoming headers.
##########
shenyu-spring-boot-starter/shenyu-spring-boot-starter-plugin/shenyu-spring-boot-starter-plugin-tracing/src/main/java/org/apache/shenyu/springboot/starter/plugin/tracing/TracingPluginConfiguration.java:
##########
@@ -93,17 +141,213 @@ public Tracer
tracingTracer(@Qualifier("tracingTracerProvider") final SdkTracerP
* Tracing plugin.
*
* @param tracer the tracer
+ * @param shenyuConfig the shenyu config
* @return the shenyu plugin
*/
@Bean
- public ShenyuPlugin tracingPlugin(@Qualifier("tracingTracer") final Tracer
tracer) {
- return new TracingPlugin(tracer);
+ public ShenyuPlugin tracingPlugin(@Qualifier("tracingTracer") final Tracer
tracer,
+ final ShenyuConfig shenyuConfig) {
+ return new TracingPlugin(tracer, new TraceContextExtractor(),
createSpan(shenyuConfig));
}
+ /**
+ * Acquires the tracing config.
+ *
+ * @param shenyuConfig the shenyu config
+ * @return the tracing config, empty while it is not configured
+ */
+ private Optional<ShenyuConfig.TracingConfig> tracingConfig(final
ShenyuConfig shenyuConfig) {
+ return Optional.ofNullable(shenyuConfig.getTracing());
+ }
+
+ /**
+ * Acquires the service name of the gateway.
+ *
+ * @param shenyuConfig the shenyu config
+ * @return the service name
+ */
private String serviceName(final ShenyuConfig shenyuConfig) {
- return Optional.ofNullable(shenyuConfig.getTracing())
+ return tracingConfig(shenyuConfig)
.map(ShenyuConfig.TracingConfig::getServiceName)
.filter(StringUtils::isNotBlank)
.orElse(DEFAULT_SERVICE_NAME);
}
+
+ /**
+ * Tells whether the gateway creates a span of its own.
+ *
+ * @param shenyuConfig the shenyu config
+ * @return true while the plugin creates a span
+ */
+ private boolean createSpan(final ShenyuConfig shenyuConfig) {
+ return
tracingConfig(shenyuConfig).map(ShenyuConfig.TracingConfig::getCreateSpan).orElse(Boolean.TRUE);
+ }
+
+ /**
+ * Acquires the resource attributes which are added to the resource of the
gateway spans.
+ *
+ * @param shenyuConfig the shenyu config
+ * @return the configured resource attributes, empty while none is
configured
+ */
+ private Optional<Properties> resourceAttributes(final ShenyuConfig
shenyuConfig) {
+ return tracingConfig(shenyuConfig)
+ .map(ShenyuConfig.TracingConfig::getResourceAttributes)
+ .filter(attributes -> !attributes.isEmpty());
+ }
+
+ /**
+ * Builds the sampler which decides whether a request is traced.
+ *
+ * @param shenyuConfig the shenyu config
+ * @return the sampler
+ */
+ private Sampler sampler(final ShenyuConfig shenyuConfig) {
+ final ShenyuConfig.TracingConfig config = tracingConfig(shenyuConfig)
+ .orElseGet(ShenyuConfig.TracingConfig::new);
+ final String sampler = Optional.ofNullable(config.getSampler())
+ .map(String::trim)
+ .filter(StringUtils::isNotBlank)
+ .orElse(TracingConstants.SAMPLER_PARENT_BASED)
+ .toLowerCase(Locale.ROOT);
+ switch (sampler) {
+ case TracingConstants.SAMPLER_ALWAYS_ON:
+ return Sampler.alwaysOn();
+ case TracingConstants.SAMPLER_ALWAYS_OFF:
+ return Sampler.alwaysOff();
+ case TracingConstants.SAMPLER_RATIO:
+ return
Sampler.parentBased(Sampler.traceIdRatioBased(ratio(config.getSampleRatio())));
+ case TracingConstants.SAMPLER_PARENT_BASED:
+ return Sampler.parentBased(Sampler.alwaysOn());
+ default:
+ LOG.warn("shenyu tracing does not know the sampler {}, the
parent based sampler is used", sampler);
+ return Sampler.parentBased(Sampler.alwaysOn());
+ }
+ }
+
+ /**
+ * Normalizes the sample ratio.
+ *
+ * <p>A ratio which is not a number or which is out of the range would
silently change the
+ * sampling behaviour, so it is reported and clamped instead of failing
the gateway.</p>
+ *
+ * @param sampleRatio the configured sample ratio
+ * @return the ratio within the valid range
+ */
+ private double ratio(final double sampleRatio) {
+ if (Double.isNaN(sampleRatio)) {
+ LOG.warn("shenyu tracing could not read the sample ratio, no
request is sampled");
+ return 0.0D;
+ }
+ final double bounded = Math.max(0.0D, Math.min(1.0D, sampleRatio));
+ if (bounded != sampleRatio) {
+ LOG.warn("shenyu tracing clamps the sample ratio {} to {}",
sampleRatio, bounded);
+ }
+ return bounded;
+ }
+
+ /**
+ * Builds the batch processor of the exporter.
+ *
+ * @param exporter the exporter of the gateway spans
+ * @return the batch processor, empty while it cannot be created
+ */
+ private Optional<SpanProcessor> spanProcessor(final SpanExporter exporter)
{
+ try {
+ return Optional.of(BatchSpanProcessor.builder(exporter).build());
+ } catch (RuntimeException | LinkageError ex) {
+ LOG.warn("shenyu tracing could not create the batch processor of
the exporter, the spans are not exported", ex);
+ return Optional.empty();
+ }
+ }
+
+ /**
+ * Creates the OTLP exporter of the gateway spans.
+ *
+ * @param shenyuConfig the shenyu config
+ * @return the span exporter, empty while no endpoint is configured or the
exporter cannot be created
+ */
+ private Optional<SpanExporter> createSpanExporter(final ShenyuConfig
shenyuConfig) {
+ final Optional<ShenyuConfig.TracingConfig.OtlpConfig> configured =
tracingConfig(shenyuConfig)
+ .map(ShenyuConfig.TracingConfig::getOtlp)
+ .filter(otlp -> StringUtils.isNotBlank(otlp.getEndpoint()));
+ if (!configured.isPresent()) {
+ return Optional.empty();
+ }
+ final ShenyuConfig.TracingConfig.OtlpConfig otlp = configured.get();
+ try {
+ final String endpoint = endpoint(otlp.getEndpoint());
+ final OtlpHttpSpanExporterBuilder builder =
OtlpHttpSpanExporter.builder().setEndpoint(endpoint);
+ if (Objects.nonNull(otlp.getTimeout())) {
+ builder.setTimeout(otlp.getTimeout());
+ }
+ if (Objects.nonNull(otlp.getHeaders())) {
+ final Properties headers = otlp.getHeaders();
+ headers.stringPropertyNames().forEach(name ->
builder.addHeader(name, headers.getProperty(name)));
+ }
+ return Optional.of(builder.build());
+ } catch (RuntimeException | LinkageError ex) {
+ // the message of the failure may quote the endpoint, so only its
type is logged at warn
+ // level while the details stay at debug level
+ LOG.warn("shenyu tracing could not create the OTLP exporter of {},
the spans are not exported: {}",
+ redactedEndpoint(otlp.getEndpoint()),
ex.getClass().getSimpleName());
+ LOG.debug("shenyu tracing could not create the OTLP exporter", ex);
Review Comment:
Logging the raw exception at DEBUG bypasses endpoint redaction. For example,
a credential-bearing endpoint with an unescaped space makes `URI.create()`
throw an exception containing the entire URL, including user information and
query tokens. When DEBUG logging is enabled, those credentials reach the logs.
Remove the throwable logging and retain only the redacted endpoint and
exception type; credentials must remain protected at every log level.
--
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]
