Skip to content

fix: skip prometheus metrics trackProtocolStream for identify #8958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lodekeeper wants to merge 1 commit into from
Closed

fix: skip prometheus metrics trackProtocolStream for identify #8958

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions packages/beacon-node/src/network/libp2p/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,30 @@ export async function createNodeJsLibp2p(
noiseCrypto.chaCha20Poly1305Encrypt = asCrypto.chaCha20Poly1305Encrypt;
}

const libp2pMetrics = nodeJsLibp2pOpts.metrics
? ((components: LodestarComponents) => {
const metrics = prometheusMetrics({
collectDefaultMetrics: false,
preserveExistingMetrics: true,
registry: nodeJsLibp2pOpts.metricsRegistry,
})(components);

// Work around identify EOF race:
// `trackProtocolStream` attaches a `message` listener immediately after protocol
// negotiation. For `/ipfs/id/1.0.0`, identify() adds its own reader later and can
// miss the first response frame when metrics listener drains events first.
const originalTrackProtocolStream = metrics.trackProtocolStream.bind(metrics);
metrics.trackProtocolStream = ((stream) => {
if (stream.protocol === "/ipfs/id/1.0.0") {
return;
}
originalTrackProtocolStream(stream);
}) as typeof metrics.trackProtocolStream;
Comment on lines +88 to +94

@gemini-code-assist gemini-code-assist Bot Feb 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this direct method reassignment (metrics.trackProtocolStream = ...) effectively implements the workaround, directly modifying methods of external library instances can be fragile. If the prometheusMetrics library's internal implementation of trackProtocolStream changes (e.g., how it uses this or other internal state), this patch might break or behave unexpectedly. For a more robust solution, consider creating a custom metrics wrapper class that implements the Metrics interface and explicitly delegates calls to an internal prometheusMetrics instance, overriding trackProtocolStream with your custom logic. This provides better encapsulation and makes the dependency on the external library's internal structure less direct.

@lodekeeper lodekeeper Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gemini-code-assist Good point about fragility. This is intentionally a minimal workaround — the proper fix belongs upstream in @libp2p/prometheus-metrics (the _track() method should use a non-consuming observer pattern rather than addEventListener('message', ...)). A full wrapper class adds complexity for what should be a temporary patch. If the upstream fix lands, we remove this entirely. If trackProtocolStream changes signature, the TypeScript compiler will catch it at build time since we preserve the type via as typeof metrics.trackProtocolStream.

@gemini-code-assist gemini-code-assist Bot Feb 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point regarding the temporary nature of the fix and the upstream root cause. Given that the proper solution lies in the _track() method of @libp2p/prometheus-metrics and that TypeScript provides compile-time checks for signature changes, this minimal workaround is a pragmatic approach for the interim.


return metrics;
})
: undefined;

return createLibp2p({
privateKey,
nodeInfo: {
Expand Down Expand Up @@ -101,13 +125,7 @@ export async function createNodeJsLibp2p(
],
streamMuxers: [mplex({disconnectThreshold})],
peerDiscovery,
metrics: nodeJsLibp2pOpts.metrics
? prometheusMetrics({
collectDefaultMetrics: false,
preserveExistingMetrics: true,
registry: nodeJsLibp2pOpts.metricsRegistry,
})
: undefined,
metrics: libp2pMetrics,
connectionManager: {
// dialer config
maxParallelDials: 100,
Expand Down