Skip to content
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

feat: add method tracing to routing #715

Merged
merged 1 commit into from
Jan 13, 2025
Merged

feat: add method tracing to routing #715

Show file tree
Hide file tree
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
4 changes: 2 additions & 2 deletions packages/interface/src/routing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { AbortOptions, PeerId, PeerInfo } from '@libp2p/interface'
import type { AbortOptions, PeerId, PeerInfo, TraceOptions } from '@libp2p/interface'
import type { CID } from 'multiformats/cid'
import type { ProgressOptions } from 'progress-events'

Expand All @@ -8,7 +8,7 @@ import type { ProgressOptions } from 'progress-events'
* local cache that may be used in preference over network calls, for example
* when a record has a TTL.
*/
export interface RoutingOptions extends AbortOptions, ProgressOptions {
export interface RoutingOptions extends AbortOptions, ProgressOptions, TraceOptions {
/**
* Pass `false` to not use the network
*
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"@ipld/dag-cbor": "^9.2.2",
"@ipld/dag-json": "^10.2.3",
"@ipld/dag-pb": "^4.1.3",
"@libp2p/interface": "^2.2.1",
"@libp2p/interface": "^2.4.0",
"@libp2p/logger": "^5.1.4",
"@libp2p/utils": "^6.2.1",
"@multiformats/dns": "^1.0.6",
Expand Down
25 changes: 24 additions & 1 deletion packages/utils/src/routing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { NotFoundError, start, stop } from '@libp2p/interface'
import { PeerQueue } from '@libp2p/utils/peer-queue'
import merge from 'it-merge'
import type { Routing as RoutingInterface, Provider, RoutingOptions } from '@helia/interface'
import type { AbortOptions, ComponentLogger, Logger, PeerId, PeerInfo, Startable } from '@libp2p/interface'
import type { AbortOptions, ComponentLogger, Logger, Metrics, PeerId, PeerInfo, Startable } from '@libp2p/interface'
import type { CID } from 'multiformats/cid'

const DEFAULT_PROVIDER_LOOKUP_CONCURRENCY = 5
Expand All @@ -15,6 +15,7 @@ export interface RoutingInit {

export interface RoutingComponents {
logger: ComponentLogger
metrics?: Metrics
}

export class Routing implements RoutingInterface, Startable {
Expand All @@ -26,6 +27,28 @@ export class Routing implements RoutingInterface, Startable {
this.log = components.logger.forComponent('helia:routing')
this.routers = init.routers ?? []
this.providerLookupConcurrency = init.providerLookupConcurrency ?? DEFAULT_PROVIDER_LOOKUP_CONCURRENCY

this.findProviders = components.metrics?.traceFunction('helia.routing.findProviders', this.findProviders.bind(this), {
optionsIndex: 1
}) ?? this.findProviders
this.provide = components.metrics?.traceFunction('helia.routing.provide', this.provide.bind(this), {
optionsIndex: 1
Copy link
Member

Choose a reason for hiding this comment

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

What's optionsIndex?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/libp2p/js-libp2p/blob/8c515f240b04e6f7cc02a5b12aa74f02c616d4bc/packages/interface/src/metrics/index.ts#L519-L522

Copy link
Member

Choose a reason for hiding this comment

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

Even after having read that, I'm not sure how optionsIndex influences the trace context and what the value should be, e.g. why do some of the calls here use optionsIndex: 2

Copy link
Member Author

Choose a reason for hiding this comment

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

The metrics implementation has to know which argument is the options object in order to extract the trace context. It can't figure it out for itself so we have to tell it:

myTracedFunction(arg0, arg1, options)  // <-- optionsIndex = 2

myOtherTracedFunction(arg0, options)  // <-- optionsIndex = 1

}) ?? this.provide
this.cancelReprovide = components.metrics?.traceFunction('helia.routing.cancelReprovide', this.cancelReprovide.bind(this), {
optionsIndex: 1
}) ?? this.cancelReprovide
this.put = components.metrics?.traceFunction('helia.routing.put', this.put.bind(this), {
optionsIndex: 2
}) ?? this.put
this.get = components.metrics?.traceFunction('helia.routing.get', this.get.bind(this), {
optionsIndex: 1
}) ?? this.get
this.findPeer = components.metrics?.traceFunction('helia.routing.findPeer', this.findPeer.bind(this), {
optionsIndex: 1
}) ?? this.findPeer
this.getClosestPeers = components.metrics?.traceFunction('helia.routing.getClosestPeers', this.getClosestPeers.bind(this), {
optionsIndex: 1
}) ?? this.getClosestPeers
}

async start (): Promise<void> {
Expand Down
Loading