diff --git a/ee/app/services/remote_development/common_service.rb b/ee/app/services/remote_development/common_service.rb index d3450ee57193ae6c0616d8535c0047a7947ca936..7e5bfc8848c34cdf762b7709e2c4d9cfe81f854e 100644 --- a/ee/app/services/remote_development/common_service.rb +++ b/ee/app/services/remote_development/common_service.rb @@ -6,6 +6,7 @@ module RemoteDevelopment class CommonService extend Gitlab::Fp::RopHelpers extend ServiceResponseFactory + include Gitlab::InternalEvents # NOTE: This service intentionally does not follow the conventions for object-based service classes as documented in # https://docs.gitlab.com/ee/development/reusing_abstractions.html#service-classes. @@ -28,9 +29,10 @@ def self.execute(domain_main_class:, domain_main_class_args:) settings = RemoteDevelopment::Settings.get(RemoteDevelopment::Settings::DefaultSettings.default_settings.keys) logger = RemoteDevelopment::Logger.build + internal_events_class = Gitlab::InternalEvents response_hash = domain_main_class.singleton_method(main_class_method).call( - **domain_main_class_args.merge(settings: settings, logger: logger) + **domain_main_class_args.merge(settings: settings, logger: logger, internal_events_class: internal_events_class) ) create_service_response(response_hash) diff --git a/ee/config/events/create_workspace_result.yml b/ee/config/events/create_workspace_result.yml new file mode 100644 index 0000000000000000000000000000000000000000..9959b5d127f089751abf83dbb03704018b85c53f --- /dev/null +++ b/ee/config/events/create_workspace_result.yml @@ -0,0 +1,21 @@ +--- +description: GitLab Workspace creation result and error message +internal_events: true +action: create_workspace_result +identifiers: +- project +- namespace +- user +additional_properties: + label: + description: workspace succeed or fail + property: + description: error message if failed +product_group: remote_development +product_categories: +- workspaces +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +tiers: +- premium +- ultimate diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..e947e6a6d479b6afbcc8f06d936362f78229d03e --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created.yml @@ -0,0 +1,24 @@ +--- +key_path: counts.count_total_failed_workspaces_created +description: Count of Total Failed Workspaces Created +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_creation_failed.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_creation_failed.yml new file mode 100644 index 0000000000000000000000000000000000000000..8c4f26a178cb51328a18095266664c666e8ab817 --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_creation_failed.yml @@ -0,0 +1,25 @@ +--- +key_path: counts.count_total_failed_workspaces_created_by_creation_failed +description: Count of Total Failed Workspaces Created By Creation Failed +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed + property: WorkspaceCreateFailed diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_flattern.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_flattern.yml new file mode 100644 index 0000000000000000000000000000000000000000..f67c6396d5ba5996244b2daeef81112f8a15b377 --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_flattern.yml @@ -0,0 +1,25 @@ +--- +key_path: counts.count_total_failed_workspaces_created_by_failed_devfile_flattern +description: Count of Total Failed Workspaces Created By Failed Devfile Flattern +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed + property: WorkspaceCreateDevfileFlattenFailed diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_validation.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_validation.yml new file mode 100644 index 0000000000000000000000000000000000000000..1b4026cd22d9fe7f9a1ac67879c95accc5105542 --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_validation.yml @@ -0,0 +1,25 @@ +--- +key_path: counts.count_total_failed_workspaces_created_by_failed_devfile_validation +description: Count of Total Failed Workspaces Created By Failed Devfile Validation +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed + property: WorkspaceCreateDevfileValidationFailed diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_yaml_load.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_yaml_load.yml new file mode 100644 index 0000000000000000000000000000000000000000..a23bf2ecd7ca4993ee51e511333e0d0d2ea841bd --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_yaml_load.yml @@ -0,0 +1,25 @@ +--- +key_path: counts.count_total_failed_workspaces_created_by_failed_devfile_yaml_load +description: Count of Total Failed Workspaces Created By Failed Devfile Yaml Load +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed + property: WorkspaceCreateDevfileLoadFailed diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_yaml_parse.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_yaml_parse.yml new file mode 100644 index 0000000000000000000000000000000000000000..469bae11c7b0c88bbe74a5284c1f523217521998 --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_devfile_yaml_parse.yml @@ -0,0 +1,25 @@ +--- +key_path: counts.count_total_failed_workspaces_created_by_failed_devfile_yaml_parse +description: Count of Total Failed Workspaces Created By Failed Devfile Yaml Parse +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed + property: WorkspaceCreateDevfileYamlParseFailed diff --git a/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_params_validation.yml b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_params_validation.yml new file mode 100644 index 0000000000000000000000000000000000000000..9f69577f5302cb470f6b802159f77d3cd453b237 --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_failed_workspaces_created_by_failed_params_validation.yml @@ -0,0 +1,25 @@ +--- +key_path: counts.count_total_failed_workspaces_created_by_failed_params_validation +description: Count of Total Failed Workspaces Created By Failed Params Validation +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.0' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: failed + property: WorkspaceCreateParamsValidationFailed diff --git a/ee/config/metrics/counts_all/count_total_succeed_workspaces_created.yml b/ee/config/metrics/counts_all/count_total_succeed_workspaces_created.yml new file mode 100644 index 0000000000000000000000000000000000000000..9bf8ba4997e22695e348aaf41f3ee6452833fe3b --- /dev/null +++ b/ee/config/metrics/counts_all/count_total_succeed_workspaces_created.yml @@ -0,0 +1,24 @@ +--- +key_path: counts.count_total_succeed_workspaces_created +description: Count of Total Succeed Workspaces Created +product_group: remote_development +product_categories: +- workspaces +performance_indicator_type: [] +value_type: number +status: active +milestone: '18.1' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188226 +time_frame: +- 28d +- 7d +- all +data_source: internal_events +data_category: optional +tiers: +- premium +- ultimate +events: +- name: create_workspace_result + filter: + label: succeed diff --git a/ee/lib/remote_development/workspace_operations/create/agent_validator.rb b/ee/lib/remote_development/workspace_operations/create/agent_validator.rb index c69e33b678638a1076e675446e4cee70bfbada30..5f3bb5f846a2ef142387a8c535c7232dbe4b7ca9 100644 --- a/ee/lib/remote_development/workspace_operations/create/agent_validator.rb +++ b/ee/lib/remote_development/workspace_operations/create/agent_validator.rb @@ -33,7 +33,8 @@ def self.validate(context) WorkspaceCreateParamsValidationFailed.new( details: "Cannot use agent '#{agent.name}' as an organization mapped agent, the provided agent " \ "is not mapped in organization '#{project.organization.name}'. It also cannot be used as a " \ - "namespace mapped agent, it is not mapped to an ancestor namespace of the workspaces' project." + "namespace mapped agent, it is not mapped to an ancestor namespace of the workspaces' project.", + context: context ) ) end @@ -51,7 +52,8 @@ def self.validate(context) "is not mapped in organization '#{project.organization.name}'. It also cannot be used as a " \ "namespace mapped agent, #{relevant_namespace_mappings.size} mapping(s) exist between the " \ "provided agent and the ancestor namespaces of the workspaces' project, but the agent does not " \ - "reside within the hierarchy of any of the mapped ancestor namespaces." + "reside within the hierarchy of any of the mapped ancestor namespaces.", + context: context ) ) end diff --git a/ee/lib/remote_development/workspace_operations/create/creator.rb b/ee/lib/remote_development/workspace_operations/create/creator.rb index a785bda6a1dcf36ad9ee7e456aa3a6bccfa76c06..8e30c2d43c10a5b8aa2f2f8c58ece99ff588bfdb 100644 --- a/ee/lib/remote_development/workspace_operations/create/creator.rb +++ b/ee/lib/remote_development/workspace_operations/create/creator.rb @@ -36,9 +36,11 @@ def self.create(context) end end - return Gitlab::Fp::Result.err(WorkspaceCreateFailed.new({ errors: model_errors })) if model_errors.present? + if model_errors.present? + return Gitlab::Fp::Result.err(WorkspaceCreateFailed.new({ errors: model_errors, context: context })) + end - Gitlab::Fp::Result.ok(WorkspaceCreateSuccessful.new(updated_value)) + Gitlab::Fp::Result.ok(updated_value) end end end diff --git a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb index f85ea15bddf40f8ce344ca908a14a7908608387f..779441920a0c9588271e8b816869bb81e52938d7 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_fetcher.rb @@ -30,7 +30,8 @@ def self.validate_agent_config_exists(context) unless agent.unversioned_latest_workspaces_agent_config return Gitlab::Fp::Result.err(WorkspaceCreateParamsValidationFailed.new( - details: "No WorkspacesAgentConfig found for agent '#{agent.name}'" + details: "No WorkspacesAgentConfig found for agent '#{agent.name}'", + context: context )) end @@ -73,13 +74,17 @@ def self.read_devfile_yaml_from_repo_if_devfile_path_is_provided(context) unless devfile_blob return Gitlab::Fp::Result.err(WorkspaceCreateDevfileLoadFailed.new( details: "Devfile path '#{devfile_path}' at ref '#{project_ref}' " \ - "does not exist in the project repository" + "does not exist in the project repository", + context: context )) end unless devfile_blob.data.present? return Gitlab::Fp::Result.err( - WorkspaceCreateDevfileLoadFailed.new(details: "Devfile could not be loaded from project") + WorkspaceCreateDevfileLoadFailed.new( + details: "Devfile could not be loaded from project", + context: context + ) ) end @@ -102,7 +107,8 @@ def self.parse_devfile_yaml(context) devfile = devfile_to_json_and_back_to_yaml.to_h.deep_symbolize_keys rescue RuntimeError, JSON::GeneratorError => e return Gitlab::Fp::Result.err(WorkspaceCreateDevfileYamlParseFailed.new( - details: "Devfile YAML could not be parsed: #{e.message}" + details: "Devfile YAML could not be parsed: #{e.message}", + context: context )) end diff --git a/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb b/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb index fd967b7f909d82e6278000b53b44156428f97504..9f64a8fb8ae4d0c21035f55540b46e2a9a311cf7 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_flattener.rb @@ -15,7 +15,7 @@ def self.flatten(context) devfile_yaml = YAML.dump(devfile.deep_stringify_keys) flattened_devfile_yaml = Devfile::Parser.flatten(devfile_yaml) rescue Devfile::CliError => e - return Gitlab::Fp::Result.err(WorkspaceCreateDevfileFlattenFailed.new(details: e.message)) + return Gitlab::Fp::Result.err(WorkspaceCreateDevfileFlattenFailed.new(details: e.message, context: context)) end # NOTE: We do not attempt to rescue any exceptions from YAML.safe_load here because we assume that the diff --git a/ee/lib/remote_development/workspace_operations/create/devfile_validator.rb b/ee/lib/remote_development/workspace_operations/create/devfile_validator.rb index 31436c9d639888b1498612afe4f60ef293f1fbc5..ebdb8bd11664725a3def570fd58038e8d28f5613 100644 --- a/ee/lib/remote_development/workspace_operations/create/devfile_validator.rb +++ b/ee/lib/remote_development/workspace_operations/create/devfile_validator.rb @@ -60,7 +60,8 @@ def self.validate_schema_version(context) devfile_schema_version = Gem::Version.new(devfile_schema_version_string) rescue ArgumentError return err( - format(_("Invalid 'schemaVersion' '%{schema_version}'"), schema_version: devfile_schema_version_string) + format(_("Invalid 'schemaVersion' '%{schema_version}'"), schema_version: devfile_schema_version_string), + context ) end @@ -71,7 +72,8 @@ def self.validate_schema_version(context) _("'schemaVersion' '%{given_version}' is not supported, it must be '%{required_version}'"), given_version: devfile_schema_version_string, required_version: REQUIRED_DEVFILE_SCHEMA_VERSION - ) + ), + context ) end @@ -83,7 +85,7 @@ def self.validate_schema_version(context) def self.validate_parent(context) devfile = devfile_to_validate(context) - return err(format(_("Inheriting from 'parent' is not yet supported"))) if devfile[:parent] + return err(format(_("Inheriting from 'parent' is not yet supported")), context) if devfile[:parent] Gitlab::Fp::Result.ok(context) end @@ -93,8 +95,8 @@ def self.validate_parent(context) def self.validate_projects(context) devfile = devfile_to_validate(context) - return err(_("'starterProjects' is not yet supported")) if devfile[:starterProjects] - return err(_("'projects' is not yet supported")) if devfile[:projects] + return err(_("'starterProjects' is not yet supported"), context) if devfile[:starterProjects] + return err(_("'projects' is not yet supported"), context) if devfile[:projects] Gitlab::Fp::Result.ok(context) end @@ -104,7 +106,7 @@ def self.validate_projects(context) def self.validate_root_attributes(context) devfile = devfile_to_validate(context) - return err(_("Attribute 'pod-overrides' is not yet supported")) if devfile.dig(:attributes, + return err(_("Attribute 'pod-overrides' is not yet supported"), context) if devfile.dig(:attributes, :"pod-overrides") Gitlab::Fp::Result.ok(context) @@ -117,7 +119,7 @@ def self.validate_components(context) components = devfile[:components] - return err(_("No components present in devfile")) if components.blank? + return err(_("No components present in devfile"), context) if components.blank? injected_main_components = components.select do |component| component.dig(:attributes, MAIN_COMPONENT_INDICATOR_ATTRIBUTE.to_sym) @@ -125,7 +127,8 @@ def self.validate_components(context) if injected_main_components.empty? return err( - format(_("No component has '%{attribute}' attribute"), attribute: MAIN_COMPONENT_INDICATOR_ATTRIBUTE) + format(_("No component has '%{attribute}' attribute"), attribute: MAIN_COMPONENT_INDICATOR_ATTRIBUTE), + context ) end @@ -135,12 +138,13 @@ def self.validate_components(context) _("Multiple components '%{name}' have '%{attribute}' attribute"), name: injected_main_components.pluck(:name), # rubocop:disable CodeReuse/ActiveRecord -- this pluck isn't from ActiveRecord, it's from ActiveSupport attribute: MAIN_COMPONENT_INDICATOR_ATTRIBUTE - ) + ), + context ) end components_all_have_names = components.all? { |component| component[:name].present? } - return err(_("Components must have a 'name'")) unless components_all_have_names + return err(_("Components must have a 'name'"), context) unless components_all_have_names components.each do |component| component_name = component.fetch(:name) @@ -150,19 +154,22 @@ def self.validate_components(context) _("Component name '%{component}' must not start with '%{prefix}'"), component: component_name, prefix: RESTRICTED_PREFIX - )) + ), context) end UNSUPPORTED_COMPONENT_TYPES.each do |unsupported_component_type| if component[unsupported_component_type] - return err(format(_("Component type '%{type}' is not yet supported"), type: unsupported_component_type)) + return err( + format(_("Component type '%{type}' is not yet supported"), type: unsupported_component_type), + context + ) end end - return err(_("Attribute 'container-overrides' is not yet supported")) if component.dig( + return err(_("Attribute 'container-overrides' is not yet supported"), context) if component.dig( :attributes, :"container-overrides") - return err(_("Attribute 'pod-overrides' is not yet supported")) if component.dig(:attributes, + return err(_("Attribute 'pod-overrides' is not yet supported"), context) if component.dig(:attributes, :"pod-overrides") end @@ -185,7 +192,8 @@ def self.validate_containers(context) format( _("Property 'dedicatedPod' of component '%{name}' is not yet supported"), name: component.fetch(:name) - ) + ), + context ) end end @@ -217,7 +225,8 @@ def self.validate_endpoints(context) endpoint: endpoint_name, component: component.fetch(:name), prefix: RESTRICTED_PREFIX - ) + ), + context ) end end @@ -241,7 +250,8 @@ def self.validate_commands(context) _("Command id '%{command}' must not start with '%{prefix}'"), command: command_id, prefix: RESTRICTED_PREFIX - ) + ), + context ) end @@ -259,7 +269,8 @@ def self.validate_commands(context) component: component_name, command: command_id, prefix: RESTRICTED_PREFIX - ) + ), + context ) end end @@ -278,7 +289,7 @@ def self.validate_events(context) if SUPPORTED_EVENTS.exclude?(event_type) && event_type_events.present? err_msg = format(_("Event type '%{type}' is not yet supported"), type: event_type) # The entries for unsupported events may be defined, but they must be blank. - return err(err_msg) + return err(err_msg, context) end # Ensure no event starts with restricted_prefix @@ -291,7 +302,8 @@ def self.validate_events(context) event: event, event_type: event_type, prefix: RESTRICTED_PREFIX - ) + ), + context ) end end @@ -316,7 +328,8 @@ def self.validate_variables(context) _("Variable name '%{variable}' must not start with '%{prefix}'"), variable: variable, prefix: prefix - ) + ), + context ) end end @@ -325,9 +338,12 @@ def self.validate_variables(context) end # @param [String] details + # @param [Hash] context # @return [Gitlab::Fp::Result] - def self.err(details) - Gitlab::Fp::Result.err(WorkspaceCreateDevfileValidationFailed.new({ details: details })) + def self.err(details, context) + Gitlab::Fp::Result.err(WorkspaceCreateDevfileValidationFailed.new( + { details: details, context: context } + )) end private_class_method :devfile_to_validate, :validate_schema_version, :validate_parent, :validate_projects, :validate_components, :validate_containers, diff --git a/ee/lib/remote_development/workspace_operations/create/main.rb b/ee/lib/remote_development/workspace_operations/create/main.rb index 461e4acf1a6896058ed933913433160f15d7f2ed..2710ee4d29759f719735b4fa455ea6f2da804468 100644 --- a/ee/lib/remote_development/workspace_operations/create/main.rb +++ b/ee/lib/remote_development/workspace_operations/create/main.rb @@ -28,6 +28,9 @@ def self.main(context) .map(ProjectClonerComponentInserter.method(:insert)) .map(VolumeComponentInserter.method(:insert)) .and_then(Creator.method(:create)) + .inspect_ok(WorkspaceObserver.method(:observe)) + .inspect_err(WorkspaceErrorsObserver.method(:observe)) + .and_then(WorkspaceSuccessfulResponseBuilder.method(:build)) # rubocop:disable Lint/DuplicateBranch -- Rubocop doesn't know the branches are different due to destructuring case result diff --git a/ee/lib/remote_development/workspace_operations/create/personal_access_token_creator.rb b/ee/lib/remote_development/workspace_operations/create/personal_access_token_creator.rb index 592eb7c9ae85e393860bf0c3c7d57b3c89f2095e..dee4c18d98bd7a61c8418a21cfd3fe201b2cc23d 100644 --- a/ee/lib/remote_development/workspace_operations/create/personal_access_token_creator.rb +++ b/ee/lib/remote_development/workspace_operations/create/personal_access_token_creator.rb @@ -30,7 +30,9 @@ def self.create(context) if personal_access_token.errors.present? return Gitlab::Fp::Result.err( - PersonalAccessTokenModelCreateFailed.new({ errors: personal_access_token.errors }) + PersonalAccessTokenModelCreateFailed.new( + { errors: personal_access_token.errors, context: context } + ) ) end diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb b/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb index 17beea2a8de5e042240a5f071d857289e6281b06..3120b89058a172caf66174645d627c218d8a2957 100644 --- a/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb +++ b/ee/lib/remote_development/workspace_operations/create/workspace_creator.rb @@ -61,7 +61,7 @@ def self.create(context) if workspace.errors.present? return Gitlab::Fp::Result.err( - WorkspaceModelCreateFailed.new({ errors: workspace.errors }) + WorkspaceModelCreateFailed.new({ errors: workspace.errors, context: context }) ) end diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_errors_observer.rb b/ee/lib/remote_development/workspace_operations/create/workspace_errors_observer.rb new file mode 100644 index 0000000000000000000000000000000000000000..bcc58bce92837e4b8adff2fb3a1e4627a8de0dd5 --- /dev/null +++ b/ee/lib/remote_development/workspace_operations/create/workspace_errors_observer.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module WorkspaceOperations + module Create + class WorkspaceErrorsObserver + include Messages + extend Gitlab::Fp::MessageSupport + + # @param [RemoteDevelopment::Message] message + # @return [void] + def self.observe(message) + message.content => { + context: { + user: user, + internal_events_class: internal_events_class, + params: { + project: project, + }, + } + } + + internal_events_class.track_event('create_workspace_result', category: name, project: project, user: user, + additional_properties: { label: "failed", property: message.class.name.demodulize }) + + nil + end + end + end + end +end diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_observer.rb b/ee/lib/remote_development/workspace_operations/create/workspace_observer.rb new file mode 100644 index 0000000000000000000000000000000000000000..01ac2a770ef80a8a4dd577efed084ae5216000d3 --- /dev/null +++ b/ee/lib/remote_development/workspace_operations/create/workspace_observer.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module WorkspaceOperations + module Create + class WorkspaceObserver + include Messages + extend Gitlab::Fp::MessageSupport + + # @param [Hash] context + # @return [void] + def self.observe(context) + context => { + user: user, + internal_events_class: internal_events_class, + params: { + project: project, + } + } + + internal_events_class.track_event('create_workspace_result', category: name, project: project, user: user, + additional_properties: { label: "succeed" }) + + nil + end + end + end + end +end diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_successful_response_builder.rb b/ee/lib/remote_development/workspace_operations/create/workspace_successful_response_builder.rb new file mode 100644 index 0000000000000000000000000000000000000000..221e57f2a59b28ec0691ab212cee3b89a110ea61 --- /dev/null +++ b/ee/lib/remote_development/workspace_operations/create/workspace_successful_response_builder.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module WorkspaceOperations + module Create + class WorkspaceSuccessfulResponseBuilder + include Messages + + def self.build(context) + Gitlab::Fp::Result.ok(WorkspaceCreateSuccessful.new(context)) + end + end + end + end +end diff --git a/ee/lib/remote_development/workspace_operations/create/workspace_variables_creator.rb b/ee/lib/remote_development/workspace_operations/create/workspace_variables_creator.rb index d3580e28d514fb7fb53435bd880a22b23d03ca9f..bf2069fe6c706e127331699c09e0d81253c6e965 100644 --- a/ee/lib/remote_development/workspace_operations/create/workspace_variables_creator.rb +++ b/ee/lib/remote_development/workspace_operations/create/workspace_variables_creator.rb @@ -39,7 +39,7 @@ def self.create(context) if workspace_variable.errors.present? return Gitlab::Fp::Result.err( - WorkspaceVariablesModelCreateFailed.new({ errors: workspace_variable.errors }) + WorkspaceVariablesModelCreateFailed.new({ errors: workspace_variable.errors, context: context }) ) end end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/creator_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/creator_spec.rb index 5faefba1d3a64b4937366b3c23072a66163506e4..10013abd19819e0efdb40d6f40bf57f482c1f66b 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/creator_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/creator_spec.rb @@ -18,7 +18,7 @@ describe "happy path" do let(:expected_value) do - Gitlab::Fp::Result.ok(Messages::WorkspaceCreateSuccessful.new(context_passed_along_steps)) + Gitlab::Fp::Result.ok(context_passed_along_steps) end it "returns expected response" do @@ -35,7 +35,7 @@ describe "error cases" do let(:error_details) { "some error details" } - let(:err_message_content) { { errors: error_details } } + let(:err_message_content) { { errors: error_details, context: context_passed_along_steps } } shared_examples "rop invocation with error response" do it "returns expected response" do diff --git a/ee/spec/lib/remote_development/workspace_operations/create/devfile_flattener_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/devfile_flattener_spec.rb index a71ee92641fae54dd5beb7e7dc67f8ad7eaa5665..3b33ea1dc70a81718406ba2ce38e4baaf2355eb4 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/devfile_flattener_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/devfile_flattener_spec.rb @@ -52,7 +52,7 @@ "- (root): Additional property random is not allowed\n" message = result.unwrap_err expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceCreateDevfileFlattenFailed) - expect(message.content).to eq(details: expected_error_message) + expect(message.content).to eq({ details: expected_error_message, context: context }) end end end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb index a14c9a1fb43d5062a08026dda8a2cb52ef86b8be..11a32c45f44dae368f69b02a19a0ddc64d2e2b6c 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb @@ -2,6 +2,35 @@ require 'spec_helper' +RSpec.shared_examples 'tracks successful workspace creation event' do + it "tracks creation event" do + expect { response } + .to trigger_internal_events('create_workspace_result') + .with( + category: 'RemoteDevelopment::WorkspaceOperations::Create::WorkspaceObserver', + user: user, + project: project, + additional_properties: { label: 'succeed' } + ) + end +end + +RSpec.shared_examples 'tracks failed workspace creation event' do |error_message| + it "tracks failed creation event with proper error details" do + expect { response } + .to trigger_internal_events('create_workspace_result') + .with( + category: 'RemoteDevelopment::WorkspaceOperations::Create::WorkspaceErrorsObserver', + user: user, + project: project, + additional_properties: { + label: 'failed', + property: error_message + } + ) + end +end + # NOTE: This spec cannot use let_it_be because, because that doesn't work when using the `custom_repo` trait of # the project factory and subsequently modifying it, because it's a real on-disk repo at `tmp/tests/gitlab-test/`, # and any changes made to it are not reverted by let it be (even with reload). This means we also cannot use @@ -83,6 +112,7 @@ { user: user, params: params, + internal_events_class: Gitlab::InternalEvents, settings: settings, vscode_extension_marketplace: vscode_extension_marketplace, vscode_extension_marketplace_metadata: { enabled: vscode_extension_marketplace_metadata_enabled } @@ -153,6 +183,8 @@ end end + it_behaves_like 'tracks successful workspace creation event' + context 'with versioned workspaces_agent_configs behavior' do before do agent.unversioned_latest_workspaces_agent_config.touch @@ -164,6 +196,8 @@ workspace = response.fetch(:payload).fetch(:workspace) expect(workspace.workspaces_agent_config_version).to eq(expected_workspaces_agent_config_version) end + + it_behaves_like 'tracks successful workspace creation event' end context "with shared namespace" do @@ -179,6 +213,8 @@ workspace = response.fetch(:payload).fetch(:workspace) expect(workspace.namespace).to eq("default") end + + it_behaves_like 'tracks successful workspace creation event' end end @@ -190,6 +226,8 @@ expect(workspace.devfile).to eq(default_devfile_yaml) end + + it_behaves_like 'tracks successful workspace creation event' end context 'when devfile is not valid', :aggregate_failures do @@ -204,6 +242,8 @@ reason: :bad_request }) end + + it_behaves_like 'tracks failed workspace creation event', 'WorkspaceCreateDevfileValidationFailed' end end @@ -225,6 +265,8 @@ reason: :bad_request }) end + + it_behaves_like 'tracks failed workspace creation event', 'WorkspaceCreateDevfileLoadFailed' end context 'when agent has no associated config' do @@ -251,6 +293,8 @@ reason: :bad_request }) end + + it_behaves_like 'tracks failed workspace creation event', 'WorkspaceCreateParamsValidationFailed' end end @@ -269,5 +313,7 @@ .dig(:container, :image) expect(image_from_processed_devfile).to eq(tools_injector_image_from_settings) end + + it_behaves_like 'tracks successful workspace creation event' end end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb index 265def41ebf98c31b5c2284e57b84a639e4a98b8..8155c33ba55474901ee46c2bb4c116055f872a8b 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_spec.rb @@ -16,12 +16,16 @@ [RemoteDevelopment::WorkspaceOperations::Create::MainComponentUpdater, :map], [RemoteDevelopment::WorkspaceOperations::Create::ProjectClonerComponentInserter, :map], [RemoteDevelopment::WorkspaceOperations::Create::VolumeComponentInserter, :map], - [RemoteDevelopment::WorkspaceOperations::Create::Creator, :and_then] + [RemoteDevelopment::WorkspaceOperations::Create::Creator, :and_then], + [observer_class, observer_method], + [RemoteDevelopment::WorkspaceOperations::Create::WorkspaceSuccessfulResponseBuilder, :and_then] ] end describe "happy path" do let(:ok_message_content) { { ok_details: "Everything is OK!" } } + let(:observer_class) { RemoteDevelopment::WorkspaceOperations::Create::WorkspaceObserver } + let(:observer_method) { :inspect_ok } let(:expected_response) do { @@ -40,7 +44,7 @@ .with_context_passed_along_steps(context_passed_along_steps) .with_ok_result_for_step( { - step_class: RemoteDevelopment::WorkspaceOperations::Create::Creator, + step_class: RemoteDevelopment::WorkspaceOperations::Create::WorkspaceSuccessfulResponseBuilder, returned_message: RemoteDevelopment::Messages::WorkspaceCreateSuccessful.new(ok_message_content) } ) @@ -51,6 +55,8 @@ describe "error cases" do let(:error_details) { "some error details" } let(:err_message_content) { { details: error_details } } + let(:observer_class) { RemoteDevelopment::WorkspaceOperations::Create::WorkspaceErrorsObserver } + let(:observer_method) { :inspect_err } shared_examples "rop invocation with error response" do it "returns expected response" do diff --git a/ee/spec/lib/remote_development/workspace_operations/create/workspace_errors_observer_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/workspace_errors_observer_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..77bad4424c7c03d1cc97cc898587e157f6409bfe --- /dev/null +++ b/ee/spec/lib/remote_development/workspace_operations/create/workspace_errors_observer_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::WorkspaceOperations::Create::WorkspaceErrorsObserver, feature_category: :workspaces do + let_it_be(:user) { create(:user) } + let_it_be(:namespace) { create(:group, developers: user) } + let_it_be(:project) { create(:project, namespace: namespace) } + let(:context) do + { + user: user, + internal_events_class: Gitlab::InternalEvents, + params: { + project: project + } + } + end + + let(:message) do + RemoteDevelopment::Messages::WorkspaceCreateDevfileLoadFailed.new( + details: "Devfile could not be loaded from project", + context: context + ) + end + + describe '.observe' do + let(:err_message) { message.class.name.demodulize } + + it 'tracks a failed event with the error class and increases failure metric' do + expect { described_class.observe(message) } + .to trigger_internal_events('create_workspace_result') + .with(user: user, project: project, additional_properties: { label: 'failed', + property: err_message }) + .and increment_usage_metrics('counts.count_total_failed_workspaces_created') + end + end +end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fa664fa878824427c5022d1ef0d24c831dbf5f4e --- /dev/null +++ b/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoteDevelopment::WorkspaceOperations::Create::WorkspaceObserver, feature_category: :workspaces do + let_it_be(:user) { create(:user) } + let_it_be(:namespace) { create(:group, developers: user) } + let_it_be(:project) { create(:project, namespace: namespace) } + let(:context) do + { + user: user, + internal_events_class: Gitlab::InternalEvents, + params: { + project: project + } + } + end + + describe '.observe' do + it 'tracks a succeeded event and increases succeed metric' do + expect { described_class.observe(context) } + .to trigger_internal_events('create_workspace_result') + .with(user: user, project: project, additional_properties: { label: 'succeed' }) + .and increment_usage_metrics('counts.count_total_succeed_workspaces_created') + end + end +end diff --git a/ee/spec/lib/remote_development/workspace_operations/reconcile/main_spec.rb b/ee/spec/lib/remote_development/workspace_operations/reconcile/main_spec.rb index 532a67f982920fdf0d65ddd4996b247f19a6c356..3c9a5f640aeaf21deffe7fb1aced526ba403a2e5 100644 --- a/ee/spec/lib/remote_development/workspace_operations/reconcile/main_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/reconcile/main_spec.rb @@ -56,7 +56,19 @@ describe "error cases" do let(:error_details) { "some error details" } - let(:err_message_content) { { details: error_details } } + let(:err_message_content) { { details: error_details, context: context_passed_along_steps } } + let(:rop_steps) do + [ + [RemoteDevelopment::WorkspaceOperations::Reconcile::Input::ParamsValidator, :and_then], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Input::ParamsExtractor, :map], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Input::ParamsToInfosConverter, :map], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Persistence::WorkspacesFromAgentInfosUpdater, :map], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Persistence::WorkspacesLifecycleManager, :map], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Persistence::WorkspacesToBeReturnedFinder, :map], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Output::ResponsePayloadBuilder, :map], + [RemoteDevelopment::WorkspaceOperations::Reconcile::Persistence::WorkspacesToBeReturnedUpdater, :map] + ] + end shared_examples "rop invocation with error response" do it "returns expected response" do diff --git a/spec/support/matchers/invoke_rop_steps.rb b/spec/support/matchers/invoke_rop_steps.rb index d8b9830dd472f73681ca65acb841064d9ee5baba..1270abe59f1b7203bfba398aca0ed43122917360 100644 --- a/spec/support/matchers/invoke_rop_steps.rb +++ b/spec/support/matchers/invoke_rop_steps.rb @@ -111,6 +111,7 @@ def build_expected_rop_steps( context_passed_along_steps: ) expected_rop_steps = [] + skip_unless_inspect = false rop_steps.each do |rop_step| step_class = rop_step[0] @@ -121,13 +122,11 @@ def build_expected_rop_steps( step_action: step_action } + next if skip_unless_inspect && [:inspect_ok, :inspect_err].freeze.exclude?(step_action) + if err_results_for_steps.key?(step_class) expected_rop_step[:returned_object] = err_results_for_steps[step_class] - - # Currently, only a single error step is supported, so we assign expected_rop_step as the last entry - # in expected_rop_steps, break out of the loop early, and do not add any more steps - expected_rop_steps << expected_rop_step - break + skip_unless_inspect = true elsif ok_results_for_steps.key?(step_class) expected_rop_step[:returned_object] = ok_results_for_steps[step_class] elsif step_action == :and_then @@ -169,7 +168,9 @@ def set_up_step_class_expectation( context_passed_along_steps:, returned_object: ) - expect(step_class).to receive(step_class_method).with(context_passed_along_steps).ordered do + expectation = expect(step_class).to receive(step_class_method) + expectation = expectation.with(context_passed_along_steps) if step_class_method != :observe + expectation.ordered do returned_object end end