Introduce telemetry for observability by Anyitechs · Pull Request #117 · lightningdevkit/ldk-server

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs · 2026-01-27T01:02:01Z

This introduces the foundational telemetry infrastructure to improve
the observability of LDK Server.

It adds a new /metrics endpoint exposed on the REST service address,
which serves Prometheus-compatible metrics. This endpoint is public and
does not require HMAC authentication, allowing for easy integration with
monitoring systems.

Added a Metrics utility struct to hold all the metrics we need to
expose.
Made Metrics configurable via config.
Added Prometheus basic auth support.

This is the first step in a larger effort to provide comprehensive telemetry.
Future updates will expand this to include other detailed metrics for channels,
balances, payments, etc.

Related issue #38

ldk-reviews-bot · 2026-01-27T01:02:04Z

👋 Thanks for assigning @benthecarman as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

tnull · 2026-01-27T08:06:30Z

 chrono = { version = "0.4", default-features = false, features = ["clock"] }
 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"


Please avoid taking this dependency and rather use one of the std::sync primitives (Once/OnceLock/LazyLock), if we need this at all.

tnull · 2026-01-27T08:08:15Z

 log = "0.4.28"
 base64 = { version = "0.21", default-features = false, features = ["std"] }
+lazy_static = "1.5.0"
+prometheus = "0.14.0"


Do we really need this dependency, or can we just reimplement it easily locally?
If we need it, this should be at the very least made optional behind the metrics feature, and disable any default features.

Yea, its a plain text file with some numbers, don't think we need to take a dep :)

Do we really need this dependency, or can we just reimplement it easily locally?
We can reimplement locally, but I think the dependency already offers some benefits that will come in handy when we want to provide metrics for things like balances in different states, payments, fees, etc.

It also pulls in several solo-maintainer dependencies (okay, with relatively well-known Rust community folks like burntsushi and dtolnay but also the apparently-kinda-unmaintained fnv crate) which we very strongly try to avoid given the security risk. Unless there's something that takes many hundred to a few thousand lines of code or very complicated and hard to test code to replicate, we should absolutely avoid taking a dependency for it.

Personally, I've implemented building prometheus endpoints in bash and python and....many times with probably less effort than it would have taken to figure out how to add a dependency and use it, so I'm not at all convinced that its worth it here.

Right, will drop the dependency and reimplement locally.

tnull · 2026-01-27T08:09:25Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


Hmm, is it customary to have such a score? I would find it very hard to interpret, tbh.?

Hmm, is it customary to have such a score?
I think it is as some users might want to rely on that to know how their node is performing per time at a glance.
I would find it very hard to interpret, tbh.?
The current computation is pretty basic and relies on the NodeStatus informations from ldk-node. Though, we might want to refine that to include more informations and decide the best weightage value to assign for each event.

I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

ldk-reviews-bot · 2026-01-29T02:00:16Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-31T02:01:04Z

🔔 2nd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-02T02:01:34Z

🔔 3rd Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-04T02:02:02Z

🔔 4th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-07T00:00:07Z

🔔 5th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-09T00:00:31Z

🔔 6th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-11T00:00:57Z

🔔 7th Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Anyitechs · 2026-02-13T14:24:52Z

This is ready for another look. Dropped the deps and reimplemented locally.

ldk-reviews-bot · 2026-02-14T00:01:00Z

🔔 8th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:01:00Z

🔔 1st Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:13Z

🔔 9th Reminder

Hey @tnull @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-02-16T13:12:38Z

+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+/// This represents a [`Metrics`] type that can go up and down in value.
+pub struct IntGauge {


Why do we need this extra newtype? Couldn't we just use a plain AtomicI64?

A plain AtomicI64 could work, but I introduce the type for better organization and to represent the Gauge metric type (this will help differentiate it from a Counter type when introduced later, which could also use a AtomicI64 type but are meant to only increase and not decrease, in the metrics world).
Happy to drop if it's too much boilerplate.

tnull · 2026-02-16T13:15:54Z

+ self.service_health_score.set(score);
+	}
+
+	/// The health score computation is pretty basic for now and simply


I guess it's okay as a first proof-of-concept, but IMO it would be much more useful to expose the actual underlying metrics such as peer/channel count, time since last successful chain sync/fee rate update, etc.

benthecarman

We should add the metric endpoint to the ldk-server-client as well. We don't really need a cli command for it, but would be worth at least putting in the client so we have 100% coverage

benthecarman · 2026-02-17T22:12:07Z

+ buffer
+	}
+
+	fn compute_health_score(is_running: bool, has_peers: bool, is_wallet_synced: bool) -> i64 {


This is such a weird health score. I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines

Lnd has a prometheus integration. Would be good to look at and see what they are exposing in theirs

This is such a weird health score.
The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I think it'd be better to give stats about the node (balance, num peers, num channels, etc) and let the user make their own guidelines
Right, I had earlier intended to do this in a follow-up. While this PR sets the structure/foundation, a follow-up will focus more on the metrics we need to expose.

The idea is to give users an indication on how their node is performing per time without having to look at other individual metrics. I've seen similar metric for other services, but will need to refine this the more.
I'm not entirely opposed to have such an aggregated metric (and we can still discuss what factors to include with what weight, etc), but I agree it only makes sense in addition to exposing the values it's based on.

benthecarman · 2026-02-17T22:14:52Z

+ runtime.spawn(async move {
+ loop {
+ interval.tick().await;
+ metrics_bg.update_service_health_score(&metrics_node);
+ }
+ });
+


instead of every minute updating this, we should be able to do it real time and just update it when we get a relevant event from ldk-node. ie we get a channel closed event so we update the metrics immediately to reflect that

I intend to maintain a hybrid approach for this, real time update for the Node events but still maintain the polling for metrics like channel/peer/payment count, balances, etc.

benthecarman · 2026-02-25T19:26:17Z

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {
+ let url = format!("https://{}/{GET_METRICS_PATH}", self.base_url);
+ let response = self.client.get(&url).send().await.map_err(|e| {


can we refactor post_request to do be able to do GET requests and use that. That has a lot of this logic already and would be better for future use

benthecarman · 2026-02-25T19:28:25Z

+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {
+ let metrics = Arc::clone(&self.metrics);
+ return Box::pin(async move {
+ Ok(Response::builder()
+ .header("Content-Type", "text/plain")
+ .body(Full::new(Bytes::from(metrics.gather_metrics())))
+ .unwrap())
+ });


We aren't validating auth here we are short cutting before its done. I'm not sure if that'll break the typical Prometheus flow though?

We aren't validating auth here we are short cutting before its done.
Yes, this is intentional because Prometheus does not support the HMAC auth scheme we use. It supports only basic auth and TLS.

benthecarman

Since this endpoint isn't authenticated and can't be because of the prometheus limitations, we should make it configurable and default off.

benthecarman · 2026-03-06T23:34:29Z

+ &mut buffer,
+ "ldk_server_total_channels_count",
+ "Total number of channels",
+ "counter",


this should be gauge because it can go down, not just up

benthecarman · 2026-03-06T23:34:37Z

+ &mut buffer,
+ "ldk_server_total_public_channels_count",
+ "Total number of public channels",
+ "counter",


benthecarman · 2026-03-06T23:34:45Z

+ &mut buffer,
+ "ldk_server_total_private_channels_count",
+ "Total number of private channels",
+ "counter",


benthecarman · 2026-03-06T23:36:13Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_successful_payments_count(&event_node);


this function does a full recount, we should be able to just increase by one

benthecarman · 2026-03-06T23:36:20Z

 Arc::clone(&event_publisher),
 Arc::clone(&paginated_store)).await;
+
+ event_metrics.update_total_failed_payments_count(&event_node);


benthecarman · 2026-03-06T23:37:42Z

+ self.update_peer_count(node);
+ self.update_total_payments_count(node);
+ self.update_total_successful_payments_count(node);
+ self.update_total_failed_payments_count(node);
+ self.update_total_channels_count(node);
+ self.update_total_public_channels_count(node);
+ self.update_total_private_channels_count(node);


For these we are making multiple calls to list all channels and payments. Would be better to just do that once and then count/filter as needed for each metric

benthecarman · 2026-03-06T23:40:07Z


 	fn call(&self, req: Request<Incoming>) -> Self::Future {
+ // Handle metrics endpoint separately to bypass auth and return plain text
+ if req.uri().path().len() > 1 && &req.uri().path()[1..] == GET_METRICS_PATH {


should also require the request is a GET request

benthecarman · 2026-03-06T23:40:49Z

 	}

+	/// Retrieve the node metrics in Prometheus format.
+	pub async fn get_metrics(&self) -> Result<String, LdkServerError> {


This is just a raw string, really should be decoded into the Response type

really should be decoded into the Response type
The Response type is protobuf, but Promotheus scrapers needs the endpoint to return plain-text

benthecarman · 2026-03-06T23:41:32Z

+ RequestType::Post => self.client.post(url),
+ };
+
+ let body_for_auth = body.as_deref().unwrap_or(&[]);


nit: can move this into the if authenticated arm

benthecarman · 2026-03-06T23:42:57Z

+
+pub const BUILD_METRICS_INTERVAL: Duration = Duration::from_secs(60);
+
+pub struct Metrics {


Can we add some docs about how this is used for Prometheus? There's a lot of logic here that is specific to it without any context

Anyitechs · 2026-03-19T01:22:42Z

Improved the metrics e2e test and rebased to fix conflicts. Ready for another look!

benthecarman · 2026-03-19T02:20:13Z

+
+	// Set up channel and make a payment to trigger metric update
+	setup_funded_channel(&bitcoind, &server_a, &server_b, 100_000).await;
+


We should be able to check that the channel (private/public) count, our LN balance, and num peers, goes up after opening a channel. It also sends funds to the nodes for anchor channels so we should be able to verify our on-chain balance numbers change.

Can we check the metrics after the channel open but before the receive

Can we also do a receive and check the metrics after that.

Can we check the metrics after the channel open but before the receive
We'll only be able to check the channels count metric as that is the only metric dependent on the ChannelReady event. But we can check other metrics if we poll.
Can we also do a receive and check the metrics after that.
This is what we're doing right now.

Anyitechs · 2026-03-20T02:25:06Z

Addressed comment and rebased/squashed. Ready for another look!

benthecarman · 2026-03-20T03:10:49Z

+	// We need to poll here because the metrics below are poll-based and
+	// updates every 60 seconds.
+	let timeout = Duration::from_secs(65);


We can't really do this. Are these not updated from the events? If we still need this even with the events updating. We should just make the interval configurable and make it like every 1 sec for the tests.

Are these not updated from the events?
No, the events are for payment-related actions that we emit events for. We do not emit events for the metrics covered there, so we rely on the node to get those information. Because of this, we need to poll to ensure the data is always accurate.

benthecarman · 2026-03-20T03:36:49Z

 Arc::clone(&paginated_store)).await;
+
+ if let Some(metrics) = &metrics {
+ metrics.update_payments_count(true);


we should be able to update our balance metric here and the payment received one

We should also update it with the channel events and their corresponding metrics

we should be able to update our balance metric here and the payment received one
Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information? Same with the channel events

Not sure how this will look, do we want to build/maintain a local balance value based on the payment received event, rather than rely on the node information?
Should be able to just use your Metrics::update_all_balances here
Same with the channel events
channel counts we can just increase/decrease like we do with the payment counts

ldk-reviews-bot · 2026-03-23T00:00:43Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman · 2026-03-23T22:14:55Z

+ help = "The polling interval for metrics in seconds. Required when
+ metrics is enabled, but defaults to 60secs if unset."
+	)]
+	poll_metrics_interval: Option<u64>,


okay one last thing, now that we have metrics configs, can we add an option to do the basic auth, that way we don't always have an unautheticated version

Done here d4e5ea2

Anyitechs · 2026-03-27T19:36:05Z

Please excuse the delay here, wasn't feeling too great earlier.

Rebased to fix conflicts and addressed the comment on adding basic auth here d4e5ea2.

ldk-reviews-bot · 2026-03-30T00:01:26Z

🔔 1st Reminder

Hey @benthecarman! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

benthecarman

one nit, but lgtm on squash!

benthecarman · 2026-03-30T17:29:51Z

+{}
 "#,
+ metrics_auth_config,


nit: can you just put {metrics_auth_config} like we do with all the other variables

This introduces the foundational telemetry infrastructure to improve the observability of LDK Server. It adds a new `/metrics` endpoint exposed on the REST service address, which serves Prometheus-compatible metrics. This endpoint is public and does not require HMAC authentication, allowing for easy integration with monitoring systems. - Added a `Metrics` utility struct to hold all the metrics we need to expose. - Made `Metrics` configurable via config. - Added Prometheus basic auth support. This is the first step in a larger effort to provide comprehensive telemetry. Future updates will expand this to include other detailed metrics for channels, balances, payments, etc.

Anyitechs · 2026-03-30T21:44:17Z

one nit, but lgtm on squash!

Thanks for the review. Addressed comment and rebased with no further changes.

…etry

Anyitechs force-pushed the introduce-telemetry branch 2 times, most recently from 8692c8c to 56b5e4aCompare January 27, 2026 01:51

Anyitechs marked this pull request as ready for review January 27, 2026 01:59

ldk-reviews-bot requested a review from benthecarman January 27, 2026 01:59

tnull reviewed Jan 27, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch 2 times, most recently from 6bcc9f8 to 1563d7cCompare February 13, 2026 14:04

Anyitechs requested a review from tnull February 13, 2026 14:25

tnull reviewed Feb 16, 2026
View reviewed changes

benthecarman reviewed Feb 17, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch 2 times, most recently from 262569f to 226b86cCompare February 25, 2026 00:41

Anyitechs requested a review from benthecarman February 25, 2026 00:49

benthecarman reviewed Feb 25, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch 2 times, most recently from 1c27292 to 432780cCompare March 6, 2026 15:21

benthecarman requested changes Mar 6, 2026
View reviewed changes

Anyitechs requested a review from benthecarman March 19, 2026 01:23

benthecarman reviewed Mar 19, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch from 0efd538 to 6856ef9Compare March 20, 2026 02:09

Anyitechs requested a review from benthecarman March 20, 2026 02:25

benthecarman reviewed Mar 20, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch 2 times, most recently from d4e4f38 to c1edca2Compare March 20, 2026 05:42

Anyitechs requested a review from benthecarman March 20, 2026 05:57

benthecarman linked an issue Mar 20, 2026 that may be closed by this pull request
Telemetry #38
Closed

Anyitechs force-pushed the introduce-telemetry branch 4 times, most recently from d2b00f3 to 23e2f7bCompare March 21, 2026 23:10

benthecarman reviewed Mar 23, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch 2 times, most recently from be0400e to d4e5ea2Compare March 27, 2026 19:22

Anyitechs requested a review from benthecarman March 27, 2026 19:36

benthecarman reviewed Mar 30, 2026
View reviewed changes

Anyitechs force-pushed the introduce-telemetry branch from d4e5ea2 to cc3a55aCompare March 30, 2026 21:40

benthecarman approved these changes Mar 31, 2026
View reviewed changes

benthecarman merged commit 7431487 into lightningdevkit:mainMar 31, 2026
8 checks passed

rsafier pushed a commit to rsafier/ldk-server that referenced this pull request Apr 20, 2026
Merge pull requestlightningdevkit#117from Anyitechs/introduce-telem…
fb4338f
…etry