From d9e8724b1d086926c6d90873fa4729ae95360629 Mon Sep 17 00:00:00 2001 From: Victor Martinez Date: Wed, 20 Oct 2021 16:24:14 +0100 Subject: [PATCH] [callback][opentelemetry] enrich span with http attributes (#3558) * [callback][elastic] enrich span with http attributes * [callback][opentelemetry] fix linting * [callback][opentelemetry] fix UTs * [opentelemetry][changelog] add fragment for the service map feature * Update plugins/callback/opentelemetry.py Co-authored-by: Ajpantuso * [opentelemetry][callback] remove comments * [opentelemetry][callback] fix UTs * [opentelemetry][callback] return the value otherwise a None value returns ParseResultBytes when using urlparse(None) * [opentelemetry][callback] fix wrong parameter order * [opentelemetry][callback] support for no interpolated URLs * [opentelemetry][callback] support for URLs without interpolation in the hostname * [opentelemetry][callback] fix linting * Update changelogs/fragments/3558-callback_opentelemetry-enrich_service_map.yml Co-authored-by: Felix Fontein Co-authored-by: Ajpantuso Co-authored-by: Felix Fontein --- ...lback_opentelemetry-enrich_service_map.yml | 2 + plugins/callback/opentelemetry.py | 52 ++++++++++++++++++- .../plugins/callback/test_opentelemetry.py | 33 +++++++++++- 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/3558-callback_opentelemetry-enrich_service_map.yml diff --git a/changelogs/fragments/3558-callback_opentelemetry-enrich_service_map.yml b/changelogs/fragments/3558-callback_opentelemetry-enrich_service_map.yml new file mode 100644 index 0000000000..f89e8cd3e2 --- /dev/null +++ b/changelogs/fragments/3558-callback_opentelemetry-enrich_service_map.yml @@ -0,0 +1,2 @@ +minor_changes: + - opentelemetry callback plugin - enriched the span attributes with HTTP metadata for those Ansible tasks that interact with third party systems (https://github.com/ansible-collections/community.general/pull/3448). diff --git a/plugins/callback/opentelemetry.py b/plugins/callback/opentelemetry.py index 7604730461..732aacb48a 100644 --- a/plugins/callback/opentelemetry.py +++ b/plugins/callback/opentelemetry.py @@ -79,6 +79,7 @@ from os.path import basename from ansible.errors import AnsibleError from ansible.module_utils.six import raise_from +from ansible.module_utils.six.moves.urllib.parse import urlparse from ansible.plugins.callback import CallbackBase try: @@ -179,7 +180,7 @@ class OpenTelemetrySource(object): args = None if not task.no_log and not hide_task_arguments: - args = ', '.join(('%s=%s' % a for a in task.args.items())) + args = task.args tasks_data[uuid] = TaskData(uuid, name, path, play_name, action, args) @@ -265,13 +266,15 @@ class OpenTelemetrySource(object): status = Status(status_code=StatusCode.UNSET) span.set_status(status) - self.set_span_attribute(span, "ansible.task.args", task_data.args) + self.set_span_attribute(span, "ansible.task.args", self.flat_args(task_data.args)) self.set_span_attribute(span, "ansible.task.module", task_data.action) self.set_span_attribute(span, "ansible.task.message", message) self.set_span_attribute(span, "ansible.task.name", name) self.set_span_attribute(span, "ansible.task.result", rc) self.set_span_attribute(span, "ansible.task.host.name", host_data.name) self.set_span_attribute(span, "ansible.task.host.status", host_data.status) + # This will allow to enrich the service map + self.add_attributes_for_service_map_if_possible(span, task_data) span.end(end_time=host_data.finish) def set_span_attribute(self, span, attributeName, attributeValue): @@ -283,6 +286,45 @@ class OpenTelemetrySource(object): if attributeValue is not None: span.set_attribute(attributeName, attributeValue) + def add_attributes_for_service_map_if_possible(self, span, task_data): + """Update the span attributes with the service that the task interacted with, if possible.""" + + redacted_url = self.parse_and_redact_url_if_possible(task_data.args) + if redacted_url: + self.set_span_attribute(span, "http.url", redacted_url.geturl()) + + @staticmethod + def parse_and_redact_url_if_possible(args): + """Parse and redact the url, if possible.""" + + try: + parsed_url = urlparse(OpenTelemetrySource.url_from_args(args)) + except ValueError: + return None + + if OpenTelemetrySource.is_valid_url(parsed_url): + return OpenTelemetrySource.redact_user_password(parsed_url) + return None + + @staticmethod + def url_from_args(args): + # the order matters + url_args = ("url", "api_url", "baseurl", "repo", "server_url", "chart_repo_url") + for arg in url_args: + if args.get(arg): + return args.get(arg) + return "" + + @staticmethod + def redact_user_password(url): + return url._replace(netloc=url.hostname) if url.password else url + + @staticmethod + def is_valid_url(url): + if all([url.scheme, url.netloc, url.hostname]): + return "{{" not in url.hostname + return False + @staticmethod def get_error_message(result): if result.get('exception') is not None: @@ -301,6 +343,12 @@ class OpenTelemetrySource(object): stderr = result.get('stderr') return ('message: "{0}"\nexception: "{1}"\nstderr: "{2}"').format(message, exception, stderr) + @staticmethod + def flat_args(args): + if args: + return ', '.join(('%s=%s' % a for a in args.items())) + return None + class CallbackModule(CallbackBase): """ diff --git a/tests/unit/plugins/callback/test_opentelemetry.py b/tests/unit/plugins/callback/test_opentelemetry.py index d7ff845bd2..be5f286934 100644 --- a/tests/unit/plugins/callback/test_opentelemetry.py +++ b/tests/unit/plugins/callback/test_opentelemetry.py @@ -57,7 +57,7 @@ class TestOpentelemetry(unittest.TestCase): self.assertEqual(task_data.path, '/mypath') self.assertEqual(task_data.play, 'myplay') self.assertEqual(task_data.action, 'myaction') - self.assertEqual(task_data.args, '') + self.assertEqual(task_data.args, {}) def test_finish_task_with_a_host_match(self): tasks_data = OrderedDict() @@ -116,6 +116,37 @@ class TestOpentelemetry(unittest.TestCase): result = self.opentelemetry.enrich_error_message(generate_test_data(tc[0], tc[1], tc[2])) self.assertEqual(result, tc[3]) + def test_url_from_args(self): + test_cases = ( + ({}, ""), + ({'url': 'my-url'}, 'my-url'), + ({'url': 'my-url', 'api_url': 'my-api_url'}, 'my-url'), + ({'api_url': 'my-api_url'}, 'my-api_url'), + ({'api_url': 'my-api_url', 'chart_repo_url': 'my-chart_repo_url'}, 'my-api_url') + ) + + for tc in test_cases: + result = self.opentelemetry.url_from_args(tc[0]) + self.assertEqual(result, tc[1]) + + def test_parse_and_redact_url_if_possible(self): + test_cases = ( + ({}, None), + ({'url': 'wrong'}, None), + ({'url': 'https://my-url'}, 'https://my-url'), + ({'url': 'https://user:pass@my-url'}, 'https://my-url'), + ({'url': 'https://my-url:{{ my_port }}'}, 'https://my-url:{{ my_port }}'), + ({'url': 'https://{{ my_hostname }}:{{ my_port }}'}, None), + ({'url': '{{my_schema}}{{ my_hostname }}:{{ my_port }}'}, None) + ) + + for tc in test_cases: + result = self.opentelemetry.parse_and_redact_url_if_possible(tc[0]) + if tc[1]: + self.assertEqual(result.geturl(), tc[1]) + else: + self.assertEqual(result, tc[1]) + def generate_test_data(exception=None, msg=None, stderr=None): res_data = OrderedDict()