Skip to content

Commit

Permalink
Merge branch 'stackhpc/yoga' into upstream/yoga-2024-05-13
Browse files Browse the repository at this point in the history
  • Loading branch information
markgoddard authored May 13, 2024
2 parents 913f061 + f45727b commit 890f8c6
Show file tree
Hide file tree
Showing 14 changed files with 131 additions and 26 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* @stackhpc/openstack
12 changes: 12 additions & 0 deletions .github/workflows/tag-and-release.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
name: Tag & Release
'on':
push:
branches:
- stackhpc/yoga
permissions:
actions: read
contents: write
jobs:
tag-and-release:
uses: stackhpc/.github/.github/workflows/tag-and-release.yml@main
7 changes: 7 additions & 0 deletions .github/workflows/tox.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
name: Tox Continuous Integration
'on':
pull_request:
jobs:
tox:
uses: stackhpc/.github/.github/workflows/tox.yml@main
21 changes: 20 additions & 1 deletion cloudkitty/collector/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ def MetricDict(value):
# (NONE, NUMBOOL, NOTNUMBOOL, FLOOR, CEIL).
# Defaults to NONE
Required('mutate', default='NONE'):
In(['NONE', 'NUMBOOL', 'NOTNUMBOOL', 'FLOOR', 'CEIL']),
In(['NONE', 'NUMBOOL', 'NOTNUMBOOL', 'FLOOR', 'CEIL', 'MAP']),
# Map dict used if mutate == 'MAP'
Optional('mutate_map'): dict,
# Collector-specific args. Should be overriden by schema provided for
# the given collector
Optional('extra_args'): dict,
Expand Down Expand Up @@ -270,6 +272,22 @@ def check_duplicates(metric_name, metric):
return metric


def validate_map_mutator(metric_name, metric):
"""Validates MAP mutator"""
mutate = metric.get('mutate')
mutate_map = metric.get('mutate_map')

if mutate == 'MAP' and mutate_map is None:
raise InvalidConfiguration(
'Metric {} uses MAP mutator but mutate_map is missing: {}'.format(
metric_name, metric))

if mutate != 'MAP' and mutate_map is not None:
raise InvalidConfiguration(
'Metric {} not using MAP mutator but mutate_map is present: '
'{}'.format(metric_name, metric))


def validate_conf(conf):
"""Validates the provided configuration."""
collector = get_collector_without_invoke()
Expand All @@ -278,4 +296,5 @@ def validate_conf(conf):
if 'alt_name' not in metric.keys():
metric['alt_name'] = metric_name
check_duplicates(metric_name, metric)
validate_map_mutator(metric_name, metric)
return output
4 changes: 3 additions & 1 deletion cloudkitty/collector/gnocchi.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,9 @@ def _format_data(self, metconf, data, resources_info=None):
qty = data['measures']['measures']['aggregated'][0][2]
converted_qty = ck_utils.convert_unit(
qty, metconf['factor'], metconf['offset'])
mutated_qty = ck_utils.mutate(converted_qty, metconf['mutate'])
mutate_map = metconf.get('mutate_map')
mutated_qty = ck_utils.mutate(converted_qty, metconf['mutate'],
mutate_map=mutate_map)
return metadata, groupby, mutated_qty

def fetch_all(self, metric_name, start, end,
Expand Down
4 changes: 3 additions & 1 deletion cloudkitty/collector/monasca.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ def _format_data(self, metconf, data, resources_info=None):
qty = data['statistics'][0][1]
converted_qty = ck_utils.convert_unit(
qty, metconf['factor'], metconf['offset'])
mutated_qty = ck_utils.mutate(converted_qty, metconf['mutate'])
mutate_map = metconf.get('mutate_map')
mutated_qty = ck_utils.mutate(converted_qty, metconf['mutate'],
mutate_map=mutate_map)
return metadata, groupby, mutated_qty

def fetch_all(self, metric_name, start, end,
Expand Down
6 changes: 5 additions & 1 deletion cloudkitty/collector/prometheus.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ def _format_data(self, metric_name, scope_key, scope_id, start, end, data):
self.conf[metric_name]['factor'],
self.conf[metric_name]['offset'],
)
qty = ck_utils.mutate(qty, self.conf[metric_name]['mutate'])
mutate_map = self.conf[metric_name].get('mutate_map')
qty = ck_utils.mutate(qty, self.conf[metric_name]['mutate'],
mutate_map=mutate_map)

return metadata, groupby, qty

Expand Down Expand Up @@ -211,6 +213,8 @@ def fetch_all(self, metric_name, start, end, scope_id, q_filter=None):
if query_suffix:
query = "{0} {1}".format(query, query_suffix)

LOG.debug("Calling Prometheus with query: %s", query)

try:
res = self._conn.get_instant(
query,
Expand Down
16 changes: 7 additions & 9 deletions cloudkitty/storage/v2/elasticsearch/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,15 @@ def _build_composite(self, groupby):
sources = []
for elem in groupby:
if elem == 'type':
sources.append({'type': {'terms': {'field': 'type'}}})
sources.append({'type': {'terms': {'field': 'type.keyword'}}})
elif elem == 'time':
# Not doing a date_histogram aggregation because we don't know
# the period
sources.append({'begin': {'terms': {'field': 'start'}}})
sources.append({'end': {'terms': {'field': 'end'}}})
else:
sources.append({elem: {'terms': {'field': 'groupby.' + elem}}})
field = 'groupby.' + elem + '.keyword'
sources.append({elem: {'terms': {'field': field}}})

return {"sources": sources}

Expand Down Expand Up @@ -158,12 +159,9 @@ def put_mapping(self, mapping):
:rtype: requests.models.Response
"""
url = '/'.join(
(self._url, self._index_name, '_mapping', self._mapping_name))
# NOTE(peschk_l): This is done for compatibility with
# Elasticsearch 6 and 7.
param = {"include_type_name": "true"}
(self._url, self._index_name, self._mapping_name))
return self._req(
self._sess.put, url, json.dumps(mapping), param, deserialize=False)
self._sess.post, url, json.dumps(mapping), {}, deserialize=False)

def get_index(self):
"""Does a GET request against ES's index API.
Expand Down Expand Up @@ -228,7 +226,7 @@ def bulk_with_instruction(self, instruction, terms):
"""Does a POST request against ES's bulk API
The POST request will be done against
`/<index_name>/<mapping_name>/_bulk`
`/<index_name>/_bulk`
The instruction will be appended before each term. For example,
bulk_with_instruction('instr', ['one', 'two']) will produce::
Expand All @@ -249,7 +247,7 @@ def bulk_with_instruction(self, instruction, terms):
*[(instruction, json.dumps(term)) for term in terms]
)) + '\n'
url = '/'.join(
(self._url, self._index_name, self._mapping_name, '_bulk'))
(self._url, self._index_name, '_bulk'))
return self._req(self._sess.post, url, data, None, deserialize=False)

def bulk_index(self, terms):
Expand Down
23 changes: 23 additions & 0 deletions cloudkitty/tests/collectors/test_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,26 @@ def test_check_duplicates(self):
self.assertRaises(
collector.InvalidConfiguration,
collector.check_duplicates, metric_name, metric)

def test_validate_map_mutator(self):
data = copy.deepcopy(self.base_data)

# Check that validation succeeds when MAP mutator is not used
for metric_name, metric in data['metrics'].items():
collector.validate_map_mutator(metric_name, metric)

# Check that validation raises an exception when mutate_map is missing
for metric_name, metric in data['metrics'].items():
metric['mutate'] = 'MAP'
self.assertRaises(
collector.InvalidConfiguration,
collector.validate_map_mutator, metric_name, metric)

data = copy.deepcopy(self.base_data)
# Check that validation raises an exception when mutate_map is present
# but MAP mutator is not used
for metric_name, metric in data['metrics'].items():
metric['mutate_map'] = {}
self.assertRaises(
collector.InvalidConfiguration,
collector.validate_map_mutator, metric_name, metric)
14 changes: 7 additions & 7 deletions cloudkitty/tests/storage/v2/elasticsearch/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ def test_build_composite(self):
self.assertEqual(
self.client._build_composite(['one', 'type', 'two']),
{'sources': [
{'one': {'terms': {'field': 'groupby.one'}}},
{'type': {'terms': {'field': 'type'}}},
{'two': {'terms': {'field': 'groupby.two'}}},
{'one': {'terms': {'field': 'groupby.one.keyword'}}},
{'type': {'terms': {'field': 'type.keyword'}}},
{'two': {'terms': {'field': 'groupby.two.keyword'}}},
]},
)

Expand Down Expand Up @@ -186,9 +186,9 @@ def test_put_mapping(self):
with mock.patch.object(self.client, '_req') as rmock:
self.client.put_mapping(mapping)
rmock.assert_called_once_with(
self.client._sess.put,
'http://elasticsearch:9200/index_name/_mapping/test_mapping',
'{"a": "b"}', {'include_type_name': 'true'}, deserialize=False)
self.client._sess.post,
'http://elasticsearch:9200/index_name/test_mapping',
'{"a": "b"}', {}, deserialize=False)

def test_get_index(self):
with mock.patch.object(self.client, '_req') as rmock:
Expand Down Expand Up @@ -259,7 +259,7 @@ def test_bulk_with_instruction(self):
self.client.bulk_with_instruction(instruction, terms)
rmock.assert_called_once_with(
self.client._sess.post,
'http://elasticsearch:9200/index_name/test_mapping/_bulk',
'http://elasticsearch:9200/index_name/_bulk',
expected_data, None, deserialize=False)

def test_bulk_index(self):
Expand Down
10 changes: 8 additions & 2 deletions cloudkitty/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ def tempdir(**kwargs):
LOG.debug('Could not remove tmpdir: %s', e)


def mutate(value, mode='NONE'):
"""Mutate value according provided mode."""
def mutate(value, mode='NONE', mutate_map=None):
"""Mutate value according to provided mode."""

if mode == 'NUMBOOL':
return float(value != 0.0)
Expand All @@ -266,6 +266,12 @@ def mutate(value, mode='NONE'):
if mode == 'CEIL':
return math.ceil(value)

if mode == 'MAP':
ret = 0.0
if mutate_map is not None:
ret = mutate_map.get(value, 0.0)
return ret

return value


Expand Down
6 changes: 3 additions & 3 deletions devstack/plugin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,11 @@ function install_elasticsearch_fedora {
sudo yum localinstall -y ${opensearch_file}
}

function install_elasticsearch {
function install_opensearch {
if is_ubuntu; then
install_elasticsearch_ubuntu
install_opensearch_ubuntu
elif is_fedora; then
install_elasticsearch_fedora
install_opensearch_fedora
else
die $LINENO "Distribution must be Debian or Fedora-based"
fi
Expand Down
27 changes: 26 additions & 1 deletion doc/source/admin/configuration/collector.rst
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ Quantity mutation
~~~~~~~~~~~~~~~~~

It is also possible to mutate the collected qty with the ``mutate`` option.
Four values are accepted for this parameter:
Five values are accepted for this parameter:

* ``NONE``: This is the default. The collected data is not modifed.

Expand All @@ -190,6 +190,11 @@ Four values are accepted for this parameter:
* ``NOTNUMBOOL``: If the collected qty equals 0, set it to 1. Else, set it to
0.

* ``MAP``: Map arbritrary values to new values as defined through the
``mutate_map`` option (dictionary). If the value is not found in
``mutate_map``, set it to 0. If ``mutate_map`` is not defined or is empty,
all values are set to 0.

.. warning::

Quantity mutation is done **after** conversion. Example::
Expand Down Expand Up @@ -233,6 +238,26 @@ when the instance is in ACTIVE state but 4 if the instance is in ERROR state:
metadata:
- flavor_id
The ``MAP`` mutator is useful when multiple statuses should be billabled. For
example, the following Prometheus metric has a value of 0 when the instance is
in ACTIVE state, but operators may want to rate other non-zero states:

.. code-block:: yaml
metrics:
openstack_nova_server_status:
unit: instance
mutate: MAP
mutate_map:
0.0: 1.0 # ACTIVE
11.0: 1.0 # SHUTOFF
12.0: 1.0 # SUSPENDED
16.0: 1.0 # PAUSED
groupby:
- id
metadata:
- flavor_id
Display name
~~~~~~~~~~~~

Expand Down
6 changes: 6 additions & 0 deletions releasenotes/notes/map-mutator-632b8629c0482e94.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
features:
- |
Adds a ``MAP`` mutator to map arbitrary values to new values. This is
useful with metrics reporting resource status as their value, but multiple
statuses are billable.

0 comments on commit 890f8c6

Please sign in to comment.