From d760d62564c64ae168d5ccd1800022b69ff7bb4a Mon Sep 17 00:00:00 2001 From: Eric Sauer Date: Wed, 16 Sep 2020 16:11:34 -0400 Subject: [PATCH] Change lead time calculation so that we can actually sort by application (#190) * Give commit_timestamp and deploy_timestamp a compatible app name * make log messages the same * Update lead time metric to actually sort by app name * Add commit hash to commit_timestamp metric * Fix typo * Add a prometheus rules unit test that proves that the current by_commit rule is broken * Update expression so tests pass * Add simple test script for prometheus rules * Add a test script and github workflow to test in PRs * Fix a command: * Fix rule * Add by_app and global metrics back into the mix * Update dashboard to use lead_time metric * Change metric to app label value --- .github/workflows/prometheus-rules.yml | 16 ++++ .gitignore | 3 +- _test/prometheus/test.yaml | 96 +++++++++++++++++++ _test/test_prometheusrules | 11 +++ charts/pelorus/Chart.yaml | 4 +- .../pelorus/templates/metrics-dashboard.yaml | 2 +- .../pelorus/templates/prometheus-rules.yaml | 5 +- exporters/committime/collector_base.py | 13 +-- exporters/deploytime/app.py | 6 +- 9 files changed, 143 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/prometheus-rules.yml create mode 100644 _test/prometheus/test.yaml create mode 100755 _test/test_prometheusrules diff --git a/.github/workflows/prometheus-rules.yml b/.github/workflows/prometheus-rules.yml new file mode 100644 index 000000000..60803cbf7 --- /dev/null +++ b/.github/workflows/prometheus-rules.yml @@ -0,0 +1,16 @@ +name: Promteheus Rules Tests +on: [push, pull_request] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: '^1.13.1' # The Go version to download (if necessary) and use. + - run: | + go version + go get github.com/prometheus/prometheus/cmd/promtool + promtool --version + _test/test_prometheusrules \ No newline at end of file diff --git a/.gitignore b/.gitignore index 5581233e9..f6c483561 100644 --- a/.gitignore +++ b/.gitignore @@ -145,4 +145,5 @@ dmypy.json policy/ # BATS -_test/test_helper/ \ No newline at end of file +_test/test_helper/ +_test/prometheus/rules.yaml \ No newline at end of file diff --git a/_test/prometheus/test.yaml b/_test/prometheus/test.yaml new file mode 100644 index 000000000..249b89769 --- /dev/null +++ b/_test/prometheus/test.yaml @@ -0,0 +1,96 @@ +# This is the main input for unit testing. +# Only this file is passed as command line argument. + +rule_files: + - rules.yaml + +evaluation_interval: 1m + +tests: +# Test 1. +- interval: 1m + # Series data. + input_series: + - series: deploy_timestamp{app="basic-nginx", endpoint="http", exported_namespace="basic-nginx-prod", image_sha="sha256:a344eb5673e7e3faf871e7375762f1869c823c957aa126ce8f52033395f2b358", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1594759735' + - series: deploy_timestamp{app="basic-nginx", endpoint="http", exported_namespace="basic-nginx-dev", image_sha="sha256:9e1348e33d416072d6301e5dc35e2c54b8dc95772b0a89ff2036fa79b368864d", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1594761060' + - series: deploy_timestamp{app="basic-nginx", endpoint="http", exported_namespace="basic-nginx-prod", image_sha="sha256:3131dfa239ec3db1e876ebea141b9d92627a432ca94949c7a6197d05261b0f84", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1597679100' + - series: deploy_timestamp{app="basic-nginx", endpoint="http", exported_namespace="basic-nginx-prod", image_sha="sha256:c291840e51a8305a4ce0b8dda4fdc9a4512e557d203e506a35d937d549958dee", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1594164059' + - series: deploy_timestamp{app="basic-nginx", endpoint="http", exported_namespace="basic-nginx-prod", image_sha="sha256:37452860725ffee5fef1ceddb9936088dd8c84b0ebadd0b1017112d6ba99f829", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1594759265' + - series: deploy_timestamp{app="basic-nginx", endpoint="http", exported_namespace="basic-nginx-prod", image_sha="sha256:1e4ae2dd0ee1eba759669a179c8fc3bdb869c3ba3fd78e7a70788a2fcb775854", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1594759432' + - series: deploy_timestamp{app="kenwilli-basic-spring-boot", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-prod", image_sha="sha256:08ab5ef50c3fa51993e4d40b62533afe6619558aaba9e3a179c451b2676db7ed", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1591796089' + - series: deploy_timestamp{app="kenwilli-basic-spring-boot", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-prod", image_sha="sha256:46332b417660900361f7185830a4eb6d5ddc7e3002944ba26ed260e83f415197", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1591813199' + - series: deploy_timestamp{app="kenwilli-basic-spring-boot", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-prod", image_sha="sha256:382947b77566babcc7c6c55f50cb17469c7d068bc4b0cfbdcd4b8b33e091d133", instance="10.129.1.254:8080", job="deploytime-exporter", namespace="app-name-consistency", pod="deploytime-exporter-1-66w2p", service="deploytime-exporter"} + values: '1591814663' + - series: commit_timestamp{app="basic-nginx", commit="c8683ac2ae37cb9dd7c4c576747bf8f3d74c4f28", endpoint="http", exported_namespace="basic-nginx-build", image_sha="sha256:3131dfa239ec3db1e876ebea141b9d92627a432ca94949c7a6197d05261b0f84", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1594760961' + - series: commit_timestamp{app="basic-nginx", commit="b4db23e0d91699165e92ea38ba03ac277c778397", endpoint="http", exported_namespace="basic-nginx-build", image_sha="sha256:a344eb5673e7e3faf871e7375762f1869c823c957aa126ce8f52033395f2b358", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1594759624' + - series: commit_timestamp{app="basic-nginx", commit="800f7c667ca08a4b596dc710de084de934facdc8", endpoint="http", exported_namespace="basic-nginx-build", image_sha="sha256:748783c99fd6639005692c6a52e75ef30a79c4e28afd48a4b486384ec606c724", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1594759788' + - series: commit_timestamp{app="basic-nginx", commit="9421e2898111a38fd5b6a17d6c94c2e3569b5c2f", endpoint="http", exported_namespace="basic-nginx-build", image_sha="sha256:9e1348e33d416072d6301e5dc35e2c54b8dc95772b0a89ff2036fa79b368864d", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1594760778' + - series: commit_timestamp{app="basic-nginx", commit="c8683ac2ae37cb9dd7c4c576747bf8f3d74c4f28", endpoint="http", exported_namespace="basic-nginx-build", image_sha="sha256:d33162494e4be33ab9fa731591840be288dece7041c03d7543d461a1cf0b4b62", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1594760961' + - series: commit_timestamp{app="deploytime-exporter", commit="1403b8d93a8e3ad8a386f0f770bb16bf885b02fa", endpoint="http", exported_namespace="deploytime-api-fix", image_sha="sha256:3a1616402ae774f8f446211c283888542b7af94db00c1f555829a5fd8d163c8f", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1599071744' + - series: commit_timestamp{app="kenwilli-basic-spring-boot", commit="1a97f4bb5f05c9ef2e3791de01c02cb0610663e6", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-build", image_sha="sha256:92e61fcec3cbc9a5f75793140b3d6996a864b326b0954dd7efd692cb9d3f45af", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1590532194' + - series: commit_timestamp{app="kenwilli-basic-spring-boot", commit="ea67853d8f9b4f43400500f807d72cdfb5b0936d", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-build", image_sha="sha256:08ab5ef50c3fa51993e4d40b62533afe6619558aaba9e3a179c451b2676db7ed", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1590767500' + - series: commit_timestamp{app="kenwilli-basic-spring-boot", commit="ea67853d8f9b4f43400500f807d72cdfb5b0936d", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-build", image_sha="sha256:46332b417660900361f7185830a4eb6d5ddc7e3002944ba26ed260e83f415197", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1590767500' + - series: commit_timestamp{app="kenwilli-basic-spring-boot", commit="ea67853d8f9b4f43400500f807d72cdfb5b0936d", endpoint="http", exported_namespace="kenwilli-basic-spring-boot-build", image_sha="sha256:382947b77566babcc7c6c55f50cb17469c7d068bc4b0cfbdcd4b8b33e091d133", instance="10.130.0.85:8080", job="github-exporter", namespace="app-name-consistency", pod="github-exporter-3-pjmmj", service="github-exporter"} + values: '1590767500' + + # Unit tests for promql expressions. + promql_expr_test: + # Check that the expected number of deploy_timestamp entries are present + - expr: count(deploy_timestamp) + eval_time: 1m + exp_samples: + # Sample 1. + - labels: '{}' + value: 9 + # Check that the expected number of commit_timestamp entries are present + - expr: count(commit_timestamp) + eval_time: 1m + exp_samples: + - labels: '{}' + value: 10 + # Ensure expected values for lead time for app 1 + - expr: sdp:lead_time:by_commit{app="basic-nginx"} + eval_time: 1m + exp_samples: + - labels: 'sdp:lead_time:by_commit{app="basic-nginx", commit="b4db23e0d91699165e92ea38ba03ac277c778397"}' + value: 111 + - labels: 'sdp:lead_time:by_commit{app="basic-nginx", commit="9421e2898111a38fd5b6a17d6c94c2e3569b5c2f"}' + value: 282 + - labels: 'sdp:lead_time:by_commit{app="basic-nginx", commit="c8683ac2ae37cb9dd7c4c576747bf8f3d74c4f28"}' + value: 2918139 + # Ensure expected values for lead time for app 2 + - expr: sdp:lead_time:by_commit{app="kenwilli-basic-spring-boot"} + eval_time: 1m + exp_samples: + - labels: 'sdp:lead_time:by_commit{app="kenwilli-basic-spring-boot", commit="ea67853d8f9b4f43400500f807d72cdfb5b0936d"}' + value: 1028589 + # Check calculation of average lead time per application + - expr: sdp:lead_time:by_app + eval_time: 1m + exp_samples: + - labels: 'sdp:lead_time:by_app{app="basic-nginx"}' + value: 972844 + - labels: 'sdp:lead_time:by_app{app="kenwilli-basic-spring-boot"}' + value: 1028589 + - expr: sdp:lead_time:global + eval_time: 1m + exp_samples: + - labels: 'sdp:lead_time:global' + value: 1000716.5 diff --git a/_test/test_prometheusrules b/_test/test_prometheusrules new file mode 100755 index 000000000..0c06b7d19 --- /dev/null +++ b/_test/test_prometheusrules @@ -0,0 +1,11 @@ +#!/bin/bash +RULES_FILE="charts/pelorus/templates/prometheus-rules.yaml" +TESTS_DIR="_test/prometheus" + +# Capture prometheus rules file out to a new temporary file +# since its embedded in a CR. +sed -n '/groups:/,$p' ${RULES_FILE} > ${TESTS_DIR}/rules.yaml + +# Must have promtool installed +# https://github.com/prometheus/prometheus/releases +promtool test rules ${TESTS_DIR}/test.yaml \ No newline at end of file diff --git a/charts/pelorus/Chart.yaml b/charts/pelorus/Chart.yaml index 7d6e6470a..7d14a73fb 100644 --- a/charts/pelorus/Chart.yaml +++ b/charts/pelorus/Chart.yaml @@ -14,8 +14,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 1.2.4 +version: 1.2.5 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: v1.2.1+44.gd25159a +appVersion: 1.2.1+29.g943b806 diff --git a/charts/pelorus/templates/metrics-dashboard.yaml b/charts/pelorus/templates/metrics-dashboard.yaml index c470afc57..ff231f794 100644 --- a/charts/pelorus/templates/metrics-dashboard.yaml +++ b/charts/pelorus/templates/metrics-dashboard.yaml @@ -155,7 +155,7 @@ spec: "tableColumn": "", "targets": [ { - "expr": "avg (deploy_timestamp - on(image_sha) group_left github_commit_timestamp)", + "expr": "sdp:lead_time:global", "format": "time_series", "intervalFactor": 1, "refId": "A" diff --git a/charts/pelorus/templates/prometheus-rules.yaml b/charts/pelorus/templates/prometheus-rules.yaml index faf0c9242..a80f2e1f1 100644 --- a/charts/pelorus/templates/prometheus-rules.yaml +++ b/charts/pelorus/templates/prometheus-rules.yaml @@ -6,9 +6,12 @@ spec: groups: - name: software-delivery-performance rules: + - record: sdp:lead_time:by_commit + expr: > + min by (app, commit) (deploy_timestamp - on(app,image_sha) group_left(commit) commit_timestamp) - record: sdp:lead_time:by_app expr: > - deploy_timestamp - on(image_sha) group_left commit_timestamp + avg by (app) (sdp:lead_time:by_commit) - record: sdp:lead_time:global expr: > avg(sdp:lead_time:by_app) diff --git a/exporters/committime/collector_base.py b/exporters/committime/collector_base.py index 9c89b1095..8e7315ba3 100644 --- a/exporters/committime/collector_base.py +++ b/exporters/committime/collector_base.py @@ -28,18 +28,19 @@ def __init__(self, kube_client, username, token, namespaces, apps, collector_nam def collect(self): commit_metric = GaugeMetricFamily('commit_timestamp', - 'Commit timestamp', labels=['namespace', 'app', 'image_sha']) + 'Commit timestamp', labels=['namespace', 'app', 'commit', 'image_sha']) commit_metrics = self.generate_metrics() for my_metric in commit_metrics: - logging.info("Namespace: %s, App: %s, Build: %s, Timestamp: %s" + logging.info("Collected commit_timestamp{ namespace=%s, app=%s, commit=%s, image_sha=%s } %s" % ( my_metric.namespace, my_metric.name, - my_metric.build_name, + my_metric.commit_hash, + my_metric.image_hash, str(float(my_metric.commit_timestamp)) ) ) - commit_metric.add_metric([my_metric.namespace, my_metric.name, my_metric.image_hash], + commit_metric.add_metric([my_metric.namespace, my_metric.name, my_metric.commit_hash, my_metric.image_hash], my_metric.commit_timestamp) yield commit_metric @@ -61,7 +62,7 @@ def generate_metrics(self): apps = [] builds_by_app = {} app_label = pelorus.get_app_label() - logging.info("Searching for builds with label: %s in namespace: %s" % (app_label, namespace)) + logging.debug("Searching for builds with label: %s in namespace: %s" % (app_label, namespace)) v1_builds = self._kube_client.resources.get(api_version='build.openshift.io/v1', kind='Build') # only use builds that have the app label @@ -144,7 +145,7 @@ def get_metric_from_build(self, build, app, namespace, repo_url): metric.labels = json.loads(str(labels).replace("\'", "\"")) metric.commit_hash = commit_sha - metric.name = app + '-' + commit_sha + metric.name = app metric.commiter = build.spec.revision.git.author.name metric.image_location = build.status.outputDockerImageReference metric.image_hash = build.status.output.to.imageDigest diff --git a/exporters/deploytime/app.py b/exporters/deploytime/app.py index 5c31a2161..08a0bc541 100644 --- a/exporters/deploytime/app.py +++ b/exporters/deploytime/app.py @@ -26,7 +26,9 @@ def collect(self): metric = GaugeMetricFamily('deploy_timestamp', 'Deployment timestamp', labels=['namespace', 'app', 'image_sha']) metrics = generate_metrics(self._namespaces) for m in metrics: - logging.info("Namespace: %s, App: %s, Image: %s", m.namespace, m.name, m.image_sha) + logging.info("Collected deploy_timestamp{namespace=%s, app=%s, image=%s} %s" % + (m.namespace, m.name, m.image_sha, pelorus.convert_date_time_to_timestamp(m.deploy_time)) + ) metric.add_metric([m.namespace, m.name, m.image_sha, m.deploy_time], pelorus.convert_date_time_to_timestamp(m.deploy_time)) yield(metric) @@ -102,7 +104,7 @@ def generate_metrics(namespaces): # pod template for i in images: if i is not None: - metric = DeployTimeMetric(rc.metadata.name, namespace) + metric = DeployTimeMetric(rc.metadata.labels[pelorus.get_app_label()], namespace) metric.labels = rc.metadata.labels metric.deploy_time = rc.metadata.creationTimestamp metric.image_sha = i