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 config option for disabling reporting #354

Merged
merged 13 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
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
11 changes: 11 additions & 0 deletions charmcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,14 @@ config:
automatically deduced from it).
See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
type: string
reporting_enabled:
description: |
When disabled, Grafana will be configured to not send anonymous usage statistics to stats.grafana.org, nor
periodically check for updates.
It is very helpful to the Grafana project, so please leave this enabled.

When enabled, Grafana will use its default values for analytics.

Ref: https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#analytics
type: boolean
default: true
4 changes: 1 addition & 3 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
jinja2 < 3
lightkube >= 0.11
markupsafe == 2.0.1
# pinned to 2.16 as 2.17 breaks our unittests
ops == 2.16
ops
pyyaml
urllib3
jsonschema
Expand Down
23 changes: 22 additions & 1 deletion src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ def _generate_grafana_config(self) -> str:
can be set in ENV variables, but leave for expansion later so we can
hide auth secrets
"""
configs = [self._generate_tracing_config()]
configs = [self._generate_tracing_config(), self._generate_analytics_config()]
if self.has_db:
configs.append(self._generate_database_config())
else:
Expand Down Expand Up @@ -799,6 +799,27 @@ def _generate_tracing_config(self) -> str:
ret = data.getvalue()
return ret

def _generate_analytics_config(self) -> str:
"""Generate analytics configuration.

Returns:
A string containing the analytics config to be stubbed into the config file.
"""
if self.config["reporting_enabled"]:
return ""
config_ini = configparser.ConfigParser()
# Ref: https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#analytics
config_ini["analytics"] = {
"reporting_enabled": "false",
"check_for_updates": "false",
"check_for_plugin_updates": "false",
}

data = StringIO()
config_ini.write(data)
ret = data.getvalue()
return ret

def _generate_database_config(self) -> str:
"""Generate a database configuration.

Expand Down
2 changes: 1 addition & 1 deletion tests/scenario/conftest.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from unittest.mock import patch

import pytest
from scenario import Context
from ops.testing import Context

from charm import GrafanaCharm

Expand Down
21 changes: 0 additions & 21 deletions tests/scenario/test_charm.py

This file was deleted.

49 changes: 49 additions & 0 deletions tests/scenario/test_config_reporting_enabled.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
from ops.testing import State, Container
from configparser import ConfigParser


containers = [
Container(name="grafana", can_connect=True),
Container(name="litestream", can_connect=True),
]


def test_reporting_enabled(ctx):
# GIVEN the "reporting_enabled" config option is set to True
state = State(leader=True, config={"reporting_enabled": True}, containers=containers)

# WHEN config-changed fires
out = ctx.run(ctx.on.config_changed(), state)

# THEN the config file is written WITHOUT the [analytics] section being rendered
simulated_pebble_filesystem = out.get_container("grafana").get_filesystem(ctx)
grafana_config_path = simulated_pebble_filesystem / "etc/grafana/grafana-config.ini"

config = ConfigParser()
config.read(grafana_config_path)
assert "analytics" not in config


def test_reporting_disabled(ctx):
# GIVEN the "reporting_enabled" config option is set to False
state = State(leader=True, config={"reporting_enabled": False}, containers=containers)

# WHEN config-changed fires
out = ctx.run(ctx.on.config_changed(), state)

# THEN the config file is written WITH the [analytics] section being rendered
simulated_pebble_filesystem = out.get_container("grafana").get_filesystem(ctx)
grafana_config_path = simulated_pebble_filesystem / "etc/grafana/grafana-config.ini"

config = ConfigParser()
config.read(grafana_config_path)
assert "analytics" in config
assert dict(config["analytics"]) == {
"reporting_enabled": "false",
"check_for_updates": "false",
"check_for_plugin_updates": "false",
}

# AND the "grafana" service is restarted
# TODO Does it make sense to check this if the charm under test's lifetime is only for the config-changed?
# TODO How to assert this?
4 changes: 3 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ deps =
pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263
responses
cosl
ops-scenario<7.0.0
ops[testing]
-r{toxinidir}/requirements.txt
commands =
/usr/bin/env sh -c 'stat sqlite-static > /dev/null 2>&1 || curl -L https://github.com/CompuRoot/static-sqlite3/releases/latest/download/sqlite3 -o sqlite-static && chmod +x sqlite-static'
Expand All @@ -105,6 +105,8 @@ deps =
asyncstdlib
# Libjuju needs to track the juju version
juju<=3.3.0,>=3.0
# https://github.com/juju/python-libjuju/issues/1184
websockets<14.0
pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263
pytest-operator
pytest-playwright
Expand Down
Loading