Merge ~rharding/charm-grafana:add-mysql-datasource into ~prometheus-charmers/charm-grafana:master

[Richard Harding]

This change adds support for a mysql based data source.

This is needed to allow building grafana dashboards on top of MySQL data that is generated as part of Juju KPIs.

In order to do this the current grafana-source endpoints needs to be made more generic. There's work around for issues in the current interface where it auto prefixes things with http:// and pulls the IP address of the relation unit vs getting it fed to the system.

The idea is that I've got a charm that pulls log data and stores it parsed into a mysql db.

mysql -> db relation -> my application charmed -> feeds the data source to grafana for dashboards.

[Richard Harding (rharding)]

Current test to make this work:

juju deploy mysql
juju deploy cs:~rharding/gypsy-danger-1
juju relate mysql:db gypsy-danger
juju deploy grafana (this updated charm)
juju relate grafana:grafana-source gypsy-danger

Log into grafana and see the data source. Hitting the verify it works button should show it works.

https://usercontent.irccloud-cdn.com/file/3KOzJdGO/Selection_010.png

[Richard Harding (rharding)]

Also wanted to make sure the prometheus grafana-source relation was still in working order and validated that in a gce model:

https://usercontent.irccloud-cdn.com/file/fS9oUqzx/Selection_011.png

[Richard Harding (rharding)]

And finally both running together

https://usercontent.irccloud-cdn.com/file/mkVCD9HR/Selection_012.png

[Tom Haddon (mthaddon)]

Could we have a documentation update for this too explaining the feature?

review: Needs Fixing

[Jacek Nykis (jacekn)]

Thanks! It's a nice addition and I only had quick look but some early feedback and questions inline. We filed RT#105512 to review and test properly.

Could you clarify what the gypsy-danger charm is? Is it mysql charm fork? Or will it proxy to the DB?

My first instinct, without knowing what the gypsy-danger is, is that if you want to talk to the DB you should relate grafana to mysql.

[Richard Harding (rharding)]

Thanks for the initial review.

gypsy-danger is a python project to parse the apache log files from the charmstore that are tossed into swift looking for important juju kpi data. It then loads that data into several tables in a mysql database that then allows for building queries against the parsed data.

The python code project is: https://github.com/canonicalltd/gypsy-danger
and I started a charm that will eventually deploy and cron job the fetching of new apache logs from swift each day (updating the mysql database with new log data each day).

I've gotten this working with hand coding/deploying here at: kpi.jujugui.org where it talks to a mysql server and I've put the gypsy-danger python parser code on it's own server and so I'm working to charm this up.

If I build a new relation for mysql->grafana directly then it adds a second path to add a new grafana datasource. It also breaks the usability. My code needs a database to store parsed data from apache logs. It needs to be the same exact database with the same user/credential as grafana needs to build a dashboard upon. In this way the theory is that:

gypsy-danger needs a database to store data in. It requests from mysql a fresh db, username, password. MySQL provides that and gypsy-danger then starts to populate the database with tables and records of data. Then, gypsy-danger wants to have a visual front end of that data and sends the data source information to grafana for visualization.

In fact, one long term thing in my wishlist, is that gypsy-danger could actually send over a saved dashboard to grafana so that both the datasource and the dashboard are loaded and ready to go out of the box.

I hope that clears up what I'm trying to do with this. Let me know if I'm unclear or I've taken a dark path of wrongness that I should stay away from.

b558079... by Richard Harding on 2017-08-30

Rebase away the initial lint work

[Paul Collins (pjdc)]

A couple remarks inline. As well as this, I think it'd be valuable to have a Mojo spec to deploy grafana, etc., along with gypsy-danger and mysql and exercise the new grafana functionality so that we can CI it, although I don't know if that should be a blocker. There are a couple of likely-looking jobs on Mojo CI Jenkins, although neither passes currently:

https://jenkins.canonical.com/is-mojo-ci/job/live-is-charm-test-grafana-xenial/
https://jenkins.canonical.com/is-mojo-ci/job/live-is-prometheus/

review: Needs Fixing

[Jacek Nykis (jacekn)]

> Thanks for the initial review.

Thank you for your contribution!

> If I build a new relation for mysql->grafana directly then it adds a second
> path to add a new grafana datasource. It also breaks the usability. My code
> needs a database to store parsed data from apache logs. It needs to be the
> same exact database with the same user/credential as grafana needs to build a
> dashboard upon.

I see, if it needs to be the same DB then the way you want to do it is probably the only sensible one.

> I hope that clears up what I'm trying to do with this. Let me know if I'm
> unclear or I've taken a dark path of wrongness that I should stay away from.

Thanks, I think it's clear why direct grafana<->mysql relation will not work.

What I'd say though is that it's still worth moving some of the mysql source handling logic into the grafana-source interface. mysql is a valid grafana source and IMO it makes logical sense for the interface to handle it without need for grafana to mangle relation data.

[Richard Harding (rharding)]

New interfaces MP located here:
https://code.launchpad.net/~rharding/interface-grafana-source/+git/interface-grafana-source/+merge/330041

Will push an updated PR here that supports using that shortly.

18aa0c5... by Richard Harding on 2017-08-31

Use the updated interface layer

ec70312... by Richard Harding on 2017-08-31

Revert the status_set lint cleanup

5bc388d... by Richard Harding on 2017-08-31

More cleanup from the rebase

[Richard Harding (rharding)]

I've updated the interface and updated this MP removing much of the lint cleanup previously done.

I can look into mojo stuff there but that'll be a longer road to hoe as I've not gotten that setup and since they're not currently working...I'd prefer to add a test/support for gypsy-danger as part of something that was working to test current Grafana/Prometheus if required.

7fa684a... by Richard Harding on 2017-09-25

Add hook to trigger when relation broken to remove record of datasource

[Richard Harding (rharding)]

Updated the testing bundle to:

 juju deploy cs:~rharding/gypsy-danger-testing-2

This includes the changes that add the hook to properly remove the datasource when the relation is broken so that it can be readded a second time.

Tested with mysql and prometheus and verified the datasources are removed and re-add successfully.

This goes with the interfaces changes in:

https://code.launchpad.net/~rharding/interface-grafana-source/+git/interface-grafana-source/+merge/330041

[Jamon Camisso (jamon)]

Looks good to me, it is working well any way that I try it along with the grafana-source charm.

Just two tiny flake8 issues and it's ready to merge and promulgate to the charmstore.

review: Approve

e2c9dc0... by Richard Harding on 2017-10-18

Flake8 cleanup per reviewj

[Richard Harding (rharding)]

Updated the flake8 issues per review.

[Jamon Camisso (jamon)]

Ah so close, but when I go to merge this I see that upstream has progressed ahead a bit in terms of admin-password, google analytics etc.

Could you please rebase your branch against master and push again? Sorry about that, I didn't realize the upstream charm had those changes.

review: Needs Fixing

d78ca59... by Richard Harding on 2017-10-18

Merge branch 'master' of git+ssh://git.launchpad.net/grafana-charm into add-mysql-datasource

Unmerged commits

d78ca59... by Richard Harding on 2017-10-18

Merge branch 'master' of git+ssh://git.launchpad.net/grafana-charm into add-mysql-datasource

e2c9dc0... by Richard Harding on 2017-10-18

Flake8 cleanup per reviewj

7fa684a... by Richard Harding on 2017-09-25

Add hook to trigger when relation broken to remove record of datasource

5bc388d... by Richard Harding on 2017-08-31

More cleanup from the rebase

ec70312... by Richard Harding on 2017-08-31

Revert the status_set lint cleanup

18aa0c5... by Richard Harding on 2017-08-31

Use the updated interface layer

b558079... by Richard Harding on 2017-08-30

Rebase away the initial lint work

37eba3c... by Richard Harding on 2017-08-29

Fix the various cleanup required from the layer

031a16d... by Richard Harding on 2017-08-29

First stab at getting the mysql source to load into the db correctly

b44f4d5... by Richard Harding on 2017-08-28

Add first bit of mysql relation definition

diff --git a/reactive/grafana.py b/reactive/grafana.py
index 8b3b126..f63ee5a 100644
--- a/reactive/grafana.py
+++ b/reactive/grafana.py
@@ -1,19 +1,33 @@
-import six
-import os
+import base64
+import datetime
 import glob
 import hashlib
-import datetime
-import requests
 import json
-import subprocess
-import base64
+import os
+import requests
 import shutil
-from charmhelpers import fetch
-from charmhelpers.core import host, hookenv, unitdata
-from charmhelpers.core.templating import render
+import six
+import subprocess
+
 from charmhelpers.contrib.charmsupport import nrpe
-from charms.reactive import hook, remove_state, set_state, when, when_not
-from charms.reactive.helpers import any_file_changed, data_changed, is_state
+from charmhelpers.core import (
+    hookenv,
+    host,
+    unitdata,
+)
+from charmhelpers.core.templating import render
+from charmhelpers import fetch
+from charms.reactive.helpers import (
+    any_file_changed,
+    is_state,
+)
+from charms.reactive import (
+    hook,
+    remove_state,
+    set_state,
+    when,
+    when_not,
+)
 
 import time
 
@@ -284,20 +298,38 @@ def wipe_nrpe_checks():
 @when('grafana-source.available')
 def configure_sources(relation):
     sources = relation.datasources()
-    if not data_changed('grafana.sources', sources):
-        return
     for ds in sources:
         hookenv.log('Found datasource: {}'.format(str(ds)))
         # Ensure datasource is configured
         check_datasource(ds)
 
 
-@when('grafana.started')
-@when_not('grafana-source.available')
-def sources_gone():
-    # Last datasource gone, remove as needed
-    # TODO implementation
-    pass
+@hook('grafana-source-relation-departed')
+def sources_gone(relation):
+    hookenv.log('sources gone')
+    hookenv.log(relation)
+    # Update the datasources now with the new information that something is
+    # not available.
+    conv = relation.conversation()
+    ds = {
+        'service_name': conv.scope.split('/')[0],
+        'type': conv.get_remote('type'),
+        'url': conv.get_remote('url'),
+        'description': conv.get_remote('description')
+    }
+    hookenv.log('Removing datasource:')
+    hookenv.log(ds)
+
+    if ds:
+        conn = sqlite3.connect('/var/lib/grafana/grafana.db', timeout=30)
+        cur = conn.cursor()
+        cur.execute(
+            'DELETE FROM DATA_SOURCE WHERE  type=? AND url=?',
+            (ds['type'], ds['url'])
+        )
+        conn.commit()
+        cur.close()
+    relation.set_local = None
 
 
 @when('website.available')
@@ -352,14 +384,11 @@ def check_datasource(ds):
     #  'username': 'username,
     #  'password': 'password
     # }
-
     conn = sqlite3.connect('/var/lib/grafana/grafana.db', timeout=30)
     cur = conn.cursor()
     query = cur.execute('SELECT id, type, name, url, is_default FROM DATA_SOURCE')
     rows = query.fetchall()
     ds_name = '{} - {}'.format(ds['service_name'], ds['description'])
-    print(ds_name)
-    print(rows)
     for row in rows:
         if (row[1] == ds['type'] and row[2] == ds_name and row[3] == ds['url']):
             hookenv.log('Datasource already exist, updating: {}'.format(ds_name))
@@ -371,7 +400,6 @@ def check_datasource(ds):
             return
     hookenv.log('Adding new datasource: {}'.format(ds_name))
     stmt, values = generate_query(ds, 0)
-    print(stmt, values)
     cur.execute(stmt, values)
     conn.commit()
     conn.close()
@@ -379,15 +407,20 @@ def check_datasource(ds):
 
 def generate_query(ds, is_default, id=None):
     if not id:
-        stmt = 'INSERT INTO DATA_SOURCE (org_id, version, type, name' + \
-               ', access, url, is_default, created, updated, basic_auth'
-        if 'username' in ds and 'password' in ds:
-            stmt += ', basic_auth_user, basic_auth_password)' + \
-                    ' VALUES (?,?,?,?,?,?,?,?,?,?,?,?)'
-        else:
-            stmt += ') VALUES (?,?,?,?,?,?,?,?,?,?)'
+        fields = [
+            'org_id',
+            'version',
+            'type',
+            'name',
+            'access',
+            'url',
+            'is_default',
+            'created',
+            'updated',
+        ]
+
         dtime = datetime.datetime.today().strftime("%F %T")
-        values = (1,
+        values = [1,
                   0,
                   ds['type'],
                   '{} - {}'.format(ds['service_name'], ds['description']),
@@ -395,11 +428,51 @@ def generate_query(ds, is_default, id=None):
                   ds['url'],
                   is_default,
                   dtime,
-                  dtime)
-        if 'username' in ds and 'password' in ds:
-            values = values + (1, ds['username'], ds['password'])
+                  dtime]
+
+        # Used in connecting a mysql source
+        if 'database' in ds:
+            hookenv.log('Adding fields for the database')
+            fields.append('database')
+            values.append(ds['database'])
+
+        if ds['type'] == 'mysql':
+            # There's only one username/password on the interface to we coopt
+            # it for the mysql login in the mysql case
+            fields.append('user')
+            values.append(ds['username'])
+            fields.append('password')
+            values.append(ds['password'])
+
+        if 'username' in ds and 'password' in ds and ds['type'] != 'msyql':
+            # We use the username and password in a backward compatible way
+            # for basic_auth when it's not a mysql data source.
+            hookenv.log('Adding basic_auth info')
+            fields.append('basic_auth')
+            fields.append('basic_auth_user')
+            fields.append('basic_auth_password')
+            values.append(1)
+            values.append(ds['username'])
+            values.append(ds['password'])
         else:
-            values = values + (0,)
+            fields.append('basic_auth')
+            values.append(0)
+
+        values_placeholders = '?, ' * len(values)
+        # Remove the space [, ] at the end of the generated string.
+        values_placeholders = values_placeholders[:-2]
+
+        stmt = """
+         INSERT INTO DATA_SOURCE ({})
+         VALUES ({})
+        """.format(
+            ", ".join(fields),
+            values_placeholders
+        )
+
+        hookenv.log('Statement is: \n{}'.format(stmt))
+        hookenv.log(values)
+
     else:
         if 'username' in ds and 'password' in ds:
             stmt = 'UPDATE DATA_SOURCE SET basic_auth_user = ?, basic_auth_password = ?, basic_auth = 1'