docker/observation/configs/otel-collector/config.yaml (2)

62-63: 🧹 Nitpick | 🔵 Trivial

Unused debug exporter — remove or comment out.

The debug exporter is declared but never added to any pipeline, so it is silently disabled. Configuring an exporter does not enable it; exporters are enabled by adding them to the appropriate pipelines within the service section. Either wire it into a pipeline when troubleshooting is needed, or remove it to keep the configuration free of dead declarations.

🧹 Proposed cleanup

-  # Debug exporter for troubleshooting (optional)
-  debug:
-    verbosity: basic
-
 service:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker/observation/configs/otel-collector/config.yaml` around lines 62 - 63,
The config declares a debug exporter block named "debug" but never uses it;
either remove or comment out the "debug:" exporter stanza or add "debug" to the
appropriate pipeline(s) under the service.pipelines section so it becomes active
during troubleshooting; locate the "debug" exporter key in the config and either
delete/comment that block or add "debug" to the desired pipeline's exporters
list (e.g., service.pipelines.<your_pipeline>.exporters).

---

19-26: ⚠️ Potential issue | 🟠 Major

Remove the now-orphaned spanmetrics connector definition.

The spanmetrics connector was removed from both its pipeline roles — traces.exporters (line 75) and metrics.receivers (line 77) — but its full definition still lives in the connectors: block. Configuring a connector doesn't enable it; connectors are enabled through pipelines within the service section. So the collector won't error on startup today. However, leaving the definition in place is dangerous: each connector MUST be used as both, in separate pipelines, so if a future edit re-wires spanmetrics into only one pipeline role (e.g., only back into traces.exporters), the collector will fail at runtime with "failed to build pipelines: connector spanmetrics used as exporter … but not used in any supported receiver pipeline". This class of error is confirmed by real-world collector failures documented in the upstream tracker.

Delete the entire connectors: section since no other connector is defined.

🗑️ Proposed fix: remove orphaned connector block

-connectors:
-  spanmetrics:
-    histogram:
-      explicit:
-        buckets: [1ms, 5ms, 10ms, 25ms, 50ms, 100ms, 250ms, 500ms, 1s, 2.5s, 5s, 10s, 30s]
-    dimensions:
-      - name: service.name
-    metrics_expiration: 5m
-
 processors:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker/observation/configs/otel-collector/config.yaml` around lines 19 - 26,
Remove the orphaned connectors block that defines the spanmetrics connector:
delete the entire connectors: section (which contains spanmetrics with
histogram, dimensions, and metrics_expiration) since spanmetrics is no longer
referenced by traces.exporters or metrics.receivers; this prevents a future
runtime failure where a connector is used only as an exporter or receiver.
Ensure no other connectors remain before removing the connectors: block
entirely.

crates/utils/utils/src/metric_macros.rs (1)

1-20: 🧹 Nitpick | 🔵 Trivial

Remove orphaned @type_str macro arms — they are no longer invoked.

The three @type_str arms (counter, gauge, histogram) were previously used when the macro generated a METRICS descriptor array. After removal of that code path, these arms became dead code. Search of the entire codebase confirms no invocations of @type_str exist; all define_metrics! calls use only the top-level macro syntax.

The OpenTelemetry 0.31.0 builder API is correctly implemented: .u64_counter(), .u64_gauge(), .f64_histogram(), .with_description(), .with_boundaries(), and .build() all match the public API for that version.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/utils/utils/src/metric_macros.rs` around lines 1 - 20, Remove the dead
macro arms named `@type_str` from the `define_metrics!` macro in
metric_macros.rs: delete the three orphaned `@type_str` variants that handled
`counter`, `gauge`, and `histogram` since they are no longer referenced by any
`define_metrics!` invocation; keep the top-level macro arms that build
`.u64_counter()`, `.u64_gauge()`, `.f64_histogram()`, `.with_description()`,
`.with_boundaries()`, and `.build()` intact and run tests to ensure nothing else
relied on the removed arms.

crates/actors/src/validation_service.rs (1)

248-278: 🧹 Nitpick | 🔵 Trivial

Consider propagating the original request's parent_span through to BlockValidationFinished.

All three BlockValidationFinished construction sites use span: tracing::Span::current(), which captures the validation service's event-loop span rather than the original parent_span passed via ValidateBlock. As a result, BlockTreeService operations triggered by validation results appear as children of the validation service's loop span in traces — not of the original API/gossip request that initiated the validation.

The request_id correctly provides end-to-end correlation (per the PR's design rationale), so this doesn't affect functional correctness. But the span hierarchy is broken at this boundary: the full chain original_request_span → validation_stages → block_tree_handling is not represented.

If richer span hierarchies are desired, the fix is to propagate parent_span alongside request_id through process_vdf() and ConcurrentValidationResult:

♻️ Optional: carry parent_span through to BlockValidationFinished

In active_validations.rs, extend process_vdf() return type and ConcurrentValidationResult:

-) -> Option<(BlockHash, VdfValidationResult, Option<irys_types::RequestId>)>
+) -> Option<(BlockHash, VdfValidationResult, Option<irys_types::RequestId>, tracing::Span)>

 pub struct ConcurrentValidationResult {
     pub block_hash: BlockHash,
     pub validation_result: ValidationResult,
     pub request_id: Option<irys_types::RequestId>,
+    pub parent_span: tracing::Span,
 }

Then in validation_service.rs, replace span: tracing::Span::current() with the propagated span in all BlockValidationFinished sites.

Also applies to: 282-295

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/actors/src/validation_service.rs` around lines 248 - 278, The current
VDF handling in process_vdf()/ConcurrentValidationResult drops the original
request span so BlockValidationFinished uses tracing::Span::current() (the
validation loop span) instead of the originating parent_span from ValidateBlock;
update active_validations::process_vdf() and the ConcurrentValidationResult to
carry an Option<tracing::Span> (e.g., parent_span) alongside request_id and
validation result, modify the callers that build ConcurrentValidationResult to
attach the original parent_span from ValidateBlock, and in validation_service
when constructing BlockValidationFinished replace span: tracing::Span::current()
with the propagated span (or fallback to current() if None) so the
BlockTreeService sees the original request's span as the parent.

crates/p2p/src/block_pool.rs (2)

633-643: 🧹 Nitpick | 🔵 Trivial

request_id missing from #[instrument] span fields.

The skip_all attribute excludes request_id from the auto-generated span, so the request ID won't be visible in traces for the block processing span. Given the PR's explicit goal of per-request tracing correlation, adding it to fields(...) would be valuable.

🔭 Add `request_id` to instrument fields

 #[instrument(
     skip_all,
     target = "BlockPool",
-    fields(block.hash = ?block.header().block_hash, block.height = block.header().height),
+    fields(block.hash = ?block.header().block_hash, block.height = block.header().height, request_id = ?request_id),
 )]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/p2p/src/block_pool.rs` around lines 633 - 643, Add request_id to the
tracing span for process_block by including it in the #[instrument(...)] fields:
update the attribute on pub(crate) async fn process_block(...) to include
request_id = ?request_id alongside block.hash and block.height (e.g.,
fields(block.hash = ?block.header().block_hash, block.height =
block.header().height, request_id = ?request_id)); keep skip_all if you need to
avoid auto-capturing other args, but ensure request_id is explicitly listed so
traces include it.

---

1058-1109: 🧹 Nitpick | 🔵 Trivial

Background task drops span context — breaks the intra-node trace chain.

tokio::spawn at Line 1071 doesn't call .in_current_span(), so the spawned task detaches from the active OpenTelemetry span. Any traceparent propagated via the PR's W3C tracecontext machinery won't extend into this background payload fetch. This directly undermines the PR's intra-node span propagation goal.

🔭 Inherit span context in the background task

Add the import at the top of the file (if not already present):

+use tracing::Instrument as _;

Then instrument the spawned task:

-        tokio::spawn(async move {
+        tokio::spawn(async move {
             match Self::pull_and_seal_execution_payload(
                 &execution_payload_provider,
                 &chain_sync_sender,
                 evm_block_hash,
                 use_trusted_peers_only,
                 None,
                 request_id,
             )
             .await
             {
                 // ...
             }
-        });
+        }.in_current_span());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/p2p/src/block_pool.rs` around lines 1058 - 1109, The background task
spawned in pull_and_seal_execution_payload_in_background detaches from the
active tracing span because tokio::spawn is called without inheriting the
current span; fix by instrumenting the spawned future so it inherits the current
span (e.g., import tracing::Instrument if missing) and call .in_current_span()
(or .instrument(tracing::Span::current())) on the async block passed to
tokio::spawn for the task that uses execution_payload_provider,
gossip_broadcast_sender, and chain_sync_sender so the span context is preserved
into the background payload fetch.

crates/p2p/src/chain_sync.rs (1)

547-620: 🧹 Nitpick | 🔵 Trivial

Three updated task spawns in handle_message drop the span context.

RequestBlockFromTheNetwork (Line 557), PullPayloadFromTheNetwork (Line 579), and AttemptReprocessingBlock (Line 599) all spawn tasks that now propagate request_id but don't call .in_current_span(). The active OpenTelemetry span (and any injected traceparent) is lost at each spawn boundary. Instrument as _ is already imported (Line 23); the fix is identical for all three.

🔭 Propagate span context into all three message-handler spawns

 // RequestBlockFromTheNetwork handler
-                tokio::spawn(async move {
+                tokio::spawn(async move {
                     let result = inner
                         .request_parent_block(parent_block_hash, request_id)
                         .await;
                     if let Some(sender) = response {
                         if let Err(e) = sender.send(result) {
                             tracing::error!("Failed to send response: {:?}", e);
                         }
                     }
-                });
+                }.in_current_span());

 // PullPayloadFromTheNetwork handler
-                tokio::spawn(async move {
+                tokio::spawn(async move {
                     let result = inner
                         .gossip_data_handler
                         .pull_and_add_execution_payload_to_cache(
                             evm_block_hash,
                             use_trusted_peers_only,
                             request_id,
                         )
                         .await;
                     if let Err(e) = response.send(result) {
                         tracing::error!("Failed to send response: {:?}", e);
                     }
-                });
+                }.in_current_span());

 // AttemptReprocessingBlock handler
-                tokio::spawn(async move {
+                tokio::spawn(async move {
                     if let Some(cached_block) = inner.block_pool.get_cached_block(&block_hash).await
                     {
                         if let Err(e) = inner
                             .block_pool
                             .process_block(
                                 Arc::clone(&cached_block.block),
                                 cached_block.is_fast_tracking,
                                 cached_block.request_id,
                             )
                             .await
                         {
                             error!("Failed to reprocess block {:?}: {:?}", block_hash, e);
                         }
                     } else {
                         error!(
                             "Cannot reprocess block {:?} - not found in cache",
                             block_hash
                         );
                     }
-                });
+                }.in_current_span());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/p2p/src/chain_sync.rs` around lines 547 - 620, The spawned tasks for
SyncChainServiceMessage::RequestBlockFromTheNetwork,
::PullPayloadFromTheNetwork, and ::AttemptReprocessingBlock drop the current
tracing/OpenTelemetry span because they call tokio::spawn without
instrumentation; wrap each tokio::spawn(...) future with .in_current_span()
(Instrument as _ is already imported) so the request trace is propagated.
Concretely, apply .in_current_span() to the spawned futures that call
inner.request_parent_block(...),
inner.gossip_data_handler.pull_and_add_execution_payload_to_cache(...), and the
block reprocessing future that invokes inner.block_pool.process_block(...) so
the active span/traceparent is preserved across the spawn boundary.