collectors: add inbound fee metric #54

joostjager · 2020-07-02T10:12:19Z

This PR adds a cumulative metric for the routing fee that senders need to pay to the last hop to reach the node that is being monitored. It is an indication of the quality of inbound liquidity.

Example output to Prometheus:

# HELP inbound_fee fee charged for forwarding to this node
# TYPE inbound_fee gauge
inbound_fee{amount="0.001 BTC"} 4e-05
inbound_fee{amount="0.002 BTC"} 4.5e-05
inbound_fee{amount="0.004 BTC"} 4.75e-05
inbound_fee{amount="0.008 BTC"} 7.75e-05
inbound_fee{amount="0.016 BTC"} 0.00030125
inbound_fee{amount="0.032 BTC"} 0.000650625
inbound_fee{amount="0.064 BTC"} 0.00082515625
inbound_fee{amount="0.128 BTC"} 0.000912578125

When used in a primary-gateway setup, it is typically required to ignore the internal channels for this metric. This can be indicated by passing the pubkey of the primary node via the --primarynode command line flag.

collectors/channel_collector_test.go

collectors/channels_collector.go

bhandras

LGTM, I think this is a nice metric to help evaluate inbound quality.

carlaKC · 2020-07-02T13:58:30Z

.gitignore

@@ -17,3 +17,5 @@ user-config/*
 !user-config/.gitkeep
 nginx/etc/ssl/*
 nginx/etc/.htpasswd
+
+cmd/lndmon/lndmon


nit: newline?

the idea was to separate our binaries from other peoples files to ignore

no I meant one after cmd/lndmon/lndmon but non-blocking

carlaKC · 2020-07-02T14:00:30Z

collectors/channel_collector_test.go

+	"github.com/stretchr/testify/require"
+)
+
+type inboundFeeTestCase struct {


Do we need this struct? Couldn't we just have testCases=[]struct{...} then pass 3 params to testGetInboundFee? I don't think the indenting would be too bad.

Not three, but five in that case. Also the remotePolicies and remoteBalances. Sure you want that?

Did move the test cases into the function.

collectors/channel_collector_test.go

carlaKC · 2020-07-02T14:07:02Z

collectors/channels_collector.go

+	// total amount. Each iteration, the best channel for that shard is
+	// selected based on the specific fee.
+	amountRemaining := amt
+	for amountRemaining > 0 {


Is this an accurate reflection of our available inbound if we don't limit the number of shards? Maybe I'm missing it in here, but it seems like we should be taking that into account, because they can't actually reach us if we exceed shard max.

Max shards is configurable, so we can't make assumptions on that.

lnd's default is probably a fair representation of reality, but we can always come back and refine.

carlaKC

LGTM! Nice to start surfacing information like this 📈

joostjager · 2020-07-03T11:28:03Z

Found one nil pointer bug while testing on staging, fixed.

Roasbeef · 2020-07-07T01:46:38Z

collectors/prometheus.go

-	for _, collectorFunc := range collectors {
-		err := prometheus.Register(collectorFunc(p.lnd))
+	collectors := []prometheus.Collector{
+		NewChainCollector(p.lnd),


Why's this preferable to the init functions?

There is an initialization parameter now that comes from the config. (primary pub key)

Needs to be updated now after the merge of the get info collector. The main reason why the existing structure was used was so we'd just need to add the new collector in the file it was created in, rather than here as well. In the end something needs to be updated either way though.

joostjager · 2020-07-13T13:47:13Z

I wanted to do a bit more testing before merge and also update the json dashboard files. But can do that in a follow-up PR.

joostjager requested review from bhandras and carlaKC July 2, 2020 10:12

joostjager force-pushed the inbound-fee branch from 93daccf to c198303 Compare July 2, 2020 10:13

bhandras reviewed Jul 2, 2020

View reviewed changes

joostjager force-pushed the inbound-fee branch from c198303 to fed69d8 Compare July 2, 2020 12:34

joostjager requested a review from bhandras July 2, 2020 12:34

bhandras reviewed Jul 2, 2020

View reviewed changes

collectors/channels_collector.go Outdated Show resolved Hide resolved

collectors/channels_collector.go Outdated Show resolved Hide resolved

collectors/channels_collector.go Show resolved Hide resolved

collectors/channels_collector.go Outdated Show resolved Hide resolved

joostjager force-pushed the inbound-fee branch from fed69d8 to 111be96 Compare July 2, 2020 14:06

joostjager requested a review from bhandras July 2, 2020 14:06

bhandras approved these changes Jul 2, 2020

View reviewed changes

carlaKC reviewed Jul 2, 2020

View reviewed changes

joostjager force-pushed the inbound-fee branch from 111be96 to fbdb5a3 Compare July 2, 2020 14:19

joostjager requested a review from carlaKC July 2, 2020 14:21

carlaKC approved these changes Jul 2, 2020

View reviewed changes

joostjager force-pushed the inbound-fee branch from fbdb5a3 to 506b2d6 Compare July 3, 2020 11:27

Roasbeef reviewed Jul 7, 2020

View reviewed changes

joostjager added 2 commits July 9, 2020 10:10

collectors: add inbound fee metric

0f51993

collectors: do not use global init functions

fbc9368

joostjager force-pushed the inbound-fee branch from dca82b1 to 0606a04 Compare July 9, 2020 08:10

collectors: filter inbound fee metric by external channels

ce20ba5

joostjager force-pushed the inbound-fee branch from 0606a04 to ce20ba5 Compare July 9, 2020 08:12

Roasbeef merged commit 9e8c887 into lightninglabs:master Jul 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

collectors: add inbound fee metric #54

collectors: add inbound fee metric #54

joostjager commented Jul 2, 2020 •

edited

Loading

bhandras left a comment

carlaKC Jul 2, 2020

joostjager Jul 2, 2020 •

edited

Loading

carlaKC Jul 2, 2020

carlaKC Jul 2, 2020

joostjager Jul 2, 2020

joostjager Jul 2, 2020

carlaKC Jul 2, 2020

joostjager Jul 2, 2020 •

edited

Loading

carlaKC Jul 2, 2020

carlaKC left a comment

joostjager commented Jul 3, 2020 •

edited

Loading

Roasbeef Jul 7, 2020

joostjager Jul 7, 2020 •

edited

Loading

Roasbeef Jul 8, 2020

joostjager Jul 9, 2020

joostjager commented Jul 13, 2020

collectors: add inbound fee metric #54

collectors: add inbound fee metric #54

Conversation

joostjager commented Jul 2, 2020 • edited Loading

bhandras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joostjager Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joostjager Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlaKC left a comment

Choose a reason for hiding this comment

joostjager commented Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

joostjager Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joostjager commented Jul 13, 2020

joostjager commented Jul 2, 2020 •

edited

Loading

joostjager Jul 2, 2020 •

edited

Loading

joostjager Jul 2, 2020 •

edited

Loading

joostjager commented Jul 3, 2020 •

edited

Loading

joostjager Jul 7, 2020 •

edited

Loading