Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prometheus sensor component #44508

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions homeassistant/components/prometheus/sensor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
"""Prometheus Sensor component."""
from datetime import timedelta
import logging
from typing import Union
from urllib.parse import urljoin

import voluptuous as vol

from homeassistant.components.sensor import PLATFORM_SCHEMA as SENSOR_PLATFORM_SCHEMA
from homeassistant.const import (
CONF_NAME,
CONF_UNIT_OF_MEASUREMENT,
CONF_URL,
STATE_PROBLEM,
STATE_UNKNOWN,
)
from homeassistant.helpers.aiohttp_client import async_get_clientsession
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.entity import Entity
from homeassistant.util import Throttle

DEFAULT_URL = "http://localhost:9090"
CONF_QUERIES = "queries"
CONF_EXPR = "expr"
MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=60)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be configurable as well, or is it hard to implement because it's passed to a decorator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I'm not sure if a decorator can access a member object.


_LOGGER = logging.getLogger(__name__)

_QUERY_SCHEMA = vol.Schema(
{
vol.Required(
CONF_NAME,
): cv.string,
mweinelt marked this conversation as resolved.
Show resolved Hide resolved
vol.Optional(CONF_UNIT_OF_MEASUREMENT): cv.string,
vol.Required(CONF_EXPR): cv.string,
}
)

PLATFORM_SCHEMA = SENSOR_PLATFORM_SCHEMA.extend(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what's idiomatic here, but I believe most integrations create one entity per each defined sensor, while in this case a single sensor definition will might result in several sensor entities created. Flattening the configuration structure might make it slightly more straightforward (even if a bit more verbose).

What I mean is something like this:

sensor:
  - platform: prometheus
    name: entropy
    query: min(node_entropy_available_bits{instance="localhost:9100"})
  - platform: prometheus
    name: free disk space
    url: http://localhost:9090
    query: min(node_filesystem_free_bytes{instance="localhost:9100", mountpoint="/"})
    unit_of_measurement: Bytes

Anyway, hopefully one of the Home Assistant core mainteners will weigh in and provide guidance here.

Copy link
Contributor Author

@mweinelt mweinelt Dec 24, 2020

Choose a reason for hiding this comment

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

Then you'd have to pass the url over and over again. The way I did it is basically how the influxdb sensor does it, so I think this is fine.

I guess you can still create multiple prometheus platforms for different prometheus instances though.

I think keeping it this way offers better usability.

Copy link
Member

Choose a reason for hiding this comment

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

We no longer allow new integrations to use a platform config section or old integrations to do changes to the platform config section. See https://github.com/home-assistant/architecture/blob/master/adr/0007-integration-config-yaml-structure.md.

Please refactor this integration to have the config schema in the base integration module and the config yaml section under the top level integration domain key.

{
vol.Optional(CONF_URL, default=DEFAULT_URL): cv.string,
vol.Required(CONF_QUERIES): [_QUERY_SCHEMA],
}
)


async def async_setup_platform(hass, config, async_add_entities, discovery_info=None):
"""Set up the sensor platform."""
session = async_get_clientsession(hass)

prometheus = Prometheus(config.get(CONF_URL), session)

sensors = []
for query in config.get("queries", dict()):
sensors.append(PrometheusSensor(prometheus, query))

async_add_entities(sensors, update_before_add=True)


class Prometheus:
Copy link
Member

Choose a reason for hiding this comment

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

Please extract the service specific interface code and data parsing to a standalone library published on PyPI.

https://developers.home-assistant.io/docs/creating_component_code_review#4-communication-with-devicesservices

"""Wrapper for Prometheus API Requests."""

def __init__(self, url: str, session) -> None:
"""Initialize the Prometheus API wrapper."""
self._session = session
self._url = urljoin(f"{url}/", "api/v1/query")

async def query(self, expr: str) -> Union[str, float]:
"""Query expression response."""
response = await self._session.get(self._url, params={"query": expr})
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you happy with the default request timeout (5 minutes) or should it be overriden or configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't looked into it. Should probably be somewhat related to the MIN_TIME_BETWEEN_UPDATES.

if response.status != 200:
_LOGGER.error(
"Unexpected HTTP status code %s for expression '%s'",
response.status,
expr,
)
return STATE_UNKNOWN

try:
result = (await response.json())["data"]["result"]
except (ValueError, KeyError) as error:
_LOGGER.error("Invalid query response: %s", error)
return STATE_UNKNOWN

if not result:
_LOGGER.error("Expression '%s' yielded no result", expr)
return STATE_PROBLEM
if len(result) > 1:
_LOGGER.error("Expression '%s' yielded multiple metrics", expr)
return STATE_PROBLEM

return result[0]["value"][1]


class PrometheusSensor(Entity):
"""Sensor entity representing the result of a PromQL expression."""

def __init__(self, prometheus: Prometheus, query: dict) -> None:
"""Initialize the sensor."""
self._name = query.get(CONF_NAME)
self._expr = query.get(CONF_EXPR)
self._unit_of_measurement = query.get(CONF_UNIT_OF_MEASUREMENT)
self._prometheus = prometheus
self._state = None

@property
def name(self) -> str:
"""Return the name of the sensor."""
return self._name

@property
def unit_of_measurement(self) -> str:
"""Return the unit of the of the expression result."""
return self._unit_of_measurement

@property
def state(self) -> float:
mweinelt marked this conversation as resolved.
Show resolved Hide resolved
"""Return the expression result."""
return self._state

@Throttle(MIN_TIME_BETWEEN_UPDATES)
async def async_update(self) -> None:
"""Update state by executing query."""
self._state = await self._prometheus.query(self._expr)