Skip to content

Conversation

@abitofevrything
Copy link
Contributor

Depends on #435 as the new filtering logic in discovered_nodes affects metric collection.

scrapePeerCount prometheus.Histogram
scrapeNodeCount prometheus.Histogram
scrapeNodeTotal *prometheus.CounterVec
logger *zap.SugaredLogger
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a LOT of properties to add here; I think much of this is already being collected here/elsewhere? https://github.com/bitmagnet-io/bitmagnet/blob/main/internal/protocol/dht/server/prometheus_collector.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the metrics I've added here are separate from the existing collectors. They serve two purposes not covered by the sucess/latency metrics already existing.

  • The histograms gather statistics about the responses received from the DHT per query type (something that isn't recorded by current metrics as the recorded metric depends on the query type). This isn't immediately useful to us, it's more interesting from a general DHT analysis perspective.
  • The counters gather statistics about the flow of data through the DHT crawler itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am aware it's a lot of metrics to add; my opinion with metrics is that it's better to err on the side of too many rather than not enough seeing as they are so cheap to add. If there's any metrics in particular you think aren't useful, we can get rid of them.

@abitofevrything abitofevrything force-pushed the feat/dht-crawler-metrics branch from fd8eaa9 to ac2831f Compare July 27, 2025 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants