diff --git a/Makefile b/Makefile index 17360b11d3..f6735a56e6 100644 --- a/Makefile +++ b/Makefile @@ -163,17 +163,11 @@ dev.checkout: ## Check out "openedx-release/$OPENEDX_RELEASE" in each repo if se dev.clone: dev.clone.ssh ## Clone service repos to the parent directory. -impl-dev.clone.https: ## Clone service repos using HTTPS method to the parent directory. - ./repo.sh clone - dev.clone.https: ## Clone service repos using HTTPS method to the parent directory. - @scripts/send_metrics.py wrap "$@" - -impl-dev.clone.ssh: ## Clone service repos using SSH method to the parent directory. - ./repo.sh clone_ssh + ./repo.sh clone dev.clone.ssh: ## Clone service repos using SSH method to the parent directory. - @scripts/send_metrics.py wrap "$@" + ./repo.sh clone_ssh ######################################################################################## # Developer interface: Docker image management. @@ -186,46 +180,30 @@ dev.prune: ## Prune dangling docker images, containers, and networks. Useful whe dev.pull.without-deps: _expects-service-list.dev.pull.without-deps dev.pull.without-deps.%: ## Pull latest Docker images for specific services. - @scripts/send_metrics.py wrap "dev.pull.without-deps.$*" - -impl-dev.pull.without-deps.%: ## Pull latest Docker images for specific services. docker compose pull $$(echo $* | tr + " ") dev.pull: - @scripts/send_metrics.py wrap "$@" - -impl-dev.pull: @scripts/make_warn_default_large.sh "dev.pull" dev.pull.large-and-slow: dev.pull.$(DEFAULT_SERVICES) ## Pull latest Docker images required by default services. @echo # at least one statement so that dev.pull.% doesn't run too -# Wildcards must be below anything they could match dev.pull.%: ## Pull latest Docker images for services and their dependencies. - @scripts/send_metrics.py wrap "dev.pull.$*" - -impl-dev.pull.%: ## Pull latest Docker images for services and their dependencies. docker compose pull --include-deps $$(echo $* | tr + " ") ######################################################################################## # Developer interface: Database management. ######################################################################################## -impl-dev.provision: ## Provision dev environment with default services, and then stop them. +dev.provision: ## Provision dev environment with default services, and then stop them. make dev.check-memory $(WINPTY) bash ./provision.sh $(DEFAULT_SERVICES) make dev.stop -dev.provision: ## Provision dev environment with default services, and then stop them. - @scripts/send_metrics.py wrap "$@" - -impl-dev.provision.%: dev.check-memory ## Provision specified services. +dev.provision.%: dev.check-memory ## Provision specified services. echo $* $(WINPTY) bash ./provision.sh $* -dev.provision.%: ## Provision specified services. - @scripts/send_metrics.py wrap "dev.provision.$*" - dev.backup: dev.up.mysql57+mysql80+mongo+elasticsearch710+opensearch12+coursegraph ## Write all data volumes to the host. docker run --rm --volumes-from $$(make --silent --no-print-directory dev.print-container.mysql57) -v $$(pwd)/.dev/backups:/backup debian:jessie tar zcvf /backup/mysql57.tar.gz /var/lib/mysql docker run --rm --volumes-from $$(make --silent --no-print-directory dev.print-container.mysql80) -v $$(pwd)/.dev/backups:/backup debian:jessie tar zcvf /backup/mysql80.tar.gz /var/lib/mysql @@ -273,11 +251,8 @@ dev.drop-db.%: ## Irreversably drop the contents of a MySQL database in each mys dev.up.attach: _expects-service.dev.up.attach -impl-dev.up.attach.%: ## Bring up a service and its dependencies + and attach to it. - docker compose up $* - dev.up.attach.%: ## Bring up a service and its dependencies + and attach to it. - @scripts/send_metrics.py wrap "dev.up.attach.$*" + docker compose up $* dev.up.shell: _expects-service.dev.up.shell @@ -297,12 +272,9 @@ dev.up.with-watchers.%: ## Bring up services and their dependencies + asset watc dev.up.without-deps: _expects-service-list.dev.up.without-deps -impl-dev.up.without-deps.%: dev.check-memory ## Bring up services by themselves. +dev.up.without-deps.%: dev.check-memory ## Bring up services by themselves. docker compose up -d --no-deps $$(echo $* | tr + " ") -dev.up.without-deps.%: ## Bring up services by themselves. - @scripts/send_metrics.py wrap "dev.up.without-deps.$*" - dev.up.without-deps.shell: _expects-service.dev.up.without-deps.shell dev.up.without-deps.shell.%: ## Bring up a service by itself + shell into it. @@ -315,15 +287,12 @@ dev.up: dev.up.large-and-slow: dev.up.$(DEFAULT_SERVICES) ## Bring up default services. @echo # at least one statement so that dev.up.% doesn't run too -impl-dev.up.%: dev.check-memory ## Bring up services and their dependencies. +dev.up.%: dev.check-memory ## Bring up services and their dependencies. docker compose up -d $$(echo $* | tr + " ") ifeq ($(ALWAYS_CACHE_PROGRAMS),true) make dev.cache-programs endif -# Wildcards must be below anything they could match -dev.up.%: - @scripts/send_metrics.py wrap "dev.up.$*" dev.ps: ## View list of created services and their statuses. docker compose ps @@ -599,17 +568,6 @@ _expects-database.%: @echo " make $*.edxapp" -######################################################################################## -# Convenient ways to opt in or out of devstack usage metrics reporting -######################################################################################## - -metrics-opt-in: ## To opt into basic data collection to help improve devstack - @./scripts/send_metrics.py opt-in - -metrics-opt-out: ## To opt out of metrics data collection - @./scripts/send_metrics.py opt-out - - ######################################################################################## # Miscellaneous targets. # These are useful, but don't fit nicely to the greater Devstack interface. diff --git a/scripts/send_metrics.py b/scripts/send_metrics.py deleted file mode 100755 index c35785c09a..0000000000 --- a/scripts/send_metrics.py +++ /dev/null @@ -1,453 +0,0 @@ -#!/usr/bin/env python3 -""" -Python script to collect metrics on devstack make targets. - -If the devstack user has consented to metrics collected, this script -calls the specified make target and records metrics about the target, -execution time, and other attributes that can be used to monitor the -usability of devstack. For more information: - -https://openedx.atlassian.net/wiki/spaces/AC/pages/2720432206/Devstack+Metrics - -If environment variable ``DEVSTACK_METRICS_TESTING`` is present and non-empty, then -metrics will be annotated to indicate that the event occurred in the -course of running tests so that it does not pollute our main data. -(Extra debugging information will also be printed, including the sent -event data.) - -Config file schema: - -- 'anonymous_user_id' (string) high-entropy, non-identifying unique user ID -- 'consent' (dict) present when user has either consented or declined - metrics collection - - 'decision' (boolean) indicates whether the user has consented or declined - - 'timestamp' (string) ISO-8601 date-time when the user consented or declined -""" - -import base64 -import json -import os -import subprocess -import sys -import traceback -import urllib.request as req -import uuid -from datetime import datetime, timedelta, timezone -from http.client import RemoteDisconnected -from os import path -from signal import SIG_DFL, SIGINT, signal -from urllib.error import URLError - - -test_mode = os.environ.get('DEVSTACK_METRICS_TESTING') - -# Provisioned as a separate source particular to devstack -segment_write_key = "MUymmyrKDLk6JVwtkveX6OHVKMhpApou" - -config_dir = path.expanduser("~/.config/devstack") -config_path = path.join(config_dir, "metrics.json") - - -def base64str(s): - """Encode a string in Base64, returning a string.""" - return base64.b64encode(s.encode()).decode() - - -def check_consent(): - """ - Check if it's OK to send metrics, returning dict of: - - - 'ok_to_collect' (boolean) True when metrics may be collected - - 'config' (dict) config data, present if `ok_to_collect` is True - - 'print_invitation' (boolean) True when an invitation to consent to - metrics collection should be printed for the user - - May throw an exception if config file cannot be parsed. - """ - try: - with open(config_path, 'r') as f: - config = json.loads(f.read()) - except FileNotFoundError: - return {'ok_to_collect': False, 'print_invitation': True} - - decision = config.get('consent', {}).get('decision') - - # Explicit decline. - if decision is False: - return {'ok_to_collect': False, 'print_invitation': False} - - # Anything other than consent: Just invite. (If they haven't - # declined *or* consented, either they've not made a decision - # (value is None) or there's some invalid value that will be - # cleared out by the opt-in/out process.) - if decision is not True: - return {'ok_to_collect': False, 'print_invitation': True} - - # At this point, we know they've consented, but one last check... - - # Opt-in process should have set an anonymous user ID, which is - # required for sending events. - if not config.get('anonymous_user_id'): - # Something's wrong with the config file if they've consented - # but not been assigned an ID, so just pretend they've not - # consented -- opting in should reset things. - return {'ok_to_collect': False, 'print_invitation': True} - - # Consented, so no need to invite. - return { - 'ok_to_collect': True, - 'config': config, - 'print_invitation': False - } - - -def send_metrics_to_segment(event_type, event_properties, config): - """ - Send collected metrics to Segment. - - May throw. - """ - # Get a shallow copy of the input and annotate it as a non-test - # event unless overridden by environment variable. - event_properties = event_properties.copy() - event_properties['is_test'] = test_mode or 'no' - - event = { - 'event': event_type, - 'userId': config['anonymous_user_id'], - 'properties': event_properties, - 'sentAt': datetime.now(timezone.utc).isoformat(), - } - - if test_mode: - print("Send metrics info: %s" % json.dumps(event), file=sys.stderr) - - # https://segment.com/docs/connections/sources/catalog/libraries/server/http-api/ - headers = { - 'Authorization': 'Basic ' + base64str(segment_write_key + ':'), - 'Content-Type': 'application/json', - 'User-Agent': 'edx-devstack-send-metrics', - } - request = req.Request( - url = 'https://api.segment.io/v1/track', - method = 'POST', - headers = headers, - data = json.dumps(event).encode(), - ) - - try: - with req.urlopen(request, timeout=8) as resp: - status_code = resp.getcode() - if status_code != 200 and test_mode: - print("Segment metrics send returned an unexpected status code ${status_code}", file=sys.stderr) - # Might just be a Segment outage; user probably doesn't care. - # Other errors can bubble up to a layer that might report them. - except (RemoteDisconnected, URLError): - if test_mode: - traceback.print_exc() - - -def read_git_state(): - """ - Return additional, git-related attributes to be merged into event - properties. - """ - # %cI gets the committer timestamp, rather than the author - # timestamp; the latter could be older when commits have been - # rewritten, and the former is more likely to be of interest when - # looking at repo checkout age. - process = subprocess.run( - ['git', 'show', '--no-patch', '--pretty=format:%cI|%D'], - capture_output=True, check=True, timeout=5 - ) - timestamp, reflist = process.stdout.decode().split('|', 2) - # Returns true if master is currently checked out. Returns false - # otherwise, which includes the following situations that are similar - # to, but different from a master checkout: - # - # - Detached-head state which happens to be on same commit as master - # - Another branch is checked out but points to the same commit as - # master - # - On commit which *used to* be the tip of master, but is no longer - # (and is just in master's history) - is_at_master = "HEAD -> master" in reflist.split(', ') - return { - 'git_commit_time': timestamp, - 'git_checked_out_master': is_at_master, - } - - -def run_wrapped(make_target, config): - """ - Runs specified make shell target and collects performance data. - - Return exit code of process (negative for signals). - """ - # Do as much as possible inside try blocks - do_collect = True - try: - start_time = datetime.now(timezone.utc) - - # Catch SIGINT (but only once) so that we can report an event - # when the developer uses Ctrl-C to kill the make command - # (which would normally kill this process as well). This - # handler just ignores the signal and then unregisters itself. - # - # There is no guarantee of whether this signal will be caught - # first or the child process will exit first. This could - # theoretically result in unregistering the signal handler - # before the signal has occurred, resulting in failure to - # report the event, but there's no way to prevent this. - signal(SIGINT, lambda _signum, _frame: signal(SIGINT, SIG_DFL)) - except: - do_collect = False - traceback.print_exc() - print("Metrics collection setup failed. " - "If this keeps happening, please let the Architecture team know. " - "(This should not affect the outcome of your make command.)", - file=sys.stderr) - - completed_process = run_target(make_target) - subprocess_exit_code = completed_process.returncode - - # Do as much as possible inside try blocks - try: - # Skip metrics reporting if setup failed - if not do_collect: - return subprocess_exit_code - - signal(SIGINT, SIG_DFL) # stop trapping SIGINT (if haven't already) - end_time = datetime.now(timezone.utc) - time_diff_millis = (end_time - start_time) // timedelta(milliseconds=1) - # Must be compatible with our Segment schema, and must not be - # expanded to include additional attributes without an - # additional consent check. - event_properties = { - 'command_type': 'make', - 'command': make_target[:50], # limit in case of mis-pastes at terminal - 'start_time': start_time.isoformat(), - 'duration_ms': time_diff_millis, - # If the subprocess was interrupted by a signal, the exit - # code will be negative signal value (e.g. -2 for SIGINT, - # rather than the 130 it returns from the shell): - # https://docs.python.org/3.8/library/subprocess.html#subprocess.CompletedProcess - # - # If a make subprocess exits non-zero, make exits with code 2. - 'exit_status': subprocess_exit_code, - **read_git_state() - } - send_metrics_to_segment('devstack.command.run', event_properties, config) - except: - # We don't want to warn about transient Segment outages or - # similar, but there might be a coding error in the - # send-metrics script. - traceback.print_exc() - print("Failed to send devstack metrics to Segment. " - "If this keeps happening, please let the Architecture team know. " - "(This should not have affected the outcome of your make command.)", - file=sys.stderr) - - return subprocess_exit_code - - -def run_target(make_target): - """ - Just run make on the given target. - - Return exit code of process (negative for signals). - """ - return subprocess.run(["make", "impl-%s" % make_target], check=False) - - -def do_wrap(make_target): - """ - Run the given make target, and collect and report data if and only if - the user has consented to data collection. - - Return exit code to exit with (signals become 128 + signal value). - """ - try: - consent_state = check_consent() - except Exception as e: # don't let errors interrupt dev's work - if test_mode: - print("Metrics disabled due to startup error: %r" % e) - consent_state = {} - - if consent_state.get('ok_to_collect'): - subprocess_exit_code = run_wrapped(make_target, consent_state.get('config')) - else: - subprocess_exit_code = run_target(make_target).returncode - if consent_state.get('print_invitation'): - print( - "Would you like to assist devstack development by sending " - "anonymous usage metrics to edX? Run `make metrics-opt-in` " - "to learn more!", - file=sys.stderr - ) - - if subprocess_exit_code < 0: - # Translate to shell convention. - return 128 + -subprocess_exit_code - else: - return subprocess_exit_code - - -def do_opt_in(): - """ - Perform the interactive opt-in process. - """ - start_time = datetime.now(timezone.utc) - try: - with open(config_path, 'r') as f: - config = json.loads(f.read()) - except FileNotFoundError: - config = {} - - # Only short-circuit here if consented *and* all necessary info present. - if config.get('consent', {}).get('decision') and config.get('anonymous_user_id'): - print( - "It appears you've previously opted-in to metrics reporting. " - "Recorded consent: {record!r}" - .format(record=config['consent']) - ) - return - # variables used to bold relevant text in the consent language below - MARKUP_BOLD = '\033[1m' - MARKUP_END = '\033[0m' - - # NOTE: This is required for informed consent on the part of the users opting-in. - # The types of data collected cannot be expanded or changed without legal review. - print( - "\n" - + MARKUP_BOLD + "Allow devstack to report anonymized usage metrics?\n" + MARKUP_END + - "\n" - "This will report usage information to edX so that devstack improvements can be planned and evaluated. " - "The metrics and attributes to be collected by edX include an anonymous user ID and information about devstack command calls (e.g. make targets, exit codes, and command durations), " - "the OS, and git repo state." - "\n\n" - + MARKUP_BOLD + "Type 'yes' or 'y' to opt in to reporting this information, or anything else to cancel. " + MARKUP_END + - "\n\n" - "You may withdraw this permission and stop sending such metrics at any time by running ‘make metrics-opt-out’." - ) - answer = input() - print() - - if answer.lower() in ['yes', 'y']: - config['consent'] = { - 'decision': True, - 'timestamp': datetime.now(timezone.utc).isoformat() - } - # Set an anonymous user ID on first opt-in, but preserve it if - # they're toggling back and forth. - config['anonymous_user_id'] = config.get('anonymous_user_id') or str(uuid.uuid4()) - - os.makedirs(config_dir, exist_ok=True) - with open(config_path, 'w') as f: - f.write(json.dumps(config)) - - print( - "Thank you for contributing to devstack development! " - "Your opt-in has been stored in {config_path} and " - "you can opt out again at any time with `make metrics-opt-out`." - .format(config_path=config_path) - ) - # Send record of opt-in so we can tell whether people are - # opting in even if they're not running any of the - # instrumented commands. - event_properties = { - 'command_type': 'make', - 'command': 'metrics-opt-in', - 'choice': 'accept', - # Note: Not quite the same as the time they accepted - # (could have left prompt open for a while). - 'start_time': start_time.isoformat(), - # This tells us what version of the consent check was - # presented to the user. - **read_git_state() - } - send_metrics_to_segment('devstack.command.run', event_properties, config) - else: - print("Cancelled opt-in.") - - -def do_opt_out(): - """ - Opt the user out. - """ - start_time = datetime.now(timezone.utc) - - os.makedirs(config_dir, exist_ok=True) - try: - with open(config_path, 'r') as f: - config = json.loads(f.read()) - except FileNotFoundError: - config = {} - - had_consented = config.get('consent', {}).get('decision') is True - - config['consent'] = { - 'decision': False, - 'timestamp': datetime.now(timezone.utc).isoformat() - } - with open(config_path, 'w') as f: - f.write(json.dumps(config)) - - print( - "You have been opted out of reporting devstack command metrics to edX. " - "No further usage metrics will be sent. " - "This preference is stored in a config file located at {config_path}; " - "you can opt back in by running `make metrics-opt-in` at any time." - .format(config_path=config_path) - ) - - # Only send an event when someone had previously consented -- this - # allows us to keep a record of the start and end of their - # consent. - if had_consented: - # Collect as little information as possible for this event - event_properties = { - 'command_type': 'make', - 'command': 'metrics-opt-out', - 'previous_consent': 'yes', - 'start_time': start_time.isoformat() - } - send_metrics_to_segment('devstack.command.run', event_properties, config) - - -def main(args): - if len(args) == 0: - print( - "Usage:\n" - " send_metrics.py wrap \n" - " send_metrics.py opt-in\n" - " send_metrics.py opt-out", - file=sys.stderr - ) - sys.exit(1) - action = args[0] - action_args = args[1:] - - # Dispatch - if action == 'wrap': - if len(action_args) != 1: - print("send-metrics wrap takes one argument", file=sys.stderr) - sys.exit(1) - conveyed_exit_code = do_wrap(action_args[0]) - sys.exit(conveyed_exit_code) - elif action == 'opt-in': - if len(action_args) != 0: - print("send-metrics opt-in takes zero arguments", file=sys.stderr) - sys.exit(1) - do_opt_in() - elif action == 'opt-out': - if len(action_args) != 0: - print("send-metrics opt-out takes zero arguments", file=sys.stderr) - sys.exit(1) - do_opt_out() - else: - print("Unrecognized action: %s" % action, file=sys.stderr) - sys.exit(1) - - -if __name__ == "__main__": - main(sys.argv[1:]) diff --git a/tests/metrics.py b/tests/metrics.py deleted file mode 100644 index 37ebb37e9e..0000000000 --- a/tests/metrics.py +++ /dev/null @@ -1,350 +0,0 @@ -""" -Tests for send_metrics.py -""" - -import json -import os -import re -from contextlib import contextmanager - -import pexpect -from pexpect import EOF - - -#### Utilities - -## Substrings to use as probes: -invitation = 'Would you like to assist devstack development' -consent_question = "Allow devstack to report anonymized usage metrics?" - -config_dir = os.path.expanduser('~/.config/devstack') -config_path = os.path.join(config_dir, 'metrics.json') - -@contextmanager -def environment_as(config_data): - """ - Context manager: Set up environment for metrics testing, or fail if there's - something wrong in the environment which can't be fixed. - - If `config_data` is not None, write it as JSON to the config file and - remove it again after the wrapped code runs. - """ - assert os.environ.get('DEVSTACK_METRICS_TESTING'), \ - "You need a DEVSTACK_METRICS_TESTING=debug if running this test " \ - "locally since this environment variable both enables printing of " \ - "metrics and also marks sent metric events as test data." - - assert not os.path.isfile(config_path), \ - "You already have a config file; failing now to avoid overwriting it." - - if config_data is not None: - os.makedirs(config_dir, exist_ok=True) - with open(config_path, 'w') as f: - f.write(json.dumps(config_data)) - - try: - yield - finally: - # Clean up before exiting. - if config_data is not None: - with open(config_path, 'r') as f: - # For debugging... - print("Metrics config file in effect was: " + f.read()) - - try: - os.remove(config_path) - except FileNotFoundError: - pass - - -def do_opt_in(): - """Opt in (and assert it worked).""" - p = pexpect.spawn('make metrics-opt-in', timeout=10) - p.expect("Type 'yes' or 'y'") - p.sendline('yes') - p.expect(EOF) - p.close() - assert_consent(True) - - -def assert_consent(status=True): - """ - Assert consent status in config file. status=True will check for consent - with timestamp, False will check for a decline with timestamp, and None - will check that the consent dict is missing. - """ - try: - with open(config_path, 'r') as f: - config = json.loads(f.read()) - except FileNotFoundError: - config = FileNotFoundError - - if status is None: - assert config is FileNotFoundError or 'consent' not in config - else: - assert isinstance(status, bool) - assert 'consent' in config - consent = config['consent'] - assert consent.get('decision') == status - # Timestamp should be a date at least (likely also has a time) - assert re.match(r'^[0-9]{4}-[0-9]{2}-[0-9]{2}', consent.get('timestamp')) - - -#### Opting in and out - -def test_initial_opt_in_accept(): - """ - Test that a consent check is provided, and what happens on accept. - """ - with environment_as(None): - p = pexpect.spawn('make metrics-opt-in', timeout=10) - p.expect_exact(consent_question) - p.expect("Type 'yes' or 'y'") - p.sendline("yes") - p.expect("metrics-opt-out") # prints instructions for opt-out - # Check that a metric is sent for opt-in - p.expect("Send metrics info:") - p.expect(EOF) - metrics_json = p.before.decode() - - data = json.loads(metrics_json) - # These keys are defined by a central document; do not send - # additional metrics without specifying them there first: - # - # https://openedx.atlassian.net/wiki/spaces/AC/pages/2720432206/Devstack+Metrics - # - # Additional metrics require approval (as do changes to - # existing ones). Changes to metrics also require an update to the - # list of metrics displayed during opt-in. - assert sorted(data.keys()) == ['event', 'properties', 'sentAt', 'userId'], \ - "Unrecognized key in envelope -- confirm that this addition is authorized." - assert sorted(data['properties'].keys()) == [ - 'choice', 'command', 'command_type', - 'git_checked_out_master', 'git_commit_time', - 'is_test', 'start_time', - ], "Unrecognized attribute -- confirm that this addition is authorized." - - assert data['event'] == 'devstack.command.run' - assert data['properties']['command_type'] == 'make' - assert data['properties']['command'] == 'metrics-opt-in' - assert data['properties']['choice'] == 'accept' - - assert_consent(True) - - -def test_initial_opt_in_decline(): - """ - Test that a consent check is provided, and what happens on decline. - """ - with environment_as(None): - p = pexpect.spawn('make metrics-opt-in', timeout=10) - p.sendline("") # empty response - p.expect(EOF) - - assert 'Send metrics info:' not in p.before.decode() - # No consent info stored on decline - assert_consent(None) - - -def test_initial_opt_out(): - """ - Test that opt-out always marks consent=False. - """ - with environment_as(None): - p = pexpect.spawn('make metrics-opt-out', timeout=10) - p.expect('metrics-opt-in') # indicates how to undo - p.expect(EOF) - - assert 'Send metrics info:' not in p.before.decode() - assert_consent(False) - - -def test_later_opt_out(): - """ - Test that opt-out after previously opting in sends an event. - """ - with environment_as(None): - do_opt_in() - p = pexpect.spawn('make metrics-opt-out', timeout=10) - p.expect('metrics-opt-in') - p.expect(r'Send metrics info:') - p.expect(EOF) - metrics_json = p.before.decode() - - data = json.loads(metrics_json) - # These keys are defined by a central document; do not send - # additional metrics without specifying them there first: - # - # https://openedx.atlassian.net/wiki/spaces/AC/pages/2720432206/Devstack+Metrics - # - # Additional metrics require approval (as do changes to - # existing ones). - assert sorted(data.keys()) == ['event', 'properties', 'sentAt', 'userId'], \ - "Unrecognized key in envelope -- confirm that this addition is authorized." - assert sorted(data['properties'].keys()) == [ - 'command', 'command_type', 'is_test', 'previous_consent', 'start_time', - ], "Unrecognized attribute -- confirm that this addition is authorized." - - assert data['event'] == 'devstack.command.run' - assert data['properties']['command_type'] == 'make' - assert data['properties']['command'] == 'metrics-opt-out' - assert data['properties']['previous_consent'] == 'yes' - - assert_consent(False) - - -#### Collection, or not, for an instrumented make target - -def test_invitation(): - """ - Test that an invitation is printed when consent record isn't present. - """ - with environment_as(None): - p = pexpect.spawn('make dev.up.redis', timeout=60) - p.expect(EOF) - assert 'Send metrics info:' not in p.before.decode() - assert invitation in p.before.decode() - - -def test_enabled_but_declined(): - """ - Test that no invitation is printed when declined, even with feature - flag enabled. - """ - with environment_as({ - 'consent': {'decision': False, 'timestamp': '2021-05-20T21:42:18.365201+00:00'} - }): - assert_consent(False) - - p = pexpect.spawn('make dev.up.redis', timeout=60) - p.expect(EOF) - assert 'Send metrics info:' not in p.before.decode() - assert invitation not in p.before.decode() - - -def test_feature_flag_does_not_gate_collection(): - """ - Test that feature flag no longer gates collection. - """ - with environment_as(None): - do_opt_in() - p = pexpect.spawn('make dev.up.redis', timeout=60) - p.expect(EOF) - assert 'Send metrics info:' in p.before.decode() - assert invitation not in p.before.decode() - - -def test_no_arbitrary_target_instrumented(): - """ - Test that arbitrary make targets are not instrumented. - """ - with environment_as(None): - do_opt_in() - p = pexpect.spawn('make xxxxx', timeout=60) - p.expect(EOF) - assert 'Send metrics info:' not in p.before.decode() - assert invitation not in p.before.decode() - - # Also confirm that the exit code is conveyed properly - p.close() - assert p.exitstatus == 2 # make's generic error code - assert p.signalstatus is None - - -def test_metrics(): - """ - Test that dev.up.% is instrumented for metrics collection. - """ - with environment_as(None): - do_opt_in() - p = pexpect.spawn('make dev.up.redis', timeout=60) - p.expect(r'Send metrics info:') - assert invitation not in p.before.decode() - p.expect(EOF) - assert invitation not in p.before.decode() - metrics_json = p.before.decode() - - data = json.loads(metrics_json) - # These keys are defined by a central document; do not send - # additional metrics without specifying them there first: - # - # https://openedx.atlassian.net/wiki/spaces/AC/pages/2720432206/Devstack+Metrics - # - # Additional metrics require approval (as do changes to - # existing ones). - assert sorted(data.keys()) == ['event', 'properties', 'sentAt', 'userId'], \ - "Unrecognized key in envelope -- confirm that this addition is authorized." - assert sorted(data['properties'].keys()) == [ - 'command', 'command_type', 'duration_ms', 'exit_status', - 'git_checked_out_master', 'git_commit_time', - 'is_test', 'start_time', - ], "Unrecognized attribute -- confirm that this addition is authorized." - - assert data['event'] == 'devstack.command.run' - assert data['properties']['command'] == 'dev.up.redis' - # Any string but 'no', really (will match env var in practice) - assert data['properties']['is_test'] in ['ci', 'debug'] - - -def test_handle_ctrl_c(): - """ - Test that wrapper can survive and report on a Ctrl-C. - """ - with environment_as(None): - do_opt_in() - p = pexpect.spawn('make dev.pull', timeout=60) - # Make sure wrapped command has started before we interrupt, - # otherwise signal handler won't even have been registered - # yet. - p.expect(r'Are you sure you want to run this command') - p.sendintr() # send Ctrl-C to process group - p.expect(EOF) - output = p.before.decode() - - assert re.search(r'make\[[0-9]+\]: [^\r\n]+ Interrupt', output) - # confirm docker has stopped - assert 'Pulling ' not in output - - metrics_match = re.search(r'Send metrics info: ([^\r\n]+)', output) - assert metrics_match - data = json.loads(metrics_match.group(1)) - - # Exit status is negative of signal's value (SIGINT = 2) - assert data['properties']['exit_status'] == -2 - - # Exit signal here actually comes from make, so this doesn't - # really test the wrapper's own exit code. This assertion - # really just serves as documentation of behavior. - p.close() - assert p.exitstatus == None - assert p.signalstatus is 2 - - -def test_signal_conversion(): - """ - This is like test_handle_ctrl_c, except calling the wrapper - directly to avoid Make's conversion of all exit codes to 2. - """ - with environment_as(None): - do_opt_in() - p = pexpect.spawn('scripts/send_metrics.py wrap dev.pull', timeout=60) - # Make sure wrapped command has started before we interrupt, - # otherwise signal handler won't even have been registered - # yet. - p.expect(r'Are you sure you want to run this command') - p.sendintr() # send Ctrl-C to process group - # Confirm that the process is actually catching the signal, as - # proven by it printing some things before ending. - p.expect(r'Send metrics info:') - p.expect(EOF) - - # This time we're calling the script directly, so we see the - # script exiting with code 130 (128 + SIGINT). - # - # ...or, depending on timing and/or operating system, the make - # process may die *first* with its generic exit code 2 and the - # wrapper would exit before it even receives the signal. So, we - # expect either scenario. - p.close() - assert p.exitstatus in [130, 2] - assert p.signalstatus is None