Merge pull request #2376 from derekpierre/be-cool-prometheus

Be cool prometheus...just be cool

docs/source/guides/network_node/ursula_configuration_guide.rst

@@ -218,6 +218,10 @@ parameters to the ``nucypher ursula run`` command:
 The corresponding endpoint, ``http://<node_ip>:<METRICS PORT>/metrics``, can be used as a Prometheus data source for
 monitoring including the creation of alert criteria.
 
+By default metrics will be collected every 90 seconds but this can be modified using the ``--metrics-interval`` option.
+Collection of metrics will increase the number of RPC requests made to your web3 endpoint; increasing the frequency
+of metrics collection will further increase this number.
+
 During the Technical Contributor Phase of our testnet, *P2P Validator*
 contributed a `self-hosted node monitoring suite <https://economy.p2p.org/nucypher-worker-node-monitoring-suite/amp/>`_
 that uses a Grafana dashboard to visualize and monitor the metrics produced by the prometheus endpoint.

nucypher/cli/commands/ursula.py

@@ -366,7 +366,8 @@ def forget(general_config, config_options, config_file):
 @click.option('--metrics-port', help="Run a Prometheus metrics exporter on specified HTTP port", type=NETWORK_PORT)
 @click.option("--metrics-listen-address", help="Run a prometheus metrics exporter on specified IP address", default='')
 @click.option("--metrics-prefix", help="Create metrics params with specified prefix", default="ursula")
-def run(general_config, character_options, config_file, interactive, dry_run, metrics_port, metrics_listen_address, metrics_prefix, prometheus):
+@click.option("--metrics-interval", help="The frequency of metrics collection", type=click.INT, default=90)
+def run(general_config, character_options, config_file, interactive, dry_run, prometheus, metrics_port, metrics_listen_address, metrics_prefix, metrics_interval):
     """Run an "Ursula" node."""
 
     worker_address = character_options.config_options.worker_address
@@ -393,7 +394,8 @@ def run(general_config, character_options, config_file, interactive, dry_run, me
         from nucypher.utilities.prometheus.metrics import PrometheusMetricsConfig
         prometheus_config = PrometheusMetricsConfig(port=metrics_port,
                                                     metrics_prefix=metrics_prefix,
-                                                    listen_address=metrics_listen_address)
+                                                    listen_address=metrics_listen_address,
+                                                    collection_interval=metrics_interval)
 
     # TODO should we just not call run at all for "dry_run"
     try:

nucypher/utilities/prometheus/collector.py

@@ -33,10 +33,9 @@ from nucypher.blockchain.eth.registry import BaseContractRegistry
 from nucypher.datastore.datastore import RecordNotFound
 from nucypher.datastore.models import Workorder, PolicyArrangement
 
-from prometheus_client.metrics import MetricWrapperBase
 from prometheus_client.registry import CollectorRegistry
 
-from typing import Dict, List, Union
+from typing import Dict, Union
 
 ContractAgents = Union[StakingEscrowAgent, WorkLockAgent, PolicyManagerAgent]
 
@@ -434,63 +433,19 @@ class WorkerBondedEventMetricsCollector(EventMetricsCollector):
             self.contract_agent.get_worker_from_staker(self.staker_address) == self.worker_address)
 
 
-class BidRefundCompositeEventMetricsCollector(MetricsCollector):
-    """
-    Collector for both Bid and Refund WorkLock events.
+class WorkLockRefundEventMetricsCollector(EventMetricsCollector):
+    """Collector for WorkLock Refund event."""
 
-    Both Bid and Refund events additionally update the same metric of how much eth is deposited by the staker
-    so they are combined into one overall collector.
-    """
-    COMMON_METRIC_KEY = "worklock_deposited_eth_gauge"
-
-    class BidRefundCommonCollector(EventMetricsCollector):
-        """Configurable and generalized event metric collector applicable to both Bid and Refund events."""
-        def __init__(self, staker_address: ChecksumAddress, *args, **kwargs):
-            super().__init__(*args, **kwargs)
-            self.staker_address = staker_address
-
-        def add_common_metric(self, metric: MetricWrapperBase):
-            self.metrics[BidRefundCompositeEventMetricsCollector.COMMON_METRIC_KEY] = metric
-
-        def _event_occurred(self, event) -> None:
-            super()._event_occurred(event)
-            self.metrics[BidRefundCompositeEventMetricsCollector.COMMON_METRIC_KEY].set(
-                self.contract_agent.get_deposited_eth(self.staker_address))
-
-    def __init__(self, staker_address: ChecksumAddress, contract_registry: BaseContractRegistry, metrics_prefix: str):
-        # Bid/Refund (Modify the same metric)
-        worklock_agent = ContractAgency.get_agent(WorkLockAgent, registry=contract_registry)
-
-        self.collectors: List[BidRefundCompositeEventMetricsCollector.BidRefundCommonCollector] = [
-            # Bid Events
-            self.BidRefundCommonCollector(
-                event_name='Bid',
-                event_args_config={
-                    "depositedETH": (Gauge, f'{metrics_prefix}_worklock_bid_depositedETH', 'Deposited ETH value')
-                },
-                argument_filters={"sender": staker_address},
-                staker_address=staker_address,
-                contract_agent=worklock_agent),
-            # Refund Events
-            self.BidRefundCommonCollector(
-                event_name='Refund',
-                event_args_config={
-                    "refundETH": (Gauge, f'{metrics_prefix}_worklock_refund_refundETH', 'Refunded ETH')
-                },
-                argument_filters={"sender": staker_address},
-                staker_address=staker_address,
-                contract_agent=worklock_agent)
-        ]
+    def __init__(self, staker_address: ChecksumAddress, event_name: str = 'Refund', *args, **kwargs):
+        super().__init__(event_name=event_name, argument_filters={'sender': staker_address}, *args, **kwargs)
+        self.staker_address = staker_address
 
     def initialize(self, metrics_prefix: str, registry: CollectorRegistry) -> None:
-        common_gauge = Gauge(f'{metrics_prefix}_worklock_current_deposited_eth',
-                             'Worklock deposited ETH',
-                             registry=registry)
-        for collector in self.collectors:
-            collector.initialize(metrics_prefix, registry)
-            # manually add common gauge to both collectors' dictionary of metrics
-            collector.add_common_metric(common_gauge)
+        super().initialize(metrics_prefix=metrics_prefix, registry=registry)
+        self.metrics["worklock_deposited_eth_gauge"] = Gauge(f'{metrics_prefix}_worklock_current_deposited_eth',
+                                                             'Worklock deposited ETH',
+                                                             registry=registry)
 
-    def collect(self) -> None:
-        for collector in self.collectors:
-            collector.collect()
+    def _event_occurred(self, event) -> None:
+        super()._event_occurred(event)
+        self.metrics["worklock_deposited_eth_gauge"].set(self.contract_agent.get_deposited_eth(self.staker_address))

nucypher/utilities/prometheus/metrics.py

@@ -41,15 +41,15 @@ from nucypher.utilities.prometheus.collector import (
     ReStakeEventMetricsCollector,
     WindDownEventMetricsCollector,
     WorkerBondedEventMetricsCollector,
-    BidRefundCompositeEventMetricsCollector,
-    CommitmentMadeEventMetricsCollector)
+    CommitmentMadeEventMetricsCollector,
+    WorkLockRefundEventMetricsCollector)
 
 from typing import List
 
 from twisted.internet import reactor, task
 from twisted.web.resource import Resource
 
-from nucypher.blockchain.eth.agents import ContractAgency, StakingEscrowAgent, WorkLockAgent, PolicyManagerAgent
+from nucypher.blockchain.eth.agents import ContractAgency, StakingEscrowAgent, PolicyManagerAgent, WorkLockAgent
 
 
 class PrometheusMetricsConfig:
@@ -58,7 +58,7 @@ class PrometheusMetricsConfig:
                  port: int,
                  metrics_prefix: str,
                  listen_address: str = '',  # default to localhost ip
-                 collection_interval: int = 10,
+                 collection_interval: int = 90,  # every 1.5 minutes
                  start_now: bool = False):
 
         if not port:
@@ -295,29 +295,15 @@ def create_worklock_events_metric_collectors(ursula: 'Ursula', metrics_prefix: s
     worklock_agent = ContractAgency.get_agent(WorkLockAgent, registry=ursula.registry)
     staker_address = ursula.checksum_address
 
-    # Deposited\
-    collectors.append(EventMetricsCollector(
-        event_name='Deposited',
+    # Refund
+    collectors.append(WorkLockRefundEventMetricsCollector(
         event_args_config={
-            "value": (Gauge, f'{metrics_prefix}_worklock_deposited_value', 'Deposited value')
+            "refundETH": (Gauge, f'{metrics_prefix}_worklock_refund_refundETH',
+                          'Refunded ETH'),
         },
-        argument_filters={"sender": staker_address},
-        contract_agent=worklock_agent))
-
-    # Claimed
-    collectors.append(EventMetricsCollector(
-        event_name='Claimed',
-        event_args_config={
-            "claimedTokens": (Gauge, f'{metrics_prefix}_worklock_claimed_claimedTokens', 'Claimed tokens value')
-        },
-        argument_filters={"sender": staker_address},
-        contract_agent=worklock_agent))
-
-    # Bid/Refund (Modify a common metric)
-    collectors.append(BidRefundCompositeEventMetricsCollector(
         staker_address=staker_address,
-        contract_registry=ursula.registry,
-        metrics_prefix=metrics_prefix))
+        contract_agent=worklock_agent,
+    ))
 
     return collectors
 

tests/unit/test_prometheus.py

@@ -62,7 +62,7 @@ def test_prometheus_metrics_config():
     assert prometheus_config.listen_address == ''
 
     # defaults
-    assert prometheus_config.collection_interval == 10
+    assert prometheus_config.collection_interval == 90
     assert not prometheus_config.start_now
     assert prometheus_config.listen_address == ''