From 8e922ddb6c4b9d8fed321f3dc7eadd5ea26d3eff Mon Sep 17 00:00:00 2001 From: Alexander Pantiukhov Date: Fri, 5 Nov 2021 13:50:42 +0100 Subject: [PATCH] Success markers telemetry (#10065) * Markers telemetry * Everything without tests * Specified events.json * Test added * Changelog entry * Naming fixes * Fix lint, fix CLI bug No way to actually change config path, now fixed with nargs * Add markers parsed telemetry * Document telemetry functions * always add ANY_MARKER; add test * Add complexity telemetry * Skip root marker to avoid double reporting total marker count Co-authored-by: Matthew Summers Co-authored-by: ka-bu --- changelog/10065.misc.md | 1 + docs/docs/telemetry/events.json | 86 ++++++++++++++++++++++++++++ rasa/cli/evaluate.py | 19 ++++++ rasa/core/evaluation/marker_base.py | 27 +++++++-- rasa/telemetry.py | 73 +++++++++++++++++++++++ tests/core/evaluation/test_marker.py | 46 +++++++++++++++ tests/test_telemetry.py | 10 +++- 7 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 changelog/10065.misc.md diff --git a/changelog/10065.misc.md b/changelog/10065.misc.md new file mode 100644 index 000000000000..259e2eb009fe --- /dev/null +++ b/changelog/10065.misc.md @@ -0,0 +1 @@ +Add telemetry to markers extraction. \ No newline at end of file diff --git a/docs/docs/telemetry/events.json b/docs/docs/telemetry/events.json index 94017d0d4eb0..2ca399ba116a 100644 --- a/docs/docs/telemetry/events.json +++ b/docs/docs/telemetry/events.json @@ -3,6 +3,7 @@ "Model Training", "Model Testing", "Model Serving", + "Markers Extraction", "Data Handling", "Miscellaneous" ], @@ -422,6 +423,91 @@ "num_synonyms", "num_regexes" ] + }, + "Markers Extraction Initiated": { + "description": "Triggered when marker extraction has been initiated.", + "type": "object", + "section": "Markers Extraction", + "properties": { + "strategy": { + "type": "string", + "description": "Strategy to use when selecting trackers to extract from." + }, + "only_extract": { + "type": "boolean", + "description": "Indicates if path to write out statistics hasn't been specified." + }, + "seed": { + "type": "boolean", + "description": "The seed to initialise the random number generator for use with the 'sample' strategy." + }, + "count": { + "type": ["integer", "null"], + "description": "Number of trackers to extract from (for any strategy except 'all')." + } + }, + "additionalProperties": false, + "required": [ + "strategy", + "only_extract", + "seed", + "count" + ] + }, + "Markers Extracted": { + "description": "Triggered when markers have been extracted.", + "type": "object", + "section": "Markers Extraction", + "properties": { + "trackers_count": { + "type": "integer", + "description": "Number of processed trackers." + } + }, + "additionalProperties": false, + "required": [ + "trackers_count" + ] + }, + "Markers Parsed": { + "description": "Triggered when markers have been successfully parsed.", + "type": "object", + "section": "Markers Extraction", + "properties": { + "marker_count": { + "type": "integer", + "description": "Number of parsed markers." + }, + "max_depth": { + "type": "integer", + "description": "Maximum depth of the parsed markers." + }, + "branching_factor": { + "type": "integer", + "description": "Maximum number of children of any of the parsed markers." + } + }, + "additionalProperties": false, + "required": [ + "marker_count", + "max_depth", + "branching_factor" + ] + }, + "Markers Statistics Computed": { + "description": "Triggered when marker statistics have been computed.", + "type": "object", + "section": "Markers Extraction", + "properties": { + "trackers_count": { + "type": "integer", + "description": "Number of processed trackers." + } + }, + "additionalProperties": false, + "required": [ + "trackers_count" + ] } } } diff --git a/rasa/cli/evaluate.py b/rasa/cli/evaluate.py index 5352d63ccad3..3869112c1b88 100644 --- a/rasa/cli/evaluate.py +++ b/rasa/cli/evaluate.py @@ -2,6 +2,7 @@ from typing import List, Text, Optional from pathlib import Path +from rasa import telemetry from rasa.core.utils import AvailableEndpoints from rasa.core.tracker_store import TrackerStore from rasa.core.evaluation.marker_tracker_loader import MarkerTrackerLoader @@ -131,6 +132,13 @@ def _run_markers( computed per session will be stored in '/statistics-per-session.csv'. """ + telemetry.track_markers_extraction_initiated( + strategy=strategy, + only_extract=stats_file_prefix is not None, + seed=seed is not None, + count=count, + ) + domain = Domain.load(domain_path) if domain_path else None markers = Marker.from_path(config) if domain and not markers.validate_against_domain(domain): @@ -139,6 +147,17 @@ def _run_markers( "Please see errors listed above and fix before running again." ) + # Calculate telemetry + # Subtract one to remove the virtual OR over all markers + num_markers = len(markers) - 1 + max_depth = markers.max_depth() - 1 + # Find maximum branching of marker + branching_factor = max( + len(sub_marker) - 1 for marker in markers.sub_markers for sub_marker in marker + ) + + telemetry.track_markers_parsed_count(num_markers, max_depth, branching_factor) + tracker_loader = _create_tracker_loader(endpoint_config, strategy, count, seed) def _append_suffix(path: Optional[Path], suffix: Text) -> Optional[Path]: diff --git a/rasa/core/evaluation/marker_base.py b/rasa/core/evaluation/marker_base.py index ae22bbe0f698..cdad03162dda 100644 --- a/rasa/core/evaluation/marker_base.py +++ b/rasa/core/evaluation/marker_base.py @@ -26,6 +26,7 @@ from rasa.shared.data import is_likely_yaml_file from rasa.shared.exceptions import InvalidConfigException, RasaException from rasa.shared.core.events import ActionExecuted, UserUttered, Event +from rasa import telemetry from rasa.shared.core.domain import Domain from rasa.shared.core.trackers import DialogueStateTracker from rasa.utils.io import WriteRow @@ -271,6 +272,11 @@ def validate_against_domain(self, domain: Domain) -> bool: """ ... + @abstractmethod + def max_depth(self) -> int: + """Gets the maximum depth from this point in the marker tree.""" + ... + def evaluate_events( self, events: List[Event], recursive: bool = False ) -> List[SessionEvaluation]: @@ -447,12 +453,8 @@ def from_path(cls, path: Union[Path, Text]) -> Marker: loaded_markers.append(marker) # combine the markers - if len(loaded_markers) > 1: - marker = OrMarker(markers=loaded_markers) - marker.name = Marker.ANY_MARKER # cannot be set via name parameter - else: - marker = loaded_markers[0] - + marker = OrMarker(markers=loaded_markers) + marker.name = Marker.ANY_MARKER # cannot be set via name parameter return marker @staticmethod @@ -608,6 +610,9 @@ def evaluate_trackers( if tracker: tracker_result = self.evaluate_events(tracker.events) processed_trackers[tracker.sender_id] = tracker_result + + processed_trackers_count = len(processed_trackers) + telemetry.track_markers_extracted(processed_trackers_count) Marker._save_results(output_file, processed_trackers) # Compute and write statistics if requested. @@ -622,6 +627,8 @@ def evaluate_trackers( session_idx=session_idx, meta_data_on_relevant_events_per_marker=session_result, ) + + telemetry.track_markers_stats_computed(processed_trackers_count) if overall_stats_file: stats.overall_statistic_to_csv(path=overall_stats_file) if session_stats_file: @@ -756,6 +763,10 @@ def validate_against_domain(self, domain: Domain) -> bool: marker.validate_against_domain(domain) for marker in self.sub_markers ) + def max_depth(self) -> int: + """Gets the maximum depth from this point in the marker tree.""" + return 1 + max(child.max_depth() for child in self.sub_markers) + @staticmethod def from_tag_and_sub_config( tag: Text, sub_config: Any, name: Optional[Text] = None, @@ -837,6 +848,10 @@ def __len__(self) -> int: """Returns the count of all markers that are part of this marker.""" return 1 + def max_depth(self) -> int: + """Gets the maximum depth from this point in the marker tree.""" + return 1 + @staticmethod def from_tag_and_sub_config( tag: Text, sub_config: Any, name: Optional[Text] = None diff --git a/rasa/telemetry.py b/rasa/telemetry.py index af4050567da9..4a5377525ee1 100644 --- a/rasa/telemetry.py +++ b/rasa/telemetry.py @@ -94,6 +94,10 @@ TELEMETRY_VISUALIZATION_STARTED_EVENT = "Story Visualization Started" TELEMETRY_TEST_CORE_EVENT = "Model Core Tested" TELEMETRY_TEST_NLU_EVENT = "Model NLU Tested" +TELEMETRY_MARKERS_EXTRACTION_INITIATED_EVENT = "Markers Extraction Initiated" +TELEMETRY_MARKERS_EXTRACTED_EVENT = "Markers Extracted" +TELEMETRY_MARKERS_STATS_COMPUTED_EVENT = "Markers Statistics Computed" +TELEMETRY_MARKERS_PARSED_COUNT = "Markers Parsed" # used to calculate the context on the first call and cache it afterwards TELEMETRY_CONTEXT = None @@ -1005,3 +1009,72 @@ def track_nlu_model_test(test_data: "TrainingData") -> None: "num_regexes": len(test_data.regex_features), }, ) + + +@ensure_telemetry_enabled +def track_markers_extraction_initiated( + strategy: Text, only_extract: bool, seed: bool, count: Optional[int], +) -> None: + """Track when a user tries to extract success markers. + + Args: + strategy: The strategy the user is using for tracker selection + only_extract: Indicates if the user is only extracting markers or also + producing stats + seed: Indicates if the user used a seed for this attempt + count: (Optional) The number of trackers the user is trying to select. + """ + _track( + TELEMETRY_MARKERS_EXTRACTION_INITIATED_EVENT, + { + "strategy": strategy, + "only_extract": only_extract, + "seed": seed, + "count": count, + }, + ) + + +@ensure_telemetry_enabled +def track_markers_extracted(trackers_count: int) -> None: + """Track when markers have been extracted by a user. + + Args: + trackers_count: The actual number of trackers processed + """ + _track( + TELEMETRY_MARKERS_EXTRACTED_EVENT, {"trackers_count": trackers_count}, + ) + + +@ensure_telemetry_enabled +def track_markers_stats_computed(trackers_count: int) -> None: + """Track when stats over markers have been computed by a user. + + Args: + trackers_count: The actual number of trackers processed + """ + _track( + TELEMETRY_MARKERS_STATS_COMPUTED_EVENT, {"trackers_count": trackers_count}, + ) + + +@ensure_telemetry_enabled +def track_markers_parsed_count( + marker_count: int, max_depth: int, branching_factor: int +) -> None: + """Track when markers have been successfully parsed from config. + + Args: + marker_count: The number of markers found in the config + max_depth: The maximum depth of any marker in the config + branching_factor: The maximum number of children of any marker in the config. + """ + _track( + TELEMETRY_MARKERS_PARSED_COUNT, + { + "marker_count": marker_count, + "max_depth": max_depth, + "branching_factor": branching_factor, + }, + ) diff --git a/tests/core/evaluation/test_marker.py b/tests/core/evaluation/test_marker.py index cc1d9858d201..ced5fa514f4b 100644 --- a/tests/core/evaluation/test_marker.py +++ b/tests/core/evaluation/test_marker.py @@ -618,6 +618,32 @@ def test_marker_from_path_only_reads_yamls(tmp_path: Path,): ) +@pytest.mark.parametrize( + "configs", + [ + {"my_marker1": {IntentDetectedMarker.positive_tag(): "this-intent"}}, + { + "my_marker1": {IntentDetectedMarker.positive_tag(): "this-intent"}, + "my_marker2": {IntentDetectedMarker.positive_tag(): "this-action"}, + }, + ], +) +def test_marker_from_path_adds_special_or_marker(tmp_path: Path, configs: Any): + + yaml_file = tmp_path / "config.yml" + rasa.shared.utils.io.write_yaml( + data=configs, target=yaml_file, + ) + loaded = Marker.from_path(tmp_path) + assert isinstance(loaded, OrMarker) + assert loaded.name == Marker.ANY_MARKER + assert len(loaded.sub_markers) == len(configs) + assert all( + isinstance(sub_marker, IntentDetectedMarker) + for sub_marker in loaded.sub_markers + ) + + @pytest.mark.parametrize( "path_config_tuples", [ @@ -656,3 +682,23 @@ def test_marker_from_path_raises( rasa.shared.utils.io.write_yaml(data=config, target=full_path) with pytest.raises(InvalidMarkerConfig): Marker.from_path(tmp_path) + + +@pytest.mark.parametrize( + "marker,expected_depth", + [ + ( + AndMarker( + markers=[ + SlotSetMarker("s1"), + OrMarker([IntentDetectedMarker("4"), IntentDetectedMarker("6"),]), + ], + ), + 3, + ), + (SlotSetMarker("s1"), 1), + (AndMarker(markers=[SlotSetMarker("s1"), IntentDetectedMarker("6"),],), 2), + ], +) +def test_marker_depth(marker: Marker, expected_depth: int): + assert marker.max_depth() == expected_depth diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 813c095812f6..35b66de28fa9 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -81,10 +81,18 @@ async def test_events_schema( telemetry.track_nlu_model_test(TrainingData()) + telemetry.track_markers_extraction_initiated("all", False, False, None) + + telemetry.track_markers_extracted(1) + + telemetry.track_markers_stats_computed(1) + + telemetry.track_markers_parsed_count(1, 1, 1) + pending = asyncio.all_tasks() - initial await asyncio.gather(*pending) - assert mock.call_count == 15 + assert mock.call_count == 19 for args, _ in mock.call_args_list: event = args[0]