diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0ef88b5..aeddeaa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,19 @@ Change Log Unreleased ~~~~~~~~~~ +[6.1.0] - 2024-12-10 +~~~~~~~~~~~~~~~~~~~~ +Changed +_______ +* Completes code owner monitoring updates, which drops owner theme and finalizes the code owner span tags. See doc and ADR updates for more details. + + * The code_owner_theme_2 tag was dropped altogether. + * The temporary suffix (_2) was removed from other span tags. + * The code_owner (formerly code_owner_2) tag no longer includes the theme name. + * The new name for the django setting is CODE_OWNER_TO_PATH_MAPPINGS (formerly CODE_OWNER_MAPPINGS). + * The django setting CODE_OWNER_THEMES was dropped. + * Updates the generate_code_owner_mappings.py script accordingly. + [6.0.0] - 2024-12-05 ~~~~~~~~~~~~~~~~~~~~ Removed diff --git a/edx_arch_experiments/__init__.py b/edx_arch_experiments/__init__.py index 6a4f275..8695494 100644 --- a/edx_arch_experiments/__init__.py +++ b/edx_arch_experiments/__init__.py @@ -2,4 +2,4 @@ A plugin to include applications under development by the architecture team at 2U. """ -__version__ = '6.0.0' +__version__ = '6.1.0' diff --git a/edx_arch_experiments/datadog_monitoring/README.rst b/edx_arch_experiments/datadog_monitoring/README.rst index 5498d75..a050126 100644 --- a/edx_arch_experiments/datadog_monitoring/README.rst +++ b/edx_arch_experiments/datadog_monitoring/README.rst @@ -1,6 +1,6 @@ Datadog Monitoring ################### -When installed in the LMS as a plugin app, the ``datadog_monitoring`` app adds additional monitoring. +When installed in the LMS as a plugin app, the ``datadog_monitoring`` app adds additional 2U-specific monitoring. -This is where our code_owner_2 monitoring code lives, for example. +This is where our code owner monitoring code lives, for example. diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py index 0199907..a1f52f5 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py @@ -1,7 +1,7 @@ """ Datadog span processor for celery span code owners. """ -from .utils import set_code_owner_attribute_from_module +from .utils import set_code_owner_span_tags_from_module class CeleryCodeOwnerSpanProcessor: @@ -17,7 +17,7 @@ def on_span_start(self, span): # We can use this for celery spans, because the resource name is more predictable # and available from the start. For django requests, we'll instead continue to use # django middleware for setting code owner. - set_code_owner_attribute_from_module(span.resource) + set_code_owner_span_tags_from_module(span.resource) def on_span_finish(self, span): pass diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py index cdf079b..a618504 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py @@ -1,5 +1,5 @@ """ -Utilities for monitoring code_owner_2 +Utilities for monitoring code_owner """ import logging @@ -19,9 +19,6 @@ def get_code_owner_from_module(module): this lookup would match on 'openedx.features.discounts' before 'openedx.features', because the former is more specific. - See how to: - https://github.com/openedx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst - """ if not module: return None @@ -54,7 +51,7 @@ def is_code_owner_mappings_configured(): def get_code_owner_mappings(): """ - Returns the contents of the CODE_OWNER_MAPPINGS Django Setting, processed + Returns the contents of the CODE_OWNER_TO_PATH_MAPPINGS Django Setting, processed for efficient lookup by path. Returns: @@ -81,12 +78,12 @@ def get_code_owner_mappings(): # processed map. Worst case, it is processed more than once at start-up. path_to_code_owner_mapping = {} - # .. setting_name: CODE_OWNER_MAPPINGS + # .. setting_name: CODE_OWNER_TO_PATH_MAPPINGS # .. setting_default: None # .. setting_description: Used for monitoring and reporting of ownership. Use a # dict with keys of code owner name and value as a list of dotted path # module names owned by the code owner. - code_owner_mappings = getattr(settings, 'CODE_OWNER_MAPPINGS', None) + code_owner_mappings = getattr(settings, 'CODE_OWNER_TO_PATH_MAPPINGS', None) if code_owner_mappings is None: return None @@ -97,7 +94,7 @@ def get_code_owner_mappings(): path_to_code_owner_mapping[path] = code_owner except TypeError as e: log.exception( - 'Error processing CODE_OWNER_MAPPINGS. {}'.format(e) # pylint: disable=logging-format-interpolation + 'Error processing CODE_OWNER_TO_PATH_MAPPINGS. {}'.format(e) # pylint: disable=logging-format-interpolation ) raise e @@ -105,54 +102,57 @@ def get_code_owner_mappings(): return _PATH_TO_CODE_OWNER_MAPPINGS -def set_code_owner_attribute_from_module(module): +def set_code_owner_span_tags_from_module(module): """ - Updates the code_owner_2 and code_owner_2_module custom attributes. - - Celery tasks or other non-web functions do not use middleware, so we need - an alternative way to set the code_owner_2 custom attribute. - - Note: These settings will be overridden by the CodeOwnerMonitoringMiddleware. - This method can't be used to override web functions at this time. + Updates the code_owner and code_owner_module custom span tags. Usage:: - set_code_owner_2_attribute_from_module(__name__) + set_code_owner_span_tags_from_module(__name__) """ - set_custom_attribute('code_owner_2_module', module) + # .. custom_attribute_name: code_owner_module + # .. custom_attribute_description: The module used to determine the code_owner. This can + # be useful for debugging issues for missing code owner span tags. + set_custom_attribute('code_owner_module', module) code_owner = get_code_owner_from_module(module) if code_owner: - set_code_owner_custom_attributes(code_owner) + set_code_owner_custom_span_tags(code_owner) -def set_code_owner_custom_attributes(code_owner): +def set_code_owner_custom_span_tags(code_owner): """ - Sets custom metrics for code_owner_2, code_owner_2_theme, and code_owner_2_squad + Sets custom span tags for code_owner, and code_owner_squad """ if not code_owner: # pragma: no cover return - set_custom_attribute('code_owner_2', code_owner) - theme = _get_theme_from_code_owner(code_owner) - if theme: - set_custom_attribute('code_owner_2_theme', theme) - squad = _get_squad_from_code_owner(code_owner) - if squad: - set_custom_attribute('code_owner_2_squad', squad) + # .. custom_attribute_name: code_owner + # .. custom_attribute_description: The squad owner name for the tagged span. + set_custom_attribute('code_owner', code_owner) + # .. custom_attribute_name: code_owner_squad + # .. custom_attribute_description: Deprecated code_owner_squad is now redundant + # to the code_owner span tag. + set_custom_attribute('code_owner_squad', code_owner) + + # .. custom_attribute_name: code_owner_plugin + # .. custom_attribute_description: This is a temporary span tag to roll out the + # the switch from edx-django-utils to this plugin. If this span tag is True, + # the plugin has added the above custom span tags (possibly in addition to + # edx-django-utils). If the code_owner_theme span tag is also seen, then + # edx-django-utils is also adding these span tags. If not, only the plugin is + # creating these tags. + set_custom_attribute('code_owner_plugin', True) -def set_code_owner_attribute(request): + +def set_code_owner_span_tags_from_request(request): """ - Sets the code_owner_2 custom attribute for the request. + Sets the code_owner custom span tag for the request. """ - code_owner = None module = _get_module_from_request(request) if module: - code_owner = get_code_owner_from_module(module) - - if code_owner: - set_code_owner_custom_attributes(code_owner) + set_code_owner_span_tags_from_module(module) def _get_module_from_request(request): @@ -160,9 +160,7 @@ def _get_module_from_request(request): Get the module from the request path or the current transaction. Side-effects: - Sets code_owner_2_module custom attribute, used to determine code_owner_2. - If module was not found, may set code_owner_2_path_error custom attribute - if applicable. + Sets code_owner_path_error custom span tag if applicable. Returns: str: module name or None if not found @@ -172,14 +170,14 @@ def _get_module_from_request(request): return None module, path_error = _get_module_from_request_path(request) - if module: - set_custom_attribute('code_owner_2_module', module) - return module - # monitor errors if module was not found if path_error: - set_custom_attribute('code_owner_2_path_error', path_error) - return None + # .. custom_attribute_name: code_owner_path_error + # .. custom_attribute_description: Error details if the module can't be found. This can + # be useful for debugging issues for missing code owner span tags. + set_custom_attribute('code_owner_path_error', path_error) + + return module def _get_module_from_request_path(request): @@ -204,100 +202,3 @@ def clear_cached_mappings(): """ global _PATH_TO_CODE_OWNER_MAPPINGS _PATH_TO_CODE_OWNER_MAPPINGS = None - global _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS - _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS = None - - -# Cached lookup table for code owner theme and squad given a code owner. -# - Although code owner is "theme-squad", a hyphen may also be in the theme or squad name, so this ensures we get both -# correctly from config. -# Do not access this directly, but instead use get_code_owner_theme_squad_mappings. -_CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS = None - - -def get_code_owner_theme_squad_mappings(): - """ - Returns the contents of the CODE_OWNER_THEMES Django Setting, processed - for efficient lookup by path. - - Returns: - (dict): dict mapping code owners to a dict containing the squad and theme, or - an empty dict if there are no configured mappings. - - Example return value:: - - { - 'theme-x-team-red': { - 'theme': 'theme-x', - 'squad': 'team-red', - }, - 'theme-x-team-blue': { - 'theme': 'theme-x', - 'squad': 'team-blue', - }, - } - - """ - global _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS - - # Return cached processed mappings if already processed - if _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS is not None: - return _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS - - # Uses temporary variable to build mappings to avoid multi-threading issue with a partially - # processed map. Worst case, it is processed more than once at start-up. - code_owner_to_theme_and_squad_mapping = {} - - # .. setting_name: CODE_OWNER_THEMES - # .. setting_default: None - # .. setting_description: Used for monitoring and reporting of ownership. Use a - # dict with keys of code owner themes and values as a list of code owner names - # including theme and squad, separated with a hyphen. - code_owner_themes = getattr(settings, 'CODE_OWNER_THEMES', {}) - - try: - for theme in code_owner_themes: - code_owner_list = code_owner_themes[theme] - for code_owner in code_owner_list: - squad = code_owner.split(theme + '-', 1)[1] - code_owner_details = { - 'theme': theme, - 'squad': squad, - } - code_owner_to_theme_and_squad_mapping[code_owner] = code_owner_details - except TypeError as e: - log.exception( - 'Error processing CODE_OWNER_THEMES setting. {}'.format(e) # pylint: disable=logging-format-interpolation - ) - raise e - - _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS = code_owner_to_theme_and_squad_mapping - return _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS - - -def _get_theme_from_code_owner(code_owner): - """ - Returns theme for a code_owner (e.g. 'theme-my-squad' => 'theme') - """ - mappings = get_code_owner_theme_squad_mappings() - if mappings is None: # pragma: no cover - return None - - if code_owner in mappings: - return mappings[code_owner]['theme'] - - return None - - -def _get_squad_from_code_owner(code_owner): - """ - Returns squad for a code_owner (e.g. 'theme-my-squad' => 'my-squad') - """ - mappings = get_code_owner_theme_squad_mappings() - if mappings is None: # pragma: no cover - return None - - if code_owner in mappings: - return mappings[code_owner]['squad'] - - return None diff --git a/edx_arch_experiments/datadog_monitoring/docs/decisions/0001-monitoring-by-code-owner.rst b/edx_arch_experiments/datadog_monitoring/docs/decisions/0001-monitoring-by-code-owner.rst new file mode 100644 index 0000000..c74617e --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/decisions/0001-monitoring-by-code-owner.rst @@ -0,0 +1,54 @@ +Monitoring by Code Owner +************************ + +Status +====== + +Accepted + +Context +======= + +It is difficult for different teams to have team-based on-calls rotations, alerting and monitoring for various parts of the edx-platform (specifically LMS). + +* The original decision to add code_owner custom span tags (custom attributes) was documented in `edx-platform in 0001-monitoring-by-code-owner`_. +* The decision to move the code for reuse across IDAs was documented in `edx-django-utils in 0001-monitoring-by-code-owner.rst`_. +* The decision for how to implement code owner details for celery tasks was documented in `0003-code-owner-for-celery-tasks_, and was limited by New Relic's instrumentation. +* The decision to break up the ``code_owner`` custom span tag (custom attribute) into ``code_owner_squad`` and ``code_owner_theme`` tags was documented in `0004-code-owner-theme-and-squad`_. + +Some changes or clarifications since this time: + +* It turns out that this functionality is only really useful for the edx-platform LMS. Our other services (IDAs) are small enough to keep to a single owner, or to solve monitoring issues in other ways. +* It is likely that the ``code_owner`` code is only really needed by 2U. +* 2U wants to drop owner themes from its code_owner custom span tag. +* 2U has switched to Datadog, which has slightly different capabilities from New Relic. + + * Note that Datadog has custom span tags, where New Relic has custom attributes to refer to its tagging capabilities. + +.. _edx-platform in 0001-monitoring-by-code-owner: https://github.com/openedx/edx-platform/blob/f29e418264f374099930a5b1f5b8345c569892e9/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst +.. _edx-django-utils in 0001-monitoring-by-code-owner.rst: https://github.com/openedx/edx-django-utils/blob/a1a1ec95d7c1d4767deb578748153c99c9562a04/edx_django_utils/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst +.. _0003-code-owner-for-celery-tasks: https://github.com/openedx/edx-django-utils/blob/a1a1ec95d7c1d4767deb578748153c99c9562a04/edx_django_utils/monitoring/docs/decisions/0003-code-owner-for-celery-tasks.rst +.. _0004-code-owner-theme-and-squad: https://github.com/openedx/edx-django-utils/blob/a1a1ec95d7c1d4767deb578748153c99c9562a04/edx_django_utils/monitoring/docs/decisions/0004-code-owner-theme-and-squad.rst + +Decision +======== + +2U has moved its code owner monitoring implementation to the datadog_monitoring plugin. + +* The owner theme name has been dropped from the ``code_owner`` custom span tag value in this new implementation. +* The ``code_owner_theme`` span tag has been dropped altogether from this new implementation. +* The now deprecated ``code_owner_squad`` span tag, which is redundant with the updated ``code_owner`` tag, will continue to be supported for backward compatibility. +* A Datadog span processor was used to add the code owner span tags for celery tasks, so there is no longer a need for a special decorator on each celery task. +* A new capability added to edx-django-utils to add `monitoring signals for plugins`_ is used to monitor Django requests. + +Also, a new search script was implemented in this repository: `search_datadog.rst`_. + +.. _monitoring signals for plugins: https://github.com/openedx/edx-django-utils/pull/467 +.. _search_datadog.rst: https://github.com/edx/edx-arch-experiments/blob/main/edx_arch_experiments/datadog_monitoring/scripts/datadog_search.py + +Consequences +============ + +- In addition to having greater flexibility to update these custom tags as-needed for 2U, we can also DEPR the code owner functionality in the Open edX codebase, where it is not likely to be needed. +- Spreadsheet changes will no longer be required when a squad moves from one part of the organization to another (e.g. changes themes). +- However, without including themes, it may take additional time to learn about a squad's place in the organization when seen in the code_owner span tag. For example, it will not be as immediately clear when dealing with an enterprise squad, unless you are familiar with all of the squad names. diff --git a/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst b/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst deleted file mode 100644 index ca1dab9..0000000 --- a/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst +++ /dev/null @@ -1,65 +0,0 @@ -Using Code_Owner Custom Span Tags -================================= - -.. contents:: - :local: - :depth: 2 - -What are the code owner custom span tags? ------------------------------------------- - -The code owner custom span tags can be used to create custom dashboards and alerts for monitoring the things that you own. It was originally introduced for the LMS, as is described in this `ADR on monitoring by code owner`_. However, it was first moved to edx-django-utils to be used in any IDA. It was later moved to this 2U-specific plugin because it is for 2U. - -The code owner custom attributes consist of: - -* code_owner_2: The owner name. When themes and squads are used, this will be the theme and squad names joined by a hyphen. -* code_owner_2_theme: The theme name of the owner. -* code_owner_2_squad: The squad name of the owner. Use this to avoid issues when theme name changes. - -Note: The ``_2`` of the code_owner_2 naming is for initial rollout to compare with edx-django-utils span tags. Ultimately, we will use adjusted names, which may include dropping the theme. - -If you want to learn more about custom span tags in general, see `Enhanced Monitoring and Custom Attributes`_. - -.. _ADR on monitoring by code owner: https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst -.. _Enhanced Monitoring and Custom Attributes: https://edx.readthedocs.io/projects/edx-django-utils/en/latest/monitoring/how_tos/using_custom_attributes.html - -What gets code_owner span tags ------------------------------- - -Simply by installing the datadog_monitoring plugin, code owner span tags will automatically be added for: - -* ``operation_name:django.request``: tags are added by using edx-django-utils monitoring signals, which are sent by its MonitoringSupportMiddleware. -* ``operation_name:celery.run``: tags are added using celery's ``worker_process_init`` signal, and then adding a custom Datadog span processor to add the span tags as appropriate. - -Configuring your app settings ------------------------------ - -Once the Middleware is made available, simply set the Django Settings ``CODE_OWNER_MAPPINGS`` and ``CODE_OWNER_THEMES`` appropriately. - -:: - - # YAML format of example CODE_OWNER_MAPPINGS - CODE_OWNER_MAPPINGS: - theme-x-team-red: - - xblock_django - - openedx.core.djangoapps.xblock - theme-x-team-blue: - - lms - - # YAML format of example CODE_OWNER_THEMES - CODE_OWNER_THEMES: - theme-x: - - theme-x-team-red - - theme-x-team-blue - -How to find and fix code_owner mappings ---------------------------------------- - -If you are missing the ``code_owner_2`` custom attributes on a particular Transaction or Error, or if ``code_owner`` is matching the catch-all, but you want to add a more specific mapping, you can use the other supporting tags like ``code_owner_2_module`` and ``code_owner_2_path_error`` to determine what the appropriate mappings should be. - -Updating Datadog monitoring ---------------------------- - -To update monitoring in the event of a squad or theme name change, see `Update Monitoring for Squad or Theme Changes`_. - -.. _Update Monitoring for Squad or Theme Changes: diff --git a/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_changes.rst b/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_changes.rst new file mode 100644 index 0000000..3f6eb10 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_changes.rst @@ -0,0 +1,40 @@ +Update Monitoring for Squad Changes +=================================== + +.. contents:: + :local: + :depth: 2 + +Understanding code owner custom span tags +----------------------------------------- + +If you first need some background on the ``code_owner`` custom span tag, see `Using Code_Owner Custom Span Tags`_. + +.. _Using Code_Owner Custom Span Tags: https://github.com/edx/edx-arch-experiments/blob/main/edx_arch_experiments/datadog_monitoring/docs/how_tos/using_code_owner_custom_span_tags.rst + +Expand and contract name changes +-------------------------------- + +Datadog monitors or dashboards may use the ``code_owner`` (or deprecated ``code_owner_squad``) custom span tags. + +To change a squad name, you should *expand* before the change, and *contract* after the change. + +Example expand phase:: + + code_owner:('old-squad-name', 'new-squad-name') + +Example contract phase:: + + code_owner:'new-squad-name' + +To find relevant usage of these span tags, see `Searching Datadog monitors and dashboards`_. + +Searching Datadog monitors and dashboards +----------------------------------------- + +See :doc:`search_datadog` for general information about the ``datadog_search.py`` script. + +This script can be especially useful for helping with the expand/contract phase when changing squad names. For example, you could use the following:: + + ./datadog_search.py --regex old-squad-name + ./datadog_search.py --regex new-squad-name diff --git a/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst b/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst deleted file mode 100644 index fe02bc8..0000000 --- a/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst +++ /dev/null @@ -1,42 +0,0 @@ -Update Monitoring for Squad or Theme Changes -============================================ - -.. contents:: - :local: - :depth: 2 - -Understanding code owner custom attributes ------------------------------------------- - -If you first need some background on the ``code_owner_2_squad`` and ``code_owner_2_theme`` custom attributes, see `Using Code_Owner Custom Span Tags`_. - -.. _Using Code_Owner Custom Span Tags: https://github.com/edx/edx-arch-experiments/blob/main/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst - -Expand and contract name changes --------------------------------- - -Datadog monitors or dashboards may use the ``code_owner_2_squad`` or ``code_owner_2_theme`` (or ``code_owner_2``) custom span tags. - -To change a squad or theme name, you should *expand* before the change, and *contract* after the change. - -Example expand phase:: - - code_owner_2_squad:('old-squad-name', 'new-squad-name') - code_owner_2_theme:('old-theme-name', 'new-theme-name') - -Example contract phase:: - - code_owner_2_squad:'new-squad-name' - code_owner_2_theme:'new-theme-name' - -To find relevant usage of these span tags, see `Searching Datadog monitors and dashboards`_. - -Searching Datadog monitors and dashboards ------------------------------------------ - -See :doc:`search_datadog` for general information about the datadog_search.py script. - -This script can be especially useful for helping with the expand/contract phase when changing squad names. For example, you could use the following:: - - ./datadog_search.py --regex old-squad-name - ./datadog_search.py --regex new-squad-name diff --git a/edx_arch_experiments/datadog_monitoring/docs/how_tos/using_code_owner_custom_span_tags.rst b/edx_arch_experiments/datadog_monitoring/docs/how_tos/using_code_owner_custom_span_tags.rst new file mode 100644 index 0000000..68f251a --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/how_tos/using_code_owner_custom_span_tags.rst @@ -0,0 +1,55 @@ +Using Code_Owner Custom Span Tags +================================= + +.. contents:: + :local: + :depth: 2 + +What are the code owner custom span tags? +----------------------------------------- + +The code owner custom span tags (formerly known as the code owner custom attributes) can be used to create custom dashboards and monitors for the things that you own. + +To read more on the decision to use these custom span tags, read `ADR on monitoring by code owner`_. + +The code owner span tags are as follows: + +* code_owner: The squad name of the owner. +* code_owner_squad: Deprecated. Redundant with code_owner for backward compatibility. +* code_owner_module: Module used for determining code_owner. Useful for debugging missing code owner details. +* code_owner_path_error: Possible error determining code_owner. Useful for debugging missing code owner details. + +If you want to learn more about custom span tags in general, see `Enhanced Monitoring and Custom Attributes`_. + +.. _ADR on monitoring by code owner: https://github.com/edx/edx-arch-experiments/blob/main/edx_arch_experiments/datadog_monitoring/docs/decisions/0001-monitoring-by-code-owner.rst +.. _Enhanced Monitoring and Custom Attributes: https://edx.readthedocs.io/projects/edx-django-utils/en/latest/monitoring/how_tos/using_custom_attributes.html + +How code owner span tags are added +---------------------------------- + +Simply by installing the datadog_monitoring plugin, code owner span tags will automatically be added for: + +* ``operation_name:django.request``: tags are added by using edx-django-utils monitoring signals, which are sent by its MonitoringSupportMiddleware. +* ``operation_name:celery.run``: tags are added using celery's ``worker_process_init`` signal, and then adding a custom Datadog span processor to add the span tags as appropriate. + +Configuring your app settings +----------------------------- + +Once the datadog_monitoring plugin is installed, set the Django Settings ``CODE_OWNER_TO_PATH_MAPPINGS`` appropriately. + +:: + + # YAML format of example CODE_OWNER_TO_PATH_MAPPINGS + CODE_OWNER_TO_PATH_MAPPINGS: + team-red: + - xblock_django + - openedx.core.djangoapps.xblock + team-blue: + - lms + +Updating Datadog monitoring +--------------------------- + +https://github.com/openedx/edx-platform/blob/master/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rstTo update monitoring in the event of a squad name change, see `Update Monitoring for Squad Changes`_. + +.. _Update Monitoring for Squad Changes: https://github.com/edx/edx-arch-experiments/blob/main/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_changes.rst diff --git a/edx_arch_experiments/datadog_monitoring/signals/handlers.py b/edx_arch_experiments/datadog_monitoring/signals/handlers.py index 6be2e22..1c1fd3d 100644 --- a/edx_arch_experiments/datadog_monitoring/signals/handlers.py +++ b/edx_arch_experiments/datadog_monitoring/signals/handlers.py @@ -12,7 +12,7 @@ ) from edx_arch_experiments.datadog_monitoring.code_owner.datadog import CeleryCodeOwnerSpanProcessor -from edx_arch_experiments.datadog_monitoring.code_owner.utils import set_code_owner_attribute +from edx_arch_experiments.datadog_monitoring.code_owner.utils import set_code_owner_span_tags_from_request log = logging.getLogger(__name__) @@ -22,11 +22,7 @@ def datadog_monitoring_support_process_response(sender, request=None, **kwargs): """ Adds datadog monitoring at monitoring process response time. """ - if request: - set_code_owner_attribute(request) - else: - log.warning('monitoring_support_process_response sent without ' - 'expected parameter: request.') + set_code_owner_span_tags_from_request(request) @receiver(monitoring_support_process_exception, dispatch_uid=f"datadog_monitoring_support_process_exception") @@ -34,11 +30,7 @@ def datadog_monitoring_support_process_exception(sender, request=None, **kwargs) """ Adds datadog monitoring at monitoring process exception time. """ - if request: - set_code_owner_attribute(request) - else: - log.warning('monitoring_support_process_exception sent without ' - 'expected parameter: request.') + set_code_owner_span_tags_from_request(request) @receiver(worker_process_init, dispatch_uid=f"datadog_span_processor_worker_process_init") diff --git a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py index 4769e0d..852e6b9 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py @@ -31,7 +31,7 @@ def test_celery_span(self, mock_set_custom_attribute): proc.on_span_start(celery_span) - mock_set_custom_attribute.assert_called_once_with('code_owner_2_module', 'test.module.for.celery.task') + mock_set_custom_attribute.assert_called_once_with('code_owner_module', 'test.module.for.celery.task') @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') def test_other_span(self, mock_set_custom_attribute): diff --git a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py index 752cf28..3b116e2 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py @@ -13,8 +13,8 @@ from edx_arch_experiments.datadog_monitoring.code_owner.utils import ( clear_cached_mappings, get_code_owner_from_module, - set_code_owner_attribute, - set_code_owner_attribute_from_module, + set_code_owner_span_tags_from_module, + set_code_owner_span_tags_from_request, ) from .mock_views import MockViewTest @@ -39,7 +39,7 @@ def setUp(self): super().setUp() clear_cached_mappings() - @override_settings(CODE_OWNER_MAPPINGS={ + @override_settings(CODE_OWNER_TO_PATH_MAPPINGS={ 'team-red': [ 'openedx.core.djangoapps.xblock', 'lms.djangoapps.grades', @@ -50,7 +50,7 @@ def setUp(self): }) @ddt.data( ('xbl', None), - ('xblock_2', None), + ('xblock', None), ('openedx.core.djangoapps', None), ('openedx.core.djangoapps.xblock', 'team-red'), ('openedx.core.djangoapps.xblock.views', 'team-red'), @@ -62,14 +62,14 @@ def test_code_owner_mapping_hits_and_misses(self, module, expected_owner): actual_owner = get_code_owner_from_module(module) self.assertEqual(expected_owner, actual_owner) - @override_settings(CODE_OWNER_MAPPINGS=['invalid_setting_as_list']) + @override_settings(CODE_OWNER_TO_PATH_MAPPINGS=['invalid_setting_as_list']) @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.log') def test_code_owner_mapping_with_invalid_dict(self, mock_logger): with self.assertRaises(TypeError): get_code_owner_from_module('xblock') mock_logger.exception.assert_called_with( - 'Error processing CODE_OWNER_MAPPINGS. list indices must be integers or slices, not str', + 'Error processing CODE_OWNER_TO_PATH_MAPPINGS. list indices must be integers or slices, not str', ) def test_code_owner_mapping_with_no_settings(self): @@ -86,7 +86,7 @@ def test_mapping_performance(self): for n in range(1, 200): path = f'openedx.core.djangoapps.{n}' code_owner_mappings['team-red'].append(path) - with override_settings(CODE_OWNER_MAPPINGS=code_owner_mappings): + with override_settings(CODE_OWNER_TO_PATH_MAPPINGS=code_owner_mappings): call_iterations = 100 time = timeit.timeit( # test a module name that matches nearly to the end, but doesn't actually match @@ -95,17 +95,18 @@ def test_mapping_performance(self): average_time = time / call_iterations self.assertLess(average_time, 0.0005, f'Mapping takes {average_time}s which is too slow.') - @override_settings(CODE_OWNER_MAPPINGS={ + @override_settings(CODE_OWNER_TO_PATH_MAPPINGS={ 'team-red': ['edx_arch_experiments.datadog_monitoring.tests.code_owner.test_utils'] }) @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') - def test_set_code_owner_attribute_from_module_success(self, mock_set_custom_attribute): - set_code_owner_attribute_from_module(__name__) + def test_set_code_owner_span_tags_from_module_success(self, mock_set_custom_attribute): + set_code_owner_span_tags_from_module(__name__) self._assert_set_custom_attribute(mock_set_custom_attribute, code_owner='team-red', module=__name__) @override_settings( - CODE_OWNER_MAPPINGS={'team-red': ['edx_arch_experiments.datadog_monitoring.tests.code_owner.mock_views']}, - CODE_OWNER_THEMES={'team': ['team-red']}, + CODE_OWNER_TO_PATH_MAPPINGS={ + 'team-red': ['edx_arch_experiments.datadog_monitoring.tests.code_owner.mock_views'] + }, ROOT_URLCONF=__name__, ) @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') @@ -114,25 +115,36 @@ def test_set_code_owner_attribute_from_module_success(self, mock_set_custom_attr ('/test/', 'team-red', 'edx_arch_experiments.datadog_monitoring.tests.code_owner.mock_views'), ) @ddt.unpack - def test_set_code_owner_attribute_from_request_success( + def test_set_code_owner_span_tags_from_request_success( self, request_path, expected_owner, expected_module, mock_set_custom_attribute ): request = RequestFactory().get(request_path) - set_code_owner_attribute(request) + set_code_owner_span_tags_from_request(request) self._assert_set_custom_attribute( mock_set_custom_attribute, code_owner=expected_owner, module=expected_module, - check_theme_and_squad=True ) @override_settings( - CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']}, + CODE_OWNER_TO_PATH_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']}, ) @patch( 'edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute', ) - def test_set_code_owner_attribute_from_request_no_resolver_for_path(self, mock_set_custom_attribute): + def test_set_code_owner_span_tags_from_request_no_resolver_for_path(self, mock_set_custom_attribute): request = RequestFactory().get('/bad/path/') - set_code_owner_attribute(request) + set_code_owner_span_tags_from_request(request) + self._assert_set_custom_attribute( + mock_set_custom_attribute, has_path_error=True + ) + + @override_settings( + CODE_OWNER_TO_PATH_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']}, + ) + @patch( + 'edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute', + ) + def test_set_code_owner_span_tags_from_request_is_none(self, mock_set_custom_attribute): + set_code_owner_span_tags_from_request(None) self._assert_set_custom_attribute( mock_set_custom_attribute, has_path_error=True ) @@ -141,28 +153,27 @@ def test_set_code_owner_attribute_from_request_no_resolver_for_path(self, mock_s ROOT_URLCONF=__name__, ) @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') - def test_set_code_owner_attribute_from_request_no_mappings(self, mock_set_custom_attribute): + def test_set_code_owner_span_tags_from_request_no_mappings(self, mock_set_custom_attribute): request = RequestFactory().get('/test/') - set_code_owner_attribute(request) + set_code_owner_span_tags_from_request(request) mock_set_custom_attribute.assert_not_called() def _assert_set_custom_attribute( self, mock_set_custom_attribute, code_owner=None, module=None, - check_theme_and_squad=False, has_path_error=False - ): # pylint: disable=too-many-positional-arguments + has_path_error=False + ): """ Helper to assert that the proper set_custom_metric calls were made. """ call_list = [] if code_owner: - call_list.append(call('code_owner_2', code_owner)) - if check_theme_and_squad: - call_list.append(call('code_owner_2_theme', code_owner.split('-')[0])) - call_list.append(call('code_owner_2_squad', code_owner.split('-')[1])) + call_list.append(call('code_owner', code_owner)) + call_list.append(call('code_owner_squad', code_owner)) + call_list.append(call('code_owner_plugin', True)) if module: - call_list.append(call('code_owner_2_module', module)) + call_list.append(call('code_owner_module', module)) if has_path_error: - call_list.append(call('code_owner_2_path_error', ANY)) + call_list.append(call('code_owner_path_error', ANY)) mock_set_custom_attribute.assert_has_calls(call_list, any_order=True) self.assertEqual( len(mock_set_custom_attribute.call_args_list), len(call_list), diff --git a/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py b/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py index f723671..22bc18b 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py +++ b/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py @@ -23,15 +23,15 @@ def setUp(self): sp for sp in tracer._span_processors if type(sp).__name__ != 'CeleryCodeOwnerSpanProcessor' ] - @patch('edx_arch_experiments.datadog_monitoring.signals.handlers.set_code_owner_attribute') - def test_datadog_monitoring_support_process_response(self, mock_set_code_owner_attribute): + @patch('edx_arch_experiments.datadog_monitoring.signals.handlers.set_code_owner_span_tags_from_request') + def test_datadog_monitoring_support_process_response(self, mock_set_code_owner_span_tags_from_request): datadog_monitoring_support_process_response(sender=None, request='fake request') - mock_set_code_owner_attribute.assert_called_once_with('fake request') + mock_set_code_owner_span_tags_from_request.assert_called_once_with('fake request') - @patch('edx_arch_experiments.datadog_monitoring.signals.handlers.set_code_owner_attribute') - def test_datadog_monitoring_support_process_exception(self, mock_set_code_owner_attribute): + @patch('edx_arch_experiments.datadog_monitoring.signals.handlers.set_code_owner_span_tags_from_request') + def test_datadog_monitoring_support_process_exception(self, mock_set_code_owner_span_tags_from_request): datadog_monitoring_support_process_exception(sender=None, request='fake request') - mock_set_code_owner_attribute.assert_called_once_with('fake request') + mock_set_code_owner_span_tags_from_request.assert_called_once_with('fake request') def test_init_worker_process(self): def get_processor_list(): diff --git a/edx_arch_experiments/scripts/generate_code_owner_mappings.py b/edx_arch_experiments/scripts/generate_code_owner_mappings.py index a80e6e6..a78614b 100644 --- a/edx_arch_experiments/scripts/generate_code_owner_mappings.py +++ b/edx_arch_experiments/scripts/generate_code_owner_mappings.py @@ -3,12 +3,15 @@ Sample usage:: - python lms/djangoapps/monitoring/scripts/generate_code_owner_mappings.py --repo-csv "Own Repos.csv" - --app-csv "Own edx-platform Apps.csv" --dep-csv "Reference edx-platform Libs.csv" --override-csv "Overrides.csv" + python generate_code_owner_mappings.py + --repo-csv "edX _ OCM Tech Ownership Assignment - Own_ Repos.csv" + --app-csv "edX _ OCM Tech Ownership Assignment - Own_ edx-platform Apps.csv" + --dep-csv "edX _ OCM Tech Ownership Assignment - Reference_ edx-platform Libs.csv" + --override-csv "edX _ OCM Tech Ownership Assignment - Own_ Ownership overrides.csv" Or for more details:: - python lms/djangoapps/monitoring/scripts/generate_code_owner_mappings.py --help + python generate_code_owner_mappings.py --help """ @@ -105,72 +108,56 @@ def main(repo_csv, app_csv, dep_csv, override_csv=None): Expected Repo CSV format: \b - repo url,owner.theme,owner.squad - https://github.com/openedx/edx-bulk-grades,theme-a,team-red + repo url,owner.squad + https://github.com/openedx/edx-bulk-grades,team-red ... Expected App CSV format: \b - Path,owner.theme,owner.squad - ./openedx/core/djangoapps/user_authn,theme-b,team-blue + Path,owner.squad + ./openedx/core/djangoapps/user_authn,team-blue ... Expected 3rd-party Dependency CSV format: \b - repo url,owner.theme,owner.squad - https://github.com/django/django,theme-a,team-red + repo url,owner.squad + https://github.com/django/django,team-red ... Expected Override CSV format: \b - app_name,owner.theme,owner.squad - integrated_channels,theme-b,team-blue + app_name,owner.squad + integrated_channels,team-blue ... Final output only includes paths which might contain views. """ - # Maps theme name to a list of code owners in the theme, and squad to full code owner name. - # Code owner is a string combining theme and squad information. - owner_map = {'theme_to_owners_map': {}, 'squad_to_theme_map': {}} # Maps owner names to a list of dotted module paths. # For example: { 'team-red': [ 'openedx.core.djangoapps.api_admin', 'openedx.core.djangoapps.auth_exchange' ] } owner_to_paths_map = {} - _map_repo_apps('edx-repo', repo_csv, override_csv, EDX_REPO_APPS, owner_map, owner_to_paths_map) - _map_repo_apps('3rd-party', dep_csv, override_csv, THIRD_PARTY_APPS, owner_map, owner_to_paths_map) - _map_edx_platform_apps(app_csv, owner_map, owner_to_paths_map) + _map_repo_apps('edx-repo', repo_csv, override_csv, EDX_REPO_APPS, owner_to_paths_map) + _map_repo_apps('3rd-party', dep_csv, override_csv, THIRD_PARTY_APPS, owner_to_paths_map) + _map_edx_platform_apps(app_csv, owner_to_paths_map) # NB: An automated script looks for this comment when updating config files, # so please update regenerate_code_owners_config.py in jenkins-job-dsl-internal # if you change the comment format here. - print(f'# Do not hand edit CODE_OWNER_MAPPINGS. Generated by {os.path.basename(__file__)}') - print('CODE_OWNER_MAPPINGS:') + print(f'# Do not hand edit CODE_OWNER_TO_PATH_MAPPINGS. Generated by {os.path.basename(__file__)}') + print('CODE_OWNER_TO_PATH_MAPPINGS:') for owner, path_list in sorted(owner_to_paths_map.items()): print(f" {owner}:") path_list.sort() for path in path_list: print(f" - {path}") - owner_with_mappings_set = set(owner_to_paths_map.keys()) - print(f'# Do not hand edit CODE_OWNER_THEMES. Generated by {os.path.basename(__file__)}') - print('CODE_OWNER_THEMES:') - for theme, owner_list in sorted(owner_map['theme_to_owners_map'].items()): - theme_owner_set = set(owner_list) - # only include the theme's list of owners that have mappings - theme_owner_with_mappings_list = list(theme_owner_set & owner_with_mappings_set) - if theme_owner_with_mappings_list: - print(f" {theme}:") - theme_owner_with_mappings_list.sort() - for owner in theme_owner_with_mappings_list: - print(f" - {owner}") - - -def _map_repo_apps(csv_type, repo_csv, override_csv, app_to_repo_map, owner_map, owner_to_paths_map): + +def _map_repo_apps(csv_type, repo_csv, override_csv, app_to_repo_map, owner_to_paths_map): """ - Reads CSV of repo ownership and uses app_to_repo_map to update owner_map and owner_to_paths_map + Reads CSV of repo ownership and uses app_to_repo_map to update owner_to_paths_map Only the paths in app_to_repo_map will be added to owner_to_paths_map. All repositories not corresponding to an app in app_to_repo_map will be ignored. @@ -180,7 +167,6 @@ def _map_repo_apps(csv_type, repo_csv, override_csv, app_to_repo_map, owner_map, repo_csv (string): File name for the edx-repo or 3rd-party repo csv override_csv (string): File name for the override csv (which may be None) app_to_repo_map (dict): Dict mapping Django apps to repo urls - owner_map (dict): Dict of owner details owner_to_paths_map (dict): Holds results mapping owner to paths """ @@ -194,13 +180,12 @@ def _map_repo_apps(csv_type, repo_csv, override_csv, app_to_repo_map, owner_map, override_data = override_file.read() override_reader = csv.DictReader(override_data.splitlines()) for row in override_reader: - # transform "app, owner" rows into a dict of {app: owner} - owner = _get_and_map_code_owner(row, owner_map) + owner = _get_code_owner(row) overrides[row['app']] = owner csv_repo_to_owner_map = {} for row in reader: - owner = _get_and_map_code_owner(row, owner_map) + owner = _get_code_owner(row) csv_repo_to_owner_map[row.get('repo url')] = owner for app, repo_url in app_to_repo_map.items(): @@ -220,7 +205,7 @@ def _map_repo_apps(csv_type, repo_csv, override_csv, app_to_repo_map, owner_map, ) -def _map_edx_platform_apps(app_csv, owner_map, owner_to_paths_map): +def _map_edx_platform_apps(app_csv, owner_to_paths_map): """ Reads CSV of edx-platform app ownership and updates mappings """ @@ -229,7 +214,7 @@ def _map_edx_platform_apps(app_csv, owner_map, owner_to_paths_map): reader = csv.DictReader(csv_data.splitlines()) for row in reader: path = row.get('Path') - owner = _get_and_map_code_owner(row, owner_map) + owner = _get_code_owner(row) # add paths that may have views may_have_views = re.match(r'.*djangoapps', path) or re.match(r'[./]*openedx\/features', path) @@ -249,47 +234,23 @@ def _map_edx_platform_apps(app_csv, owner_map, owner_to_paths_map): owner_to_paths_map[owner].append(path) -def _get_and_map_code_owner(row, owner_map): +def _get_code_owner(row): """ - From a csv row, takes the theme and squad, update ownership maps, and return the code_owner. - - Will also warn if the squad appears in multiple themes. + From a csv row, returns the squad as code_owner. Arguments: - row: A csv row that should have 'owner.theme' and 'owner.squad'. - owner_map: A dict with 'theme_to_owners_map' and 'squad_to_theme_map' keys. + row: A csv row that should have 'owner.squad'. Returns: - The code_owner for the row. This is made from the theme+squad (or squad if there is no theme). + The code_owner for the row, which comes from the squad name. """ - theme = row.get('owner.theme') squad = row.get('owner.squad') assert squad, 'Csv row is missing required owner.squad: %s' % row # use lower case names only squad = squad.lower() - if theme: - theme = theme.lower() - - owner = f'{theme}-{squad}' if theme else squad - theme = theme or squad - - if squad not in owner_map['squad_to_theme_map']: - # store the theme for each squad for a later data integrity check - owner_map['squad_to_theme_map'][squad] = theme - - # add to the list of owners for each theme - if theme not in owner_map['theme_to_owners_map']: - owner_map['theme_to_owners_map'][theme] = [] - owner_map['theme_to_owners_map'][theme].append(owner) - - # assert that squads have a unique theme. otherwise we have a data integrity issues in the csv. - assert owner_map['squad_to_theme_map'][squad] == theme, \ - 'Squad %s is associated with theme %s in row %s, but theme %s elsewhere in the csv.' % \ - (squad, theme, row, owner_map['squad_to_theme_map'][squad]) - - return owner + return squad if __name__ == "__main__":