From 4b0b438f00b6be7e5df019bde6233ad57b73ac15 Mon Sep 17 00:00:00 2001 From: Azfaar Qureshi Date: Fri, 20 Nov 2020 13:20:27 -0500 Subject: [PATCH] Adding param validation fix lint changes --- .../CHANGELOG.md | 4 +- .../prometheus_remote_write/__init__.py | 98 +++++++++++++++++-- .../test_prometheus_remote_write_exporter.py | 93 +++++++++++++++--- 3 files changed, 173 insertions(+), 22 deletions(-) diff --git a/exporter/opentelemetry-exporter-prometheus-remote-write/CHANGELOG.md b/exporter/opentelemetry-exporter-prometheus-remote-write/CHANGELOG.md index a57d13b97a..6e702a2bdf 100644 --- a/exporter/opentelemetry-exporter-prometheus-remote-write/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-prometheus-remote-write/CHANGELOG.md @@ -4,4 +4,6 @@ ## Initial Release - Prometheus Remote Write Exporter Setup - ((#180)[https://github.com/open-telemetry/opentelemetry-python-contrib/pull/180]) \ No newline at end of file + ((#180)[https://github.com/open-telemetry/opentelemetry-python-contrib/pull/180]) +- Add Exporter constructor validation methods + ((#206)[https://github.com/open-telemetry/opentelemetry-python-contrib/pull/206]) diff --git a/exporter/opentelemetry-exporter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py b/exporter/opentelemetry-exporter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py index b431ddd2d0..1f940d9501 100644 --- a/exporter/opentelemetry-exporter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py +++ b/exporter/opentelemetry-exporter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py @@ -33,20 +33,106 @@ class PrometheusRemoteWriteMetricsExporter(MetricsExporter): Args: endpoint: url where data will be sent (Required) basic_auth: username and password for authentication (Optional) - bearer_token: token used for authentication (Optional) - bearer_token_file: filepath to file containing authentication token (Optional) - headers: additional headers for remote write request (Optional + headers: additional headers for remote write request (Optional) + timeout: timeout for requests to the remote write endpoint in seconds (Optional) + proxies: dict mapping request proxy protocols to proxy urls (Optional) + tls_config: configuration for remote write TLS settings (Optional) """ def __init__( self, endpoint: str, basic_auth: Dict = None, - bearer_token: str = None, - bearer_token_file: str = None, headers: Dict = None, + timeout: int = 30, + tls_config: Dict = None, + proxies: Dict = None, ): - raise NotImplementedError() + self.endpoint = endpoint + self.basic_auth = basic_auth + self.headers = headers + self.timeout = timeout + self.tls_config = tls_config + self.proxies = proxies + + @property + def endpoint(self): + return self._endpoint + + @endpoint.setter + def endpoint(self, endpoint: str): + if endpoint == "": + raise ValueError("endpoint required") + self._endpoint = endpoint + + @property + def basic_auth(self): + return self._basic_auth + + @basic_auth.setter + def basic_auth(self, basic_auth: Dict): + if basic_auth: + if "username" not in basic_auth: + raise ValueError("username required in basic_auth") + if ( + "password" not in basic_auth + and "password_file" not in basic_auth + ): + raise ValueError("password required in basic_auth") + if "password" in basic_auth and "password_file" in basic_auth: + raise ValueError( + "basic_auth cannot contain password and password_file" + ) + self._basic_auth = basic_auth + + @property + def timeout(self): + return self._timeout + + @timeout.setter + def timeout(self, timeout: int): + if timeout <= 0: + raise ValueError("timeout must be greater than 0") + self._timeout = timeout + + @property + def tls_config(self): + return self.tls_config + + @tls_config.setter + def tls_config(self, tls_config: Dict): + if tls_config: + new_config = {} + if "ca_file" in tls_config: + new_config["ca_file"] = tls_config.ca_file + if "cert_file" in tls_config and "key_file" in tls_config: + new_config["cert_file"] = tls_config.cert_file + new_config["key_file"] = tls_config.key_file + elif "cert_file" in tls_config or "key_file" in tls_config: + raise ValueError( + "tls_config requires both cert_file and key_file" + ) + if "insecure_skip_verify" in tls_config: + new_config[ + "insecure_skip_verify" + ] = tls_config.insecure_skip_verify + self._tls_config = tls_config + + @property + def proxies(self): + return self._proxies + + @proxies.setter + def proxies(self, proxies: Dict): + self._proxies = proxies + + @property + def headers(self): + return self._headers + + @headers.setter + def headers(self, headers: Dict): + self._headers = headers def export( self, export_records: Sequence[ExportRecord] diff --git a/exporter/opentelemetry-exporter-prometheus-remote-write/tests/test_prometheus_remote_write_exporter.py b/exporter/opentelemetry-exporter-prometheus-remote-write/tests/test_prometheus_remote_write_exporter.py index 4069a5fc82..b41ac05faf 100644 --- a/exporter/opentelemetry-exporter-prometheus-remote-write/tests/test_prometheus_remote_write_exporter.py +++ b/exporter/opentelemetry-exporter-prometheus-remote-write/tests/test_prometheus_remote_write_exporter.py @@ -14,35 +14,98 @@ import unittest +from opentelemetry.exporter.prometheus_remote_write import ( + PrometheusRemoteWriteMetricsExporter, +) + class TestValidation(unittest.TestCase): # Test cases to ensure exporter parameter validation works as intended def test_valid_standard_param(self): - pass + exporter = PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", + ) + self.assertEqual(exporter.endpoint, "/prom/test_endpoint") def test_valid_basic_auth_param(self): - pass - - def test_valid_bearer_token_param(self): - pass + exporter = PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", + basic_auth={ + "username": "test_username", + "password": "test_password", + }, + ) + self.assertEqual(exporter.basic_auth["username"], "test_username") + self.assertEqual(exporter.basic_auth["password"], "test_password") def test_invalid_no_endpoint_param(self): - pass + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter("") def test_invalid_no_username_param(self): - pass + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", + basic_auth={"password": "test_password"}, + ) def test_invalid_no_password_param(self): - pass + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", + basic_auth={"username": "test_username"}, + ) def test_invalid_conflicting_passwords_param(self): - pass - - def test_invalid_conflicting_bearer_tokens_param(self): - pass - - def test_invalid_conflicting_auth_param(self): - pass + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", + basic_auth={ + "username": "test_username", + "password": "test_password", + "password_file": "test_file", + }, + ) + + def test_invalid_timeout_param(self): + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", timeout=0 + ) + + def valid_tls_config(self): + tls_config = { + "ca_file": "test_ca_file", + "cert_file": "test_cert_file", + "key_file": "test_key_file", + "insecure_skip_verify": True, + } + exporter = PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", tls_config=tls_config + ) + self.assertEqual(exporter.tls_config.ca_file, tls_config.ca_file) + self.assertEqual(exporter.tls_config.cert_file, tls_config.cert_file) + self.assertEqual(exporter.tls_config.key_file, tls_config.key_file) + self.assertEqual( + exporter.tls_config.keyinsecure_skip_verify_file, + tls_config.insecure_skip_verify, + ) + + # if cert_file is provided, then key_file must also be provided + def test_invalid_tls_config_cert_only_param(self): + tls_config = {"cert_file": "value"} + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", tls_config=tls_config + ) + + # if cert_file is provided, then key_file must also be provided + def test_invalid_tls_config_key_only_param(self): + tls_config = {"cert_file": "value"} + with self.assertRaises(ValueError): + PrometheusRemoteWriteMetricsExporter( + endpoint="/prom/test_endpoint", tls_config=tls_config + ) class TestConversion(unittest.TestCase):