readme fixes, small improvements and todos

Author: gistart

README.md

@@ -1,19 +1,19 @@
-[![GitHub Workflow Status](https://img.shields.io/github/workflow/status/gistart/prometheus-push-client/test-all)](https://github.com/gistart/prometheus-push-client/actions)
-[![Codecov](https://img.shields.io/codecov/c/github/gistart/prometheus-push-client)](https://codecov.io/gh/gistart/prometheus-push-client)
+[![test-all](https://github.com/gistart/prometheus-push-client/actions/workflows/tests.yml/badge.svg?branch=master)](https://github.com/gistart/prometheus-push-client/actions)
+[![codecov](https://codecov.io/gh/gistart/prometheus-push-client/branch/master/graph/badge.svg?token=6K4G8CDU2R)](https://codecov.io/gh/gistart/prometheus-push-client)
 
 # prometheus-push-client
 
-Push metrics from your regular and/or long-running jobs into existing Prometheus/VictoriaMetrics monitoring system.
+Push metrics from your regular and/or long-running jobs to existing Prometheus/VictoriaMetrics monitoring system.
 
-:no-entry: not tested in real environment yet!
+:no_entry: not tested in real environment yet!
 
-Currently supports pushes directly into VictoriaMetrics via UDP and HTTP using InfluxDB line protocol as [described here](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html?highlight=telegraf#how-to-send-data-from-influxdb-compatible-agents-such-as-telegraf), and into StatsD/statsd-exporter in StatsD format via UDP ([not ideal](https://github.com/prometheus/statsd_exporter#with-statsd)).
+Currently supports pushes directly to VictoriaMetrics via UDP and HTTP using InfluxDB line protocol as [described here](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html?highlight=telegraf#how-to-send-data-from-influxdb-compatible-agents-such-as-telegraf), and into StatsD/statsd-exporter in StatsD format via UDP ([not ideal](https://github.com/prometheus/statsd_exporter#with-statsd)).
 
 ## Default labelvalues
 
-With regular prometheus_client we can define defaults for either none or all our labels with `labelvalues`, but that's not enough.
+With regular prometheus_client, defaults may be defined for either _none_ or _all_ the labels (with `labelvalues`), but that's not enough.
 
-We probably want to define some defaults, like `hostname`, or more importantly, **if we use VictoriaMetrics cluster** we need to push label `VictoriaMetrics_AccountID=<int>` (usually 1) or else our metrics will be ignored!
+We probably want to define _some_ defaults, like `hostname`, or more importantly, **if we use VictoriaMetrics cluster**, we always need to push label `VictoriaMetrics_AccountID=<int>` (usually 1) or else our metrics will be ignored.
 
 Following example shows how to use defaults, and how to override them if necessary.
 
@@ -40,16 +40,15 @@ counter1.labels("non-default", "login").inc()
 
 Metrics with no labels are initialized at creation time. This can have unpleasant side-effect: if we initialize lots of metrics not used in currently running job, background clients will have to push their non-changing values in every synchronization session.
 
-To avoid that we'll have to properly isolate each task's metrics, which can be impossible or rather tricky, or we can create metrics with default, non-changing labels (like `hostname`). Such metrics will initialize on fisrt use (inc), and we'll be pusing only metrics we actually used!
-
+To avoid that we'll have to properly isolate each task's metrics, which can be impossible or rather tricky, or we can create metrics with default, non-changing labels (like `hostname`). Such metrics will be initialized on fisrt use (inc), and we'll be pusing only those we actually used!
 
 ## Background clients
 
-Background clients spawn synchronization jobs "in background" (meaning in thread or asyncio task) to periodially send all metrics from `ppc.PUSH_REGISTRY` to the destination.
+Background clients spawn synchronization jobs "in background" (meaning in a thread or asyncio task) to periodically send all metrics from `ppc.PUSH_REGISTRY` to the destination.
 
-:warning: background clients will attempt to stop gracefully, syncronizing registry "for the last time" after our job exits or crashes. This _may_ mess up aggregation with sampling, I'm not sure yet.
+:warning: background clients will attempt to stop gracefully, synchronizing registry "one last time" after job exits or crashes. This _may_ mess up sampling aggregation, I'm not sure yet.
 
-Best way to use them -- via decorators. These clients are intended to be used with long running, but finite tasks, that could be spawned anywhere, therefor not easily accessible by the scraper. If that's not the case -- just use "passive mode" w/ the scaper)
+Best way to use them is via decorators. These clients are intended to be used with long running, but finite tasks, which could be spawned anywhere, therefor not easily accessible by the scraper. If that's not the case -- just use "passive mode" w/ the scraper instead.
 
 ``` python
 def influx_udp_async(host, port, period=15.0):
@@ -61,8 +60,9 @@ def statsd_udp_thread(host, port, period=15.0):
 ```
 
 Usage example:
+
 ```python
-import priometheus_push_client as ppc
+import prometheus_push_client as ppc
 
 
 req_hist = ppc.Histogram(
@@ -74,10 +74,9 @@ req_hist = ppc.Histogram(
 )
 
 
-
 @ppc.influx_udp_async("victoria.acme.inc.net", 9876, period=15)
 async def main(urls):
     # the job ...
     req_hist.labels(gethostname(url)).observe(response.elapsed)
     # ...
-```
+```

prometheus_push_client/clients/bg.py

@@ -1,10 +1,14 @@
 import asyncio
 import threading
 import time
+import logging
 
 from prometheus_push_client import compat
 
 
+log = logging.getLogger("prometheus.bg")
+
+
 class BackgroundClient:
 
     def __init__(self, format, transport, period=15.0, *args, **kwargs):
@@ -52,7 +56,7 @@ def run(self):
             try:
                 self.transport.push_all(samples_iter)
             except Exception:
-                pass  # TODO log?
+                log.error("push crashed", exc_info=True)
             period = self.period - (time.time() - ts_start)
 
 
@@ -86,5 +90,5 @@ async def run(self):
             try:
                 await self.transport.push_all(samples_iter)
             except Exception:
-                pass  # TODO: log?
+                log.error("push crashed", exc_info=True)
             period = self.period - (time.time() - ts_start)

prometheus_push_client/compat.py

@@ -3,11 +3,11 @@
 if hasattr(asyncio, "get_running_loop"):
     get_running_loop = asyncio.get_running_loop
 else:  # < 3.7, may be unsafe outside coroutines (running loop)
-    get_running_loop = asyncio.get_event_loop
+    get_running_loop = asyncio.get_event_loop  # pragma: no cover
 
 
 if hasattr(asyncio, "create_task"):
     create_task = asyncio.create_task
 else: # < 3.7, totally unsafe outside loop
-    def create_task(coro):
+    def create_task(coro):  # pragma: no cover
         return asyncio.get_event_loop().create_task(coro)

prometheus_push_client/transports/udp.py

@@ -42,6 +42,9 @@ def push_one(self, data):
         raise NotImplementedError()
 
 
+# TODO: crashes on creation time DNS errors -- retry?
+
+
 class SyncUdpTransport(BaseUdpTransport):
     def start(self):
         self.transport = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)

setup.py

@@ -1,19 +1,43 @@
 from distutils.core import setup
 
 
+# version
 with open("./prometheus_push_client/version.py") as fd:
     exec(fd.read())
 
+
+# description
 github_url = 'https://github.com/gistart/prometheus-push-client'
 
 readme_lines = []
 with open('README.md') as fd:
     readme_lines = filter(None, fd.read().splitlines())
-readme_lines = list(readme_lines)[:3]
+readme_lines = list(readme_lines)[:5]
 readme_lines.append('Read more at [github page](%s).' % github_url)
 readme = '\n\n'.join(readme_lines)
 
 
+# requirements
+extras_require = {}
+extras_require["http"] = [
+    "aiohttp<4",
+    "requests<3",
+]
+extras_require["test"] = [
+    "pytest",
+    "pytest-asyncio",
+    "pytest-cov",
+    "pytest-mock",
+    *extras_require["http"]
+]
+extras_require["dev"] = [
+    "ipython",
+    "twine",
+    *extras_require["test"]
+]
+
+
+# setup
 setup(
     name="prometheus_push_client",
     version=__version__,
@@ -40,17 +64,5 @@
     install_requires=[
         "prometheus_client>=0.4.0",
     ],
-    extras_require={
-        "http": [
-            "aiohttp<4",
-            "requests<3",
-        ],
-        "test": [
-            "pytest",
-            "pytest-asyncio",
-            "pytest-cov",
-            "aiohttp<4",
-            "requests<3",
-        ],
-    }
+    extras_require=extras_require,
 )

test/test_offline/test_misc.py

@@ -0,0 +1,61 @@
+import pytest
+import sys
+import time
+import asyncio
+import logging
+
+import prometheus_push_client as ppc
+from prometheus_push_client import compat
+
+
+def test_missing_requests(mocker):
+    mocker.patch("prometheus_push_client.transports.http.requests", None)
+    with pytest.raises(RuntimeError):
+        ppc.SyncHttpTransport("")
+
+
+def test_missing_aiohttp(mocker):
+    mocker.patch("prometheus_push_client.transports.http.aiohttp", None)
+    with pytest.raises(RuntimeError):
+        ppc.AioHttpTransport("")
+
+
+def test_thread_client_transport_crash(mocker, caplog):
+    mocker.patch(
+        "prometheus_push_client.transports.udp.BaseUdpTransport.push_all",
+        new=lambda *args, **kwargs: 1 / 0
+    )
+
+    @ppc.influx_udp_thread("localhost", 9876, 0.1)
+    def _test():
+        for i in range(3):
+            time.sleep(0.1)
+
+    with caplog.at_level(logging.INFO):
+        _test()
+
+    assert any(
+        lr.name == "prometheus.bg" and lr.msg == "push crashed"
+        for lr in caplog.records
+    )
+
+
+@pytest.mark.asyncio
+async def test_thread_client_transport_crash(mocker, caplog):
+    mocker.patch(
+        "prometheus_push_client.transports.udp.BaseUdpTransport.push_all",
+        new=lambda *args, **kwargs: 1 / 0
+    )
+
+    @ppc.influx_udp_async("localhost", 9876, 0.1)
+    async def _test():
+        for i in range(3):
+            await asyncio.sleep(0.1)
+
+    with caplog.at_level(logging.INFO):
+        await _test()
+
+    assert any(
+        lr.name == "prometheus.bg" and lr.msg == "push crashed"
+        for lr in caplog.records
+    )