From f64a639bcfa1fc2bc89ca7db268f594306edfd7c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 16 Mar 2021 18:18:33 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-10-stable-ee --- doc/development/README.md | 3 +- doc/development/agent/user_stories.md | 2 +- doc/development/api_graphql_styleguide.md | 54 +- doc/development/application_limits.md | 12 +- doc/development/appsec/index.md | 32 + doc/development/architecture.md | 18 +- doc/development/auto_devops.md | 6 +- doc/development/build_test_package.md | 4 +- doc/development/changelog.md | 5 +- doc/development/chatops_on_gitlabcom.md | 2 +- doc/development/cicd/templates.md | 10 +- doc/development/code_intelligence/index.md | 2 +- doc/development/code_review.md | 16 +- doc/development/contributing/issue_workflow.md | 6 +- .../contributing/merge_request_workflow.md | 5 +- doc/development/contributing/style_guides.md | 16 +- .../database/client_side_connection_pool.md | 2 +- doc/development/database/index.md | 1 + .../database/setting_multiple_values.md | 59 +- doc/development/database_query_comments.md | 17 +- doc/development/directory_structure.md | 58 + doc/development/distributed_tracing.md | 2 +- doc/development/documentation/feature_flags.md | 6 +- .../documentation/site_architecture/index.md | 4 +- .../site_architecture/release_process.md | 228 +- doc/development/documentation/styleguide.md | 8 - doc/development/documentation/styleguide/index.md | 37 +- doc/development/documentation/testing.md | 6 +- doc/development/elasticsearch.md | 105 +- doc/development/emails.md | 5 +- .../experiment_guide/experimentation.md | 399 + .../experiment_guide/gitlab_experiment.md | 547 + doc/development/experiment_guide/index.md | 397 +- doc/development/export_csv.md | 2 +- doc/development/fe_guide/accessibility.md | 193 +- doc/development/fe_guide/architecture.md | 7 + doc/development/fe_guide/dark_mode.md | 4 +- doc/development/fe_guide/dependencies.md | 2 +- doc/development/fe_guide/editor_lite.md | 222 +- doc/development/fe_guide/graphql.md | 61 +- doc/development/fe_guide/index.md | 2 +- doc/development/fe_guide/keyboard_shortcuts.md | 63 +- doc/development/fe_guide/security.md | 8 +- doc/development/fe_guide/style/html.md | 23 +- doc/development/fe_guide/style/index.md | 2 +- doc/development/fe_guide/style/javascript.md | 23 +- doc/development/fe_guide/style/scss.md | 12 +- doc/development/fe_guide/style/vue.md | 36 +- doc/development/fe_guide/tooling.md | 91 +- doc/development/fe_guide/troubleshooting.md | 27 + doc/development/fe_guide/vue.md | 8 +- doc/development/fe_guide/vuex.md | 11 +- doc/development/fe_guide/widgets.md | 143 + doc/development/feature_flags/controls.md | 18 +- doc/development/feature_flags/development.md | 594 +- doc/development/feature_flags/index.md | 657 +- doc/development/feature_flags/process.md | 177 +- doc/development/features_inside_dot_gitlab.md | 4 +- doc/development/gemfile.md | 55 + doc/development/geo/framework.md | 13 +- doc/development/git_object_deduplication.md | 5 +- doc/development/go_guide/index.md | 16 +- doc/development/graphql_guide/graphql_pro.md | 4 +- doc/development/graphql_guide/index.md | 2 +- doc/development/i18n/translation.md | 12 +- doc/development/image_scaling.md | 2 +- doc/development/integrations/jenkins.md | 58 +- doc/development/integrations/jira_connect.md | 2 +- doc/development/integrations/secure.md | 34 +- .../integrations/secure_partner_integration.md | 4 +- doc/development/internal_api.md | 16 +- doc/development/licensed_feature_availability.md | 10 +- doc/development/maintenance_mode.md | 2 +- .../merge_request_performance_guidelines.md | 2 +- doc/development/migration_style_guide.md | 33 +- doc/development/multi_version_compatibility.md | 2 +- doc/development/namespaces_storage_statistics.md | 6 +- .../new_fe_guide/development/components.md | 2 +- doc/development/new_fe_guide/tips.md | 8 +- doc/development/packages.md | 60 +- doc/development/performance.md | 111 +- doc/development/pipelines.md | 43 +- doc/development/product_analytics/usage_ping.md | 4 +- doc/development/prometheus.md | 8 - doc/development/query_performance.md | 4 +- doc/development/rake_tasks.md | 4 + doc/development/redis.md | 9 +- doc/development/secure_coding_guidelines.md | 2 +- doc/development/service_measurement.md | 2 +- doc/development/sidekiq_style_guide.md | 2 +- doc/development/snowplow.md | 7 +- doc/development/sql.md | 14 +- doc/development/testing_guide/best_practices.md | 16 +- .../testing_guide/end_to_end/best_practices.md | 6 +- .../end_to_end/dynamic_element_validation.md | 6 +- .../end_to_end/environment_selection.md | 2 +- .../testing_guide/end_to_end/feature_flags.md | 4 +- .../testing_guide/end_to_end/page_objects.md | 2 +- .../running_tests_that_require_special_setup.md | 2 +- .../testing_guide/end_to_end/style_guide.md | 2 +- doc/development/testing_guide/frontend_testing.md | 161 +- doc/development/testing_guide/review_apps.md | 4 +- doc/development/transient/prevention-patterns.md | 2 +- doc/development/usage_ping.md | 1189 +- doc/development/usage_ping/dictionary.md | 18256 ++++++++++++++++++- doc/development/usage_ping/index.md | 1337 ++ doc/development/usage_ping/metrics_dictionary.md | 33 +- .../usage_ping/product_intelligence_review.md | 80 + doc/development/value_stream_analytics.md | 6 +- doc/development/wikis.md | 2 +- 110 files changed, 22205 insertions(+), 3927 deletions(-) create mode 100644 doc/development/appsec/index.md delete mode 100644 doc/development/documentation/styleguide.md create mode 100644 doc/development/experiment_guide/experimentation.md create mode 100644 doc/development/experiment_guide/gitlab_experiment.md create mode 100644 doc/development/fe_guide/widgets.md delete mode 100644 doc/development/prometheus.md create mode 100644 doc/development/usage_ping/index.md create mode 100644 doc/development/usage_ping/product_intelligence_review.md (limited to 'doc/development') diff --git a/doc/development/README.md b/doc/development/README.md index 3d5335feb11..43ceb737dde 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -261,7 +261,7 @@ See [database guidelines](database/index.md). ## Product Intelligence guides - [Product Intelligence guide](https://about.gitlab.com/handbook/product/product-intelligence-guide/) -- [Usage Ping guide](usage_ping.md) +- [Usage Ping guide](usage_ping/index.md) - [Snowplow guide](snowplow.md) ## Experiment guide @@ -287,6 +287,7 @@ See [database guidelines](database/index.md). ## Domain-specific guides - [CI/CD development documentation](cicd/index.md) +- [AppSec development documentation](appsec/index.md) ## Other Development guides diff --git a/doc/development/agent/user_stories.md b/doc/development/agent/user_stories.md index 609be47a3cb..ab135cad9d3 100644 --- a/doc/development/agent/user_stories.md +++ b/doc/development/agent/user_stories.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Kubernetes Agent user stories **(PREMIUM SELF)** -The [personas in action](https://about.gitlab.com/handbook/marketing/product-marketing/roles-personas/#user-personas) +The [personas in action](https://about.gitlab.com/handbook/marketing/strategic-marketing/roles-personas/#user-personas) for the Kubernetes Agent are: - [Sasha, the Software Developer](https://about.gitlab.com/handbook/marketing/strategic-marketing/roles-personas/#sasha-software-developer). diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 85098689392..8bac02c99af 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -13,7 +13,7 @@ This document outlines the style guide for the GitLab [GraphQL API](../api/graph We use the [GraphQL Ruby gem](https://graphql-ruby.org/) written by [Robert Mosolgo](https://github.com/rmosolgo/). -In addition, we have a subscription to [GraphQL Pro](https://www.graphql.pro). For details see [GraphQL Pro subscription](graphql_guide/graphql_pro.md). +In addition, we have a subscription to [GraphQL Pro](https://graphql.pro/). For details see [GraphQL Pro subscription](graphql_guide/graphql_pro.md). All GraphQL queries are directed to a single endpoint ([`app/controllers/graphql_controller.rb#execute`](https://gitlab.com/gitlab-org/gitlab/blob/master/app%2Fcontrollers%2Fgraphql_controller.rb)), @@ -76,6 +76,28 @@ Rake task. Requests time out at 30 seconds. +## Breaking changes + +The GitLab GraphQL API is [versionless](https://graphql.org/learn/best-practices/#versioning) which means +developers must familiarize themselves with our [deprecation cycle of breaking changes](#breaking-changes). + +Breaking changes are: + +- Removing or renaming a field, argument, enum value or mutation. +- Changing the type of a field, argument or enum value. +- Raising the [complexity](#max-complexity) of a field or complexity multipliers in a resolver. +- Changing a field from being _not_ nullable (`null: false`) to nullable (`null: true`), as +discussed in [Nullable fields](#nullable-fields). +- Changing an argument from being optional (`required: false`) to being required (`required: true`). +- Changing the [max page size](#page-size-limit) of a connection. +- Lowering the global limits for query complexity and depth. +- Anything else that can result in queries hitting a limit that previously was allowed. + +Fields that use the [`feature_flag` property](#feature_flag-property) and the flag is disabled by default are exempt +from the deprecation process, and can be removed at any time without notice. + +See the [deprecating fields and enum values](#deprecating-fields-arguments-and-enum-values) section for how to deprecate items. + ## Global IDs The GitLab GraphQL API uses Global IDs (i.e: `"gid://gitlab/MyObject/123"`) @@ -448,7 +470,7 @@ fails. Consider this when toggling the visibility of the feature on or off on production. The `feature_flag` property does not allow the use of -[feature gates based on actors](../development/feature_flags/development.md). +[feature gates based on actors](../development/feature_flags/index.md). This means that the feature flag cannot be toggled only for particular projects, groups, or users, but instead can only be toggled globally for everyone. @@ -490,15 +512,18 @@ def foo end ``` -## Deprecating fields and enum values +## Deprecating fields, arguments, and enum values The GitLab GraphQL API is versionless, which means we maintain backwards -compatibility with older versions of the API with every change. Rather -than removing a field or [enum value](#enums), we need to _deprecate_ it instead. +compatibility with older versions of the API with every change. + +Rather than removing fields, arguments, or [enum values](#enums), they +must be _deprecated_ instead. + The deprecated parts of the schema can then be removed in a future release -in accordance with the [GitLab deprecation process](../api/graphql/index.md#deprecation-process). +in accordance with the [GitLab deprecation process](../api/graphql/index.md#deprecation-and-removal-process). -Fields and enum values are deprecated using the `deprecated` property. +Fields, arguments, and enum values are deprecated using the `deprecated` property. The value of the property is a `Hash` of: - `reason` - Reason for the deprecation. @@ -518,14 +543,15 @@ is appended to the `description`. ### Deprecation reason style guide -Where the reason for deprecation is due to the field or enum value being -replaced, the `reason` must be: +Where the reason for deprecation is due to the field, argument, or enum value being +replaced, the `reason` must indicate the replacement. For example, the +following is a `reason` for a replaced field: ```plaintext Use `otherFieldName` ``` -Example: +Examples: ```ruby field :designs, ::Types::DesignManagement::DesignCollectionType, null: true, @@ -544,8 +570,8 @@ module Types end ``` -If the field is not being replaced by another field, a descriptive -deprecation `reason` should be given. +If the field, argument, or enum value being deprecated is not being replaced, +a descriptive deprecation `reason` should be given. See also [Aliasing and deprecating mutations](#aliasing-and-deprecating-mutations). @@ -594,7 +620,7 @@ end ``` Enum values can be deprecated using the -[`deprecated` keyword](#deprecating-fields-and-enum-values). +[`deprecated` keyword](#deprecating-fields-arguments-and-enum-values). ### Defining GraphQL enums dynamically from Rails enums @@ -1567,7 +1593,7 @@ mount_aliased_mutation 'BarMutation', Mutations::FooMutation ``` This allows us to rename a mutation and continue to support the old name, -when coupled with the [`deprecated`](#deprecating-fields-and-enum-values) +when coupled with the [`deprecated`](#deprecating-fields-arguments-and-enum-values) argument. Example: diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index 3608636dd55..c42e9224105 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -139,14 +139,14 @@ end Self-managed: -- `default` - Everyone +- `default`: Everyone. GitLab.com: -- `default` - Any system-wide feature -- `free` - Namespaces and projects with a Free subscription -- `bronze`- Namespaces and projects with a Bronze subscription -- `silver` - Namespaces and projects with a Silver subscription -- `gold` - Namespaces and projects with a Gold subscription +- `default`: Any system-wide feature. +- `free`: Namespaces and projects with a Free subscription. +- `bronze`: Namespaces and projects with a Bronze subscription. This tier is no longer available for purchase. +- `silver`: Namespaces and projects with a Premium subscription. +- `gold`: Namespaces and projects with an Ultimate subscription. The `test` environment doesn't have any plans. diff --git a/doc/development/appsec/index.md b/doc/development/appsec/index.md new file mode 100644 index 00000000000..e8ce885e75d --- /dev/null +++ b/doc/development/appsec/index.md @@ -0,0 +1,32 @@ +--- +stage: Secure, Protect +group: all +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +type: index, dev, reference +--- + +# Application Security development documentation + +Development guides that are specific to the stages that work on Application Security features are listed here. + +Please go to [Application Security](../../user/application_security/index.md) if you are looking for documentation on how to use those features. + +## Namespaces + +Application Security code in the Rails monolith is organized into the following namespaces, which generally follows +the feature categories in the [Secure](https://about.gitlab.com/stages-devops-lifecycle/secure/) and [Protect](https://about.gitlab.com/stages-devops-lifecycle/protect/) stages. + +- `AppSec`: shared code. + - `AppSec::ContainerScanning`: Container Scanning code. + - `AppSec::Dast`: DAST code. + - `AppSec::DependencyScanning`: Dependency Scanning code. + - `AppSec::Fuzzing::Api`: API Fuzzing code. + - `AppSec::Fuzzing::Coverage`: Coverage Fuzzing code. + - `AppSec::Fuzzing`: Shared fuzzing code. + - `AppSec::LicenseCompliance`: License Compliance code. + - `AppSec::Sast`: SAST code. + - `AppSec::SecretDetection`: Secret Detection code. + - `AppSec::VulnMgmt`: Vulnerability Management code. + +Most AppSec code does not conform to these namespace guidelines. When developing, make an effort +to move existing code into the appropriate namespace whenever possible. diff --git a/doc/development/architecture.md b/doc/development/architecture.md index 69055131ae8..5564d0722b0 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -224,17 +224,7 @@ Component statuses are linked to configuration documentation for each component. ### Component list -Table description links: - -- [Omnibus GitLab](https://docs.gitlab.com/omnibus/) -- [GitLab Environment Toolkit (GET)](https://gitlab.com/gitlab-org/quality/gitlab-environment-toolkit) -- [GitLab chart](https://docs.gitlab.com/charts/) -- [Minikube Minimal](https://docs.gitlab.com/charts/development/minikube/#deploying-gitlab-with-minimal-settings) -- [GitLab.com](https://gitlab.com) -- [Source](../install/installation.md) -- [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit) - -| Component | Description | Omnibus GitLab | GitLab Environment Toolkit (GET) | GitLab chart | Minikube Minimal | GitLab.com | Source | GDK | CE/EE | +| Component | Description | [Omnibus GitLab](https://docs.gitlab.com/omnibus/) | [GitLab Environment Toolkit (GET)](https://gitlab.com/gitlab-org/quality/gitlab-environment-toolkit) | [GitLab chart](https://docs.gitlab.com/charts/) | [Minikube Minimal](https://docs.gitlab.com/charts/development/minikube/#deploying-gitlab-with-minimal-settings) | [GitLab.com](https://gitlab.com) | [Source](../install/installation.md) | [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit) | [CE/EE](https://about.gitlab.com/install/ce-or-ee/) | |-------------------------------------------------------|----------------------------------------------------------------------|:--------------:|:--------------:|:------------:|:----------------:|:----------:|:------:|:---:|:-------:| | [Certificate Management](#certificate-management) | TLS Settings, Let's Encrypt | ✅ | ✅ | ✅ | ⚙ | ✅ | ⚙ | ⚙ | CE & EE | | [Consul](#consul) | Database node discovery, failover | ⚙ | ✅ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | @@ -332,7 +322,7 @@ Consul is a tool for service discovery and configuration. Consul is distributed, - Configuration: - [Omnibus](https://docs.gitlab.com/omnibus/settings/database.html#disabling-automatic-database-migration) - [Charts](https://docs.gitlab.com/charts/charts/gitlab/migrations/) - - [Source](../update/upgrading_from_source.md#14-install-libraries-migrations-etc) + - [Source](../update/upgrading_from_source.md#10-install-libraries-migrations-etc) - Layer: Core Service (Data) #### Elasticsearch @@ -651,7 +641,7 @@ Redis is packaged to provide a place to store: - [Project page](https://github.com/docker/distribution/blob/master/README.md) - Configuration: - - [Omnibus](../update/upgrading_from_source.md#14-install-libraries-migrations-etc) + - [Omnibus](../update/upgrading_from_source.md#10-install-libraries-migrations-etc) - [Charts](https://docs.gitlab.com/charts/charts/registry/) - [Source](../administration/packages/container_registry.md#enable-the-container-registry) - [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/registry.md) @@ -909,7 +899,7 @@ in Rails, scheduled to run whenever an SSH key is modified by a user. instead of keys. In this case, `AuthorizedKeysCommand` is replaced with an `AuthorizedPrincipalsCommand`. This extracts a username from the certificate without using the Rails internal API, which is used instead of `key_id` in the -`/api/internal/allowed` call later. +[`/api/internal/allowed`](internal_api.md) call later. GitLab Shell also has a few operations that do not involve Gitaly, such as resetting two-factor authentication codes. These are handled in the same way, diff --git a/doc/development/auto_devops.md b/doc/development/auto_devops.md index eaf1d712f17..c127858d3e7 100644 --- a/doc/development/auto_devops.md +++ b/doc/development/auto_devops.md @@ -47,12 +47,12 @@ found in ## Development environment -Configuring [GDK for Auto -DevOps](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/auto_devops.md). +See the [Simple way to develop/test Kubernetes workflows with a local cluster](https://gitlab.com/gitlab-org/gitlab-development-kit/-/issues/1064) +issue for discussion around setting up Auto DevOps development environments. ## Monitoring on GitLab.com The metric -[`auto_devops_completed_pipelines_total`](https://thanos-query.ops.gitlab.net/graph?g0.range_input=72h&g0.max_source_resolution=0s&g0.expr=sum(increase(auto_devops_pipelines_completed_total%7Benvironment%3D%22gprd%22%7D%5B60m%5D))%20by%20(status)&g0.tab=0) +[`auto_devops_completed_pipelines_total`](https://thanos.gitlab.net/graph?g0.range_input=72h&g0.max_source_resolution=0s&g0.expr=sum(increase(auto_devops_pipelines_completed_total%7Benvironment%3D%22gprd%22%7D%5B60m%5D))%20by%20(status)&g0.tab=0) (only available to GitLab team members) counts completed Auto DevOps pipelines, labeled by status. diff --git a/doc/development/build_test_package.md b/doc/development/build_test_package.md index 421ee5d0c84..1506017c6d5 100644 --- a/doc/development/build_test_package.md +++ b/doc/development/build_test_package.md @@ -40,7 +40,7 @@ used to trigger the build. In scenarios where a configuration change is to be introduced and Omnibus GitLab repository already has the necessary changes in a specific branch, you can build -a package against that branch through an environment variable named -`OMNIBUS_BRANCH`. To do this, specify that environment variable with the name of +a package against that branch through a CI/CD variable named +`OMNIBUS_BRANCH`. To do this, specify that variable with the name of the branch as value in `.gitlab-ci.yml` and push a commit. This will create a manual job that can be used to trigger the build. diff --git a/doc/development/changelog.md b/doc/development/changelog.md index f2c8aa4db62..98a3e75bb3c 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -45,7 +45,7 @@ the `author` field. GitLab team members **should not**. **must** have a changelog entry, without `merge_request` value and with `type` set to `security`. - Any user-facing change **must** have a changelog entry. This includes both visual changes (regardless of how minor), and changes to the rendered DOM which impact how a screen reader may announce the content. -- Any client-facing change to our REST and GraphQL APIs **must** have a changelog entry. This includes modifying complexity of GraphQL fields. +- Any client-facing change to our REST and GraphQL APIs **must** have a changelog entry. See the [complete list what comprises a GraphQL breaking change](api_graphql_styleguide.md#breaking-changes). - Performance improvements **should** have a changelog entry. - Changes that need to be documented in the Product Intelligence [Event Dictionary](https://about.gitlab.com/handbook/product/product-intelligence-guide/#event-dictionary) also require a changelog entry. @@ -57,8 +57,7 @@ the `author` field. GitLab team members **should not**. - Any change behind an enabled feature flag **should** have a changelog entry. - Any change that adds new usage data metrics and changes that needs to be documented in Product Intelligence [Event Dictionary](https://about.gitlab.com/handbook/product/product-intelligence-guide/#event-dictionary) **should** have a changelog entry. - A change that adds snowplow events **should** have a changelog entry - -- A change that [removes a feature flag](feature_flags/development.md) **should** have a changelog entry - - only if the feature flag did not default to true already. +- A change that [removes a feature flag](feature_flags/index.md) **must** have a changelog entry. - A fix for a regression introduced and then fixed in the same release (i.e., fixing a bug introduced during a monthly release candidate) **should not** have a changelog entry. diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index 0341abf5eeb..4ae49103d1b 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -61,4 +61,4 @@ To request access to ChatOps on GitLab.com: - [ChatOps Usage](../ci/chatops/index.md) - [Understanding EXPLAIN plans](understanding_explain_plans.md) -- [Feature Groups](feature_flags/development.md#feature-groups) +- [Feature Groups](feature_flags/index.md#feature-groups) diff --git a/doc/development/cicd/templates.md b/doc/development/cicd/templates.md index 94b03634e25..ed0d217c247 100644 --- a/doc/development/cicd/templates.md +++ b/doc/development/cicd/templates.md @@ -47,7 +47,7 @@ performance: ``` and users include this template with passing an argument to the `performance` job. -This can be done by specifying the environment variable `TARGET_URL` in _their_ `.gitlab-ci.yml`: +This can be done by specifying the CI/CD variable `TARGET_URL` in _their_ `.gitlab-ci.yml`: ```yaml include: @@ -72,6 +72,10 @@ Please read [versioning](#versioning) section for introducing breaking change sa When a root `.gitlab-ci.yml` [includes](../../ci/yaml/README.md#include) multiple templates, these global keywords could be overridden by the others and cause an unexpected behavior. +- Include [a changelog](../changelog.md) if your merge request introduces a user-facing change. +- Use [`$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH`](../../ci/variables/predefined_variables.md) + instead of a hardcoded `main` branch, and never use `master`. +- Use [`rules`](../../ci/yaml/README.md#rules) instead of [`only` or `except`](../../ci/yaml/README.md#onlyexcept-basic), if possible. ## Versioning @@ -88,7 +92,7 @@ for example `Jobs/Deploy.gitlab-ci.yml`. You can make a new stable template by copying [the latest template](#latest-version) available in a major milestone release of GitLab like `13.0`. All breaking changes must be announced in a blog post before the official release, for example -[GitLab.com is moving to 13.0, with narrow breaking changes](https://about.gitlab.com/releases/2020/05/06/gitlab-com-13-0-breaking-changes/) +[GitLab.com is moving to 13.0, with narrow breaking changes](https://about.gitlab.com/blog/2020/05/06/gitlab-com-13-0-breaking-changes/) You can change a stable template version in a minor GitLab release like `13.1` if: @@ -180,7 +184,7 @@ is updated in a major version GitLab release. ## Security A template could contain malicious code. For example, a template that contains the `export` shell command in a job -might accidentally expose project secret variables in a job log. +might accidentally expose secret project CI/CD variables in a job log. If you're unsure if it's secure or not, you need to ask security experts for cross-validation. ## Contribute CI/CD Template Merge Requests diff --git a/doc/development/code_intelligence/index.md b/doc/development/code_intelligence/index.md index ac962e3ae3e..790ba1539b7 100644 --- a/doc/development/code_intelligence/index.md +++ b/doc/development/code_intelligence/index.md @@ -1,6 +1,6 @@ --- stage: Create -group: Source Code +group: Code Review info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/development/code_review.md b/doc/development/code_review.md index dada6adcce7..0a811ceba65 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -70,16 +70,16 @@ It picks reviewers and maintainers from the list at the [engineering projects](https://about.gitlab.com/handbook/engineering/projects/) page, with these behaviors: -1. It doesn't pick people whose Slack or [GitLab status](../user/profile/index.md#current-status): +1. It doesn't pick people whose Slack or [GitLab status](../user/profile/index.md#set-your-current-status): - contains the string 'OOO', 'PTO', 'Parental Leave', or 'Friends and Family' - emoji is `:palm_tree:`, `:beach:`, `:beach_umbrella:`, `:beach_with_umbrella:`, `:ferris_wheel:`, `:thermometer:`, `:face_with_thermometer:`, `:red_circle:`, `:bulb:`, `:sun_with_face:`. 1. [Trainee maintainers](https://about.gitlab.com/handbook/engineering/workflow/code-review/#trainee-maintainer) are three times as likely to be picked as other reviewers. -1. Team members whose Slack or [GitLab status](../user/profile/index.md#current-status) emoji +1. Team members whose Slack or [GitLab status](../user/profile/index.md#set-your-current-status) emoji is 🔵 `:large_blue_circle:` are more likely to be picked. This applies to both reviewers and trainee maintainers. - Reviewers with `:large_blue_circle:` are two times as likely to be picked as other reviewers. - Trainee maintainers with `:large_blue_circle:` are four times as likely to be picked as other reviewers. -1. People whose [GitLab status](../user/profile/index.md#current-status) emoji +1. People whose [GitLab status](../user/profile/index.md#set-your-current-status) emoji is 🔶 `:large_orange_diamond:` are half as likely to be picked. This applies to both reviewers and trainee maintainers. 1. It always picks the same reviewers and maintainers for the same branch name (unless their OOO status changes, as in point 1). It @@ -283,6 +283,8 @@ first time. you forget to remove any debugging code? - Consider providing instructions on how to test the merge request. This can be helpful for reviewers not familiar with the product feature or area of the codebase. +- If you know your change depends on another being merged first, note it in the + description and set an [merge request dependency](../user/project/merge_requests/merge_request_dependencies.md). - Be grateful for the reviewer's suggestions. (`Good call. I'll make that change.`) - Don't take it personally. The review is of the code, not of you. - Explain why the code exists. ("It's like that because of these reasons. Would @@ -345,6 +347,8 @@ experience, refactors the existing code). Then: - For non-mandatory suggestions, decorate with (non-blocking) so the author knows they can optionally resolve within the merge request or follow-up at a later stage. - There's a [Chrome/Firefox add-on](https://gitlab.com/conventionalcomments/conventional-comments-button) which you can use to apply [Conventional Comment](https://conventionalcomments.org/) prefixes. +- Ensure there are no open dependencies. Check [related issues](../user/project/issues/related_issues.md) for blockers. Clarify with the author(s) +if necessary. If blocked by one or more open MRs, set an [MR dependency](../user/project/merge_requests/merge_request_dependencies.md). - After a round of line notes, it can be helpful to post a summary note such as "Looks good to me", or "Just a couple things to address." - Assign the merge request to the author if changes are required following your @@ -390,6 +394,8 @@ When ready to merge: - **Start a new merge request pipeline with the `Run Pipeline` button in the merge request's "Pipelines" tab, and enable "Merge When Pipeline Succeeds" (MWPS).** Note that: + - If **[master is broken](https://about.gitlab.com/handbook/engineering/workflow/#broken-master), + do not merge the merge request**. Follow these specific [handbook instructions](https://about.gitlab.com/handbook/engineering/workflow/#maintaining-throughput-during-broken-master). - If the **latest [Pipeline for Merged Results](../ci/merge_request_pipelines/pipelines_for_merged_results/#pipelines-for-merged-results)** finished less than 2 hours ago, you might merge without starting a new pipeline as the merge request is close enough to `master`. @@ -397,10 +403,6 @@ When ready to merge: Before triggering the pipeline, review all changes for **malicious code**. If you cannot trigger the pipeline, review the status of the fork relative to `master`. If it's more than 100 commits behind, ask the author to rebase it before merging. - - If [master is broken](https://about.gitlab.com/handbook/engineering/workflow/#broken-master), - in addition to the two above rules, check that any failure also happens - in `master` and post a link to the ~"master:broken" issue before clicking the - red "Merge" button. - When you set the MR to "Merge When Pipeline Succeeds", you should take over subsequent revisions for anything that would be spotted after that. diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index da38d1e73b4..7029c8a8384 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -149,8 +149,8 @@ and `~"group::knowledge"` is picked up by someone in the Access group of the Pla the issue should be relabeled as `~"group::access"` while keeping the original `~"devops::create"` unchanged. -We also use stage and group labels to help quantify our [throughput](https://about.gitlab.com/handbook/engineering/management/throughput/). -Please read [Stage and Group labels in Throughput](https://about.gitlab.com/handbook/engineering/management/throughput/#stage-and-group-labels-in-throughput) for more information on how the labels are used in this context. +We also use stage and group labels to help measure our [merge request rates](https://about.gitlab.com/handbook/engineering/merge-request-rate/). +Please read [Stage and Group labels](https://about.gitlab.com/handbook/engineering/metrics/#stage-and-group-labels) for more information on how the labels are used in this context. ### Category labels @@ -181,7 +181,7 @@ For instance, the "DevOps Report" category is represented by the `devops_reports.name` value is "DevOps Reports". If a category's label doesn't respect this naming convention, it should be specified -with [the `label` attribute](https://about.gitlab.com/handbook/marketing/website/#category-attributes) +with [the `label` attribute](https://about.gitlab.com/handbook/marketing/inbound-marketing/digital-experience/website/#category-attributes) in . ### Feature labels diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 166f7b350bf..9051b621bcd 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -68,10 +68,11 @@ request is as follows: 1. Use the syntax `Solves #XXX`, `Closes #XXX`, or `Refs #XXX` to mention the issue(s) your merge request addresses. Referenced issues do not [close automatically](../../user/project/issues/managing_issues.md#closing-issues-automatically). You must close them manually once the merge request is merged. + 1. The MR must include *Before* and *After* screenshots if UI changes are made. + 1. Include any steps or setup required to ensure reviewers can view the changes you've made (e.g. include any information about feature flags). 1. If you're allowed to, set a relevant milestone and [labels](issue_workflow.md). 1. UI changes should use available components from the GitLab Design System, - [Pajamas](https://design.gitlab.com/). The MR must include *Before* and - *After* screenshots. + [Pajamas](https://design.gitlab.com/). 1. If the MR changes CSS classes, please include the list of affected pages, which can be found by running `grep css-class ./app -R`. 1. If your MR touches code that executes shell commands, reads or opens files, or diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 2a2cfebe964..444a067a780 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -56,10 +56,10 @@ The current Lefthook configuration can be found in [`lefthook.yml`](https://gitl Before you push your changes, Lefthook automatically runs the following checks: - Danger: Runs a subset of checks that `danger-review` runs on your merge requests. -- ES lint: Run `yarn eslint` checks (with the [`.eslintrc.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.eslintrc.yml) configuration) on the modified `*.{js,vue}` files. Tags: `frontend`, `style`. +- ES lint: Run `yarn run lint:eslint` checks (with the [`.eslintrc.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.eslintrc.yml) configuration) on the modified `*.{js,vue}` files. Tags: `frontend`, `style`. - HAML lint: Run `bundle exec haml-lint` checks (with the [`.haml-lint.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.haml-lint.yml) configuration) on the modified `*.html.haml` files. Tags: `view`, `haml`, `style`. - Markdown lint: Run `yarn markdownlint` checks on the modified `*.md` files. Tags: `documentation`, `style`. -- SCSS lint: Run `bundle exec scss-lint` checks (with the [`.scss-lint.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.scss-lint.yml) configuration) on the modified `*.scss{,.css}` files. Tags: `stylesheet`, `css`, `style`. +- SCSS lint: Run `yarn lint:stylelint` checks (with the [`.stylelintrc`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.stylelintrc) configuration) on the modified `*.scss{,.css}` files. Tags: `stylesheet`, `css`, `style`. - RuboCop: Run `bundle exec rubocop` checks (with the [`.rubocop.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.rubocop.yml) configuration) on the modified `*.rb` files. Tags: `backend`, `style`. - Vale: Run `vale` checks (with the [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.vale.ini) configuration) on the modified `*.md` files. Tags: `documentation`, `style`. @@ -146,7 +146,7 @@ One way to generate the initial list is to run the `todo` auto generation, with `exclude limit` set to a high number. ```shell -bundle exec rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit=10000 +bundle exec rubocop --auto-gen-config --auto-gen-only-exclude --exclude-limit=100000 ``` You can then move the list from the freshly generated `.rubocop_todo.yml` for the Cop being actively @@ -154,6 +154,16 @@ resolved and place it in the `.rubocop_manual_todo.yml`. In this scenario, do no changes to the `.rubocop_todo.yml` as an `exclude limit` that is higher than 15 will make the `.rubocop_todo.yml` hard to parse. +### Reveal existing RuboCop exceptions + +To reveal existing RuboCop exceptions in the code that have been excluded via `.rubocop_todo.yml` and +`.rubocop_manual_todo.yml`, set the environment variable `REVEAL_RUBOCOP_TODO` to `1`. + +This allows you to reveal existing RuboCop exceptions during your daily work cycle and fix them along the way. + +NOTE: +Permanent `Exclude`s should be defined in `.rubocop.yml` instead of `.rubocop_manual_todo.yml`. + ## Database migrations See the dedicated [Database Migrations Style Guide](../migration_style_guide.md). diff --git a/doc/development/database/client_side_connection_pool.md b/doc/development/database/client_side_connection_pool.md index 1a30d2d73a3..8316a75ac8d 100644 --- a/doc/development/database/client_side_connection_pool.md +++ b/doc/development/database/client_side_connection_pool.md @@ -43,7 +43,7 @@ hardcoded value (10). At this point, we need to investigate what is using more connections than we anticipated. To do that, we can use the `gitlab_ruby_threads_running_threads` metric. For example, [this -graph](https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum%20by%20(thread_name)%20(%20gitlab_ruby_threads_running_threads%7Buses_db_connection%3D%22yes%22%7D%20)&g0.tab=0) +graph](https://thanos.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum%20by%20(thread_name)%20(%20gitlab_ruby_threads_running_threads%7Buses_db_connection%3D%22yes%22%7D%20)&g0.tab=0) shows all running threads that connect to the database by their name. Threads labeled `puma worker` or `sidekiq_worker_thread` are the threads that define `Gitlab::Runtime.max_threads` so those are diff --git a/doc/development/database/index.md b/doc/development/database/index.md index 367ef455898..870ae1542bd 100644 --- a/doc/development/database/index.md +++ b/doc/development/database/index.md @@ -69,3 +69,4 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## Miscellaneous - [Maintenance operations](maintenance_operations.md) +- [Update multiple database objects](setting_multiple_values.md) diff --git a/doc/development/database/setting_multiple_values.md b/doc/development/database/setting_multiple_values.md index 54870380047..0f23aae9f79 100644 --- a/doc/development/database/setting_multiple_values.md +++ b/doc/development/database/setting_multiple_values.md @@ -4,24 +4,22 @@ group: Database info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Setting Multiple Values +# Update multiple database objects > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/32921) in GitLab 13.5. -There's often a need to update multiple objects with new values for one -or more columns. One method of doing this is using `Relation#update_all`: +You can update multiple database objects with new values for one or more columns. +One method is to use `Relation#update_all`: ```ruby user.issues.open.update_all(due_date: 7.days.from_now) # (1) user.issues.update_all('relative_position = relative_position + 1') # (2) ``` -But what do you do if you cannot express the update as either a static value (1) -or as a calculation (2)? - -Thankfully we can use `UPDATE FROM` to express the need to update multiple rows -with distinct values in a single query. One can either use a temporary table, or -a Common Table Expression (CTE), and then use that as the source of the updates: +If you cannot express the update as either a static value (1) or as a calculation (2), +use `UPDATE FROM` to express the need to update multiple rows with distinct values +in a single query. Create a temporary table, or a Common Table Expression (CTE), +and use it as the source of the updates: ```sql with updates(obj_id, new_title, new_weight) as ( @@ -34,23 +32,22 @@ update issues where id = obj_id ``` -The bad news: there is no way to express this in ActiveRecord or even dropping -down to ARel. The `UpdateManager` does not support `update from`, so this -is not expressible. - -The good news: we supply an abstraction to help you generate these kinds of -updates, called `Gitlab::Database::BulkUpdate`. This constructs queries such as the -above, and uses binding parameters to avoid SQL injection. +You can't express this in ActiveRecord, or by dropping down to [Arel](https://api.rubyonrails.org/v6.1.0/classes/Arel.html), +because the `UpdateManager` does not support `update from`. However, we supply +an abstraction to help you generate these kinds of updates: `Gitlab::Database::BulkUpdate`. +This abstraction constructs queries like the previous example, and uses +binding parameters to avoid SQL injection. ## Usage -To use this, we need: +To use `Gitlab::Database::BulkUpdate`, we need: -- the list of columns to update -- a mapping from object/ID to the new values to set for that object -- a way to determine the table for each object +- The list of columns to update. +- A mapping from the object (or ID) to the new values to set for that object. +- A way to determine the table for each object. -For example, we can express the query above as: +For example, we can express the example query in a way that determines the +table by calling `object.class.table_name`: ```ruby issue_a = Issue.find(..) @@ -63,10 +60,7 @@ issue_b = Issue.find(..) }) ``` -Here the table can be determined automatically, from calling -`object.class.table_name`, so we don't need to provide anything. - -We can even pass heterogeneous sets of objects, if the updates all make sense +You can even pass heterogeneous sets of objects, if the updates all make sense for them: ```ruby @@ -82,8 +76,8 @@ merge_request = MergeRequest.find(..) }) ``` -If your objects do not return the correct model class (perhaps because they are -part of a union), then we need to specify this explicitly in a block: +If your objects do not return the correct model class, such as if they are part +of a union, then specify the model class explicitly in a block: ```ruby bazzes = params @@ -103,7 +97,10 @@ end ## Caveats -Note that this is a **very low level** tool, and operates on the raw column -values. Enumerations and state fields must be translated into their underlying -representations, for example, and nested associations are not supported. No -validations or hooks are called. +This tool is **very low level**, and operates directly on the raw column +values. You should consider these issues if you implement it: + +- Enumerations and state fields must be translated into their underlying + representations. +- Nested associations are not supported. +- No validations or hooks are called. diff --git a/doc/development/database_query_comments.md b/doc/development/database_query_comments.md index 8a5abad3815..39b14074073 100644 --- a/doc/development/database_query_comments.md +++ b/doc/development/database_query_comments.md @@ -39,24 +39,11 @@ Examples of queries with comments as observed in `development.log`: 1. Rails: ```sql - SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = $1 LIMIT $2 [["project_id", 5], ["LIMIT", 1]] /*application:web,controller:jobs,action:trace,correlation_id:rYF4mey9CH3,line:/app/policies/project_policy.rb:504:in `feature_available?'*/ + /*application:web,controller:jobs,action:trace,correlation_id:rYF4mey9CH3,line:/app/policies/project_policy.rb:504:in `feature_available?'*/ SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = $1 LIMIT $2 [["project_id", 5], ["LIMIT", 1]] ``` 1. Sidekiq: ```sql - SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 LIMIT $2 [["id", 64], ["LIMIT", 1]] /*application:sidekiq,jid:e7d6668a39a991e323009833,job_class:ExpireJobCacheWorker,correlation_id:rYF4mey9CH3,line:/app/workers/expire_job_cache_worker.rb:14:in `perform'*/ + /*application:sidekiq,jid:e7d6668a39a991e323009833,job_class:ExpireJobCacheWorker,correlation_id:rYF4mey9CH3,line:/app/workers/expire_job_cache_worker.rb:14:in `perform'*/ SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 LIMIT $2 [["id", 64], ["LIMIT", 1]] ``` - -## Enable/Disable the feature - -Enabling or disabling the feature requires a **restart/SIGHUP** of the Web and -Sidekiq workers, as the feature flag's state is memoized upon starting up. - -The `feature_flag` for this feature is **disabled** by default. You can enable -or disable it with: - -```ruby -Feature.enable(:marginalia) -Feature.disable(:marginalia) -``` diff --git a/doc/development/directory_structure.md b/doc/development/directory_structure.md index c2329feb941..c96e2cc3254 100644 --- a/doc/development/directory_structure.md +++ b/doc/development/directory_structure.md @@ -34,3 +34,61 @@ module MyDomain end end ``` + +### About namespace naming + +A good guideline for naming a top-level namespace (bounded context) is to use the related +feature category. For example, `Continuous Integration` feature category maps to `Ci::` namespace. + +Alternatively a new class could be added to `Projects::` or `Groups::` if it's either: + +- Strictly related to one of these domains. For example `Projects::Alias`. +- A new component that does not have yet a more specific domain. In this case, when + a more explicit domain does emerge we would need to move the class to a more specific + namespace. + +Do not use the [stage or group name](https://about.gitlab.com/handbook/product/categories/#devops-stages) +since a feature category could be reassigned to a different group in the future. + +```ruby +# bad +module Create + class Commit + end +end + +# good +module Repositories + class Commit + end +end +``` + +On the other hand, a feature category may sometimes be too granular. Features tend to be +treated differently according to Product and Marketing, while they may share a lot of +domain models and behavior under the hood. In this case, having too many bounded contexts +could make them shallow and more coupled with other contexts. + +Bounded contexts (or top-level namespaces) can be seen as macro-components in the overall app. +Good bounded contexts should be [deep](https://medium.com/@nakabonne/depth-of-module-f62dac3c2fdb) +so consider having nested namespaces to further break down complex parts of the domain. +E.g. `Ci::Config::`. + +For example, instead of having separate and granular bounded contexts like: `ContainerScanning::`, +`ContainerHostSecurity::`, `ContainerNetworkSecurity::`, we could have: + +```ruby +module ContainerSecurity + module HostSecurity + end + + module NetworkSecurity + end + + module Scanning + end +end +``` + +If classes that are defined into a namespace have a lot in common with classes in other namespaces, +chances are that these two namespaces are part of the same bounded context. diff --git a/doc/development/distributed_tracing.md b/doc/development/distributed_tracing.md index eb20e721e46..17967c5f63c 100644 --- a/doc/development/distributed_tracing.md +++ b/doc/development/distributed_tracing.md @@ -206,7 +206,7 @@ If `GITLAB_TRACING` is not configured correctly, this issue is logged: By default, GitLab ships with the Jaeger tracer, but other tracers can be included at compile time. Details of how this can be done are included in the [LabKit tracing -documentation](https://godoc.org/gitlab.com/gitlab-org/labkit/tracing). +documentation](https://pkg.go.dev/gitlab.com/gitlab-org/labkit/tracing). If no log messages about tracing are emitted, the `GITLAB_TRACING` environment variable is likely not set. diff --git a/doc/development/documentation/feature_flags.md b/doc/development/documentation/feature_flags.md index c9c291abd2c..8fe3f0cbf8e 100644 --- a/doc/development/documentation/feature_flags.md +++ b/doc/development/documentation/feature_flags.md @@ -14,9 +14,13 @@ feature flag depends on its state (enabled or disabled). When the state changes, the developer who made the change **must update the documentation** accordingly. +Every feature introduced to the codebase, even if it's behind a feature flag, +must be documented. For context, see the +[latest merge request that updated this guideline](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47917#note_459984428). + ## Criteria -According to the process of [deploying GitLab features behind feature flags](../feature_flags/process.md): +According to the process of [deploying GitLab features behind feature flags](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle): > - _By default, feature flags should be off._ > - _Feature flags should remain in the codebase for a short period as possible to reduce the need for feature flag accounting._ diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index 92fd17f9d3e..70fa80b3306 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -118,7 +118,7 @@ If you change the Dockerfile configuration and rebuild the images, you can break pipeline in the main `gitlab` repository as well as in `gitlab-docs`. Create an image with a different name first and test it to ensure you do not break the pipelines. -1. In [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs), go to **{rocket}** **CI / CD > Pipelines**. +1. In [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs), go to **{rocket}** **CI/CD > Pipelines**. 1. Click the **Run Pipeline** button. 1. See that a new pipeline is running. The jobs that build the images are in the first stage, `build-images`. You can click the pipeline number to see the larger pipeline @@ -137,7 +137,7 @@ and deploys it to . If you need to build and deploy the site immediately (must have maintainer level permissions): -1. In [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs), go to **{rocket}** **CI / CD > Schedules**. +1. In [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs), go to **{rocket}** **CI/CD > Schedules**. 1. For the `Build docs.gitlab.com every 4 hours` scheduled pipeline, click the **play** (**{play}**) button. Read more about the [deployment process](deployment_process.md). diff --git a/doc/development/documentation/site_architecture/release_process.md b/doc/development/documentation/site_architecture/release_process.md index ba5363dbb71..7bdf3fbdcf8 100644 --- a/doc/development/documentation/site_architecture/release_process.md +++ b/doc/development/documentation/site_architecture/release_process.md @@ -4,191 +4,165 @@ group: unassigned info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# GitLab Docs monthly release process +# Monthly release process -When a new GitLab version is released on the 22nd, we need to create the respective -single Docker image, and update some files so that the dropdown works correctly. +When a new GitLab version is released on the 22nd, we need to release the published documentation +for the new version. -## 1. Add the chart version +This should be done as soon as possible after the GitLab version is announced, so that: -Since the charts use a different version number than all the other GitLab -products, we need to add a -[version mapping](https://docs.gitlab.com/charts/installation/version_mappings.html): +- The published documentation includes the three most recent minor releases of the current major + version, and the most recent minor releases of the last two major versions. For example 13.9, + 13.8, 13.7, 12.10, and 11.11. +- Documentation updates after the 22nd are for the next release. The versions drop down + should have the current milestone with `-pre` appended to it, for example `13.10-pre`. -The charts stable branch is not created automatically like the other products. -There's an [issue to track this](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/1442). -It is usually created on the 21st or the 22nd. +Each documentation release: -To add a new charts version: +- Has a dedicated branch, named in the format `XX.yy`. +- Has a Docker image that contains a build of that branch. + +For example: + +- For [GitLab 13.9](https://docs.gitlab.com/13.9/index.html), the + [stable branch](https://gitlab.com/gitlab-org/gitlab-docs/-/tree/13.9) and Docker image: + [`registry.gitlab.com/gitlab-org/gitlab-docs:13.9`](https://gitlab.com/gitlab-org/gitlab-docs/container_registry/631635). +- For [GitLab 13.8](https://docs.gitlab.com/13.8/index.html), the + [stable branch](https://gitlab.com/gitlab-org/gitlab-docs/-/tree/13.8) and Docker image: + [`registry.gitlab.com/gitlab-org/gitlab-docs:13.8`](https://gitlab.com/gitlab-org/gitlab-docs/container_registry/631635). + +To set up a documentation release, follow these steps: + +1. [Add the charts version](#add-chart-version), so that the documentation is built using the + [version of the charts project that maps to](https://docs.gitlab.com/charts/installation/version_mappings.html) + the GitLab release. This step may have been completed already. +1. [Create a stable branch and Docker image](#create-stable-branch-and-docker-image-for-release) for + the new version. +1. [Create a release merge request](#create-release-merge-request) for the new version, which + updates the version dropdown menu for the current documentation and adds the release to the + Docker configuration. For example, the + [release merge request for 13.9](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/1555). +1. [Update the three online versions](#update-dropdown-for-online-versions), so that they display the new release on their + version dropdown menus. For example: + - The merge request to [update the 13.9 version dropdown menu for the 13.9 release](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/1556). + - The merge request to [update the 13.8 version dropdown menu for the 13.9 release](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/1557). + - The merge request to [update the 13.7 version dropdown menu for the 13.9 release](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/1558). +1. [Merge the release merge request and run the necessary Docker image builds](#merge-release-merge-request-and-run-docker-image-builds). + +## Add chart version + +To add a new charts version for the release: 1. Make sure you're in the root path of the `gitlab-docs` repository. 1. Open `content/_data/chart_versions.yaml` and add the new stable branch version using the - version mapping. Note that only the `major.minor` version is needed. + [version mapping](https://docs.gitlab.com/charts/installation/version_mappings.html). Only the + `major.minor` version is needed. 1. Create a new merge request and merge it. NOTE: -It can be handy to create the future mappings since they are pretty much known. -In that case, when a new GitLab version is released, you don't have to repeat -this first step. +If you have time, add anticipated future mappings to `content/_data/chart_versions.yaml`. This saves +a step for the next GitLab release. -## 2. Create an image for a single version +## Create stable branch and Docker image for release -The single docs version must be created before the release merge request, but -this needs to happen when the stable branches for all products have been created. +To create a stable branch and Docker image for the release: 1. Make sure you're in the root path of the `gitlab-docs` repository. -1. Run the Rake task to create the single version: +1. Run the Rake task to create the single version. For example, to create the 13.9 release branch + and perform others tasks: ```shell - ./bin/rake "release:single[12.0]" + ./bin/rake "release:single[13.9]" ``` - A new `Dockerfile.12.0` should have been created and `.gitlab-ci.yml` should - have the branches variables updated into a new branch. They are automatically - committed. + A branch for the release is created, a new `Dockerfile.13.9` is created, and `.gitlab-ci.yml` + has branches variables updated into a new branch. These files are automatically committed. + +1. Push the newly created branch, but **don't create a merge request**. After you push, the + `image:docs-single` job creates a new Docker image tagged with the name of the branch you created + earlier. You can see the Docker image in the `registry` environment at + . -1. Push the newly created branch, but **don't create a merge request**. - After you push, the `image:docs-single` job creates a new Docker image - tagged with the branch name you created in the first step. In the end, the - image is uploaded in the [Container Registry](https://gitlab.com/gitlab-org/gitlab-docs/container_registry) - and it is listed under the `registry` environment folder at - `https://gitlab.com/gitlab-org/gitlab-docs/-/environments/folders/registry` (must - have developer access). +For example, see [the 13.9 release pipeline](https://gitlab.com/gitlab-org/gitlab-docs/-/pipelines/260288747). -Optionally, you can test locally by building the image and running it: +Optionally, you can test locally by: -```shell -docker build -t docs:12.0 -f Dockerfile.12.0 . -docker run -it --rm -p 4000:4000 docs:12.0 -``` +1. Building the image and running it. For example, for GitLab 13.9 documentation: -Visit `http://localhost:4000/12.0/` to see if everything works correctly. + ```shell + docker build -t docs:13.9 -f Dockerfile.13.9 . + docker run -it --rm -p 4000:4000 docs:13.9 + ``` -## 3. Create the release merge request +1. Visiting to see if everything works correctly. + +## Create release merge request NOTE: -To be [automated](https://gitlab.com/gitlab-org/gitlab-docs/-/issues/750). +An [epic is open](https://gitlab.com/groups/gitlab-org/-/epics/4361) to automate this step. -Now it's time to create the monthly release merge request that adds the new -version and rotates the old one: +To create the release merge request for the release: 1. Make sure you're in the root path of the `gitlab-docs` repository. -1. Create a branch `release-X-Y`: +1. Create a branch `release-X-Y`. For example: ```shell git checkout master - git checkout -b release-12-0 + git checkout -b release-13-9 ``` -1. **Rotate the online and offline versions:** - - At any given time, there are 4 browsable online versions: one pulled from - the upstream master branches (docs for GitLab.com) and the three latest - stable versions. +1. Edit `content/_data/versions.yaml` and update the lists of versions to reflect the new release: - Edit `content/_data/versions.yaml` and rotate the versions to reflect the - new changes: + - Add the latest version to the `online:` section. + - Move the oldest version in `online:` to the `offline:` section. There should now be three + versions in `online:`. - - `online`: The 3 latest stable versions. - - `offline`: All the previous versions offered as an offline archive. +1. Update these Dockerfiles: -1. **Update the `:latest` and `:archives` Docker images:** + - `dockerfiles/Dockerfile.archives`: Add the latest version to the top of the list. + - `Dockerfile.master`: Remove the oldest version, and add the newest version to the + top of the list. - The following two Dockerfiles need to be updated: - - 1. `dockerfiles/Dockerfile.archives` - Add the latest version at the top of - the list. - 1. `Dockerfile.master` - Rotate the versions (oldest gets removed and latest - is added at the top of the list). - -1. In the end, there should be four files in total that have changed. - Commit and push to create the merge request using the "Release" template: +1. Commit and push to create the merge request. For example: ```shell git add content/ Dockerfile.master dockerfiles/Dockerfile.archives - git commit -m "Release 12.0" - git push origin release-12-0 + git commit -m "Release 13.9" + git push origin release-13-9 ``` -## 4. Update the dropdown for all online versions - -The versions dropdown is in a way "hardcoded". When the site is built, it looks -at the contents of `content/_data/versions.yaml` and based on that, the dropdown -is populated. Older branches have different content, which means the -dropdown list is one or more releases behind. Remember that the new changes of -the dropdown are included in the unmerged `release-X-Y` branch. +Do not merge the release merge request yet. -The content of `content/_data/versions.yaml` needs to change for all online -versions (stable branches `X.Y` of the `gitlab-docs` project): +## Update dropdown for online versions -1. Run the Rake task that creates all the respective merge requests needed to - update the dropdowns. Set these to automatically be merged when their - pipelines succeed: +To update`content/_data/versions.yaml` for all online versions (stable branches `X.Y` of the +`gitlab-docs` project): - NOTE: - The `release-X-Y` branch needs to be present locally, - and you need to have switched to it, otherwise the Rake task fails. +1. Run the Rake task that creates all of the necessary merge requests to update the dropdowns. For + example, for the 13.9 release: ```shell - git checkout release-X-Y + git checkout release-13-9 ./bin/rake release:dropdowns ``` -1. [Visit the merge requests page](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests?label_name%5B%5D=release) - to check that their pipelines pass, and once all are merged, proceed to the - following and final step. + These merge requests are set to automatically merge. -NOTE: -In case a pipeline fails, see [troubleshooting](#troubleshooting). +1. [Visit the merge requests page](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests?label_name%5B%5D=release) + to check that their pipelines pass. After all MRs are merged, proceed to the following and final + step. -## 5. Merge the release merge request +## Merge release merge request and run Docker image builds -The dropdown merge requests should have now been merged into their respective -version (stable `X.Y` branch), which triggers another pipeline. At this point, -you need to only babysit the pipelines and make sure they don't fail: +The merge requests for the dropdowns should now all be merged into their respective stable branches. +Each merge triggers a new pipeline for each stable branch. Wait for the stable branch pipelines to +complete, then: 1. Check the [pipelines page](https://gitlab.com/gitlab-org/gitlab-docs/pipelines) and make sure all stable branches have green pipelines. -1. After all the pipelines of the online versions succeed, merge the release merge request. +1. After all the pipelines succeed, merge the [release merge request](#create-release-merge-request). 1. Finally, run the [`Build docker images weekly` pipeline](https://gitlab.com/gitlab-org/gitlab-docs/pipeline_schedules) that builds the `:latest` and `:archives` Docker images. -Once the scheduled pipeline succeeds, the docs site is deployed with all -new versions online. - -## Troubleshooting - -Releasing a new version is a long process that involves many moving parts. - -### `test_internal_links_and_anchors` failing on dropdown merge requests - -WARNING: -We now pin versions in the `.gitlab-ci.yml` of the respective branch, -so the steps below are deprecated. - -When [updating the dropdown for the stable versions](#4-update-the-dropdown-for-all-online-versions), -there may be cases where some links might fail. The process of how the -dropdown MRs are created have a caveat, and that is that the tests run by -pulling the master branches of all products, instead of the respective stable -ones. - -In a real world scenario, the [Update 12.2 dropdown to match that of 12.4](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/604) -merge request failed because of the [`test_internal_links_and_anchors` test](https://gitlab.com/gitlab-org/gitlab-docs/-/jobs/328042431). - -This happened because there has been a rename of a product (`gitlab-monitor` to `gitlab-exporter`) -and the old name was still referenced in the 12.2 docs. If the respective stable -branches for 12.2 were used, this wouldn't have failed, but as we can see from -the [`compile_dev` job](https://gitlab.com/gitlab-org/gitlab-docs/-/jobs/328042427), -the `master` branches were pulled. - -To fix this, re-run the pipeline (`https://gitlab.com/gitlab-org/gitlab-docs/pipelines/new`) -for the `update-12-2-for-release-12-4` branch, by including the following environment variables: - -- `BRANCH_CE` set to `12-2-stable` -- `BRANCH_EE` set to `12-2-stable-ee` -- `BRANCH_OMNIBUS` set to `12-2-stable` -- `BRANCH_RUNNER` set to `12-2-stable` -- `BRANCH_CHARTS` set to `2-2-stable` - -This should make the MR pass. +As the last step in the scheduled pipeline, the documentation site deploys with all new versions. diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md deleted file mode 100644 index ade0b89a92c..00000000000 --- a/doc/development/documentation/styleguide.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -redirect_to: 'styleguide/index.md' ---- - -This document was moved to [another location](styleguide/index.md). - - - diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 7737aa58506..4831e5bbce5 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -108,7 +108,7 @@ of GitLab more efficient. New information that would be useful toward the future usage or troubleshooting of GitLab should not be written directly in a forum or other messaging system, -but added to a documentation MR and then referenced, as described above. +but added to a documentation MR and then referenced, as described above. The more we reflexively add information to the documentation, the more the documentation helps others efficiently accomplish tasks and solve problems. @@ -601,6 +601,7 @@ Follow these guidelines for punctuation: | Rule | Example | |------------------------------------------------------------------|--------------------------------------------------------| +| Avoid semicolons. Use two sentences instead. | _That's the way that the world goes 'round. You're up one day and the next you're down._ | Always end full sentences with a period. | _For a complete overview, read through this document._ | | Always add a space after a period when beginning a new sentence. | _For a complete overview, check this doc. For other references, check out this guide._ | | Do not use double spaces. (Tested in [`SentenceSpacing.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/SentenceSpacing.yml).) | --- | @@ -1606,34 +1607,32 @@ displayed for the page or feature. #### Version text in the **Version History** -If all content in a section is related, add version text after the header -for the section. The version information must be surrounded by blank lines, and -each entry should be on its own line. +If all content in a section is related, add version text after the header for +the section. The version information must: -Add the version history information as a blockquote: +- Be surrounded by blank lines. +- Start with `>`. If there are multiple bullets, each line must start with `> -`. +- The string must include these words in this order (capitalization doesn't matter): + - `introduced`, `deprecated`, `moved` + - `in` or `to` + - `GitLab` +- Whenever possible, include a link to the completed issue, merge request, or epic + that introduced the feature. An issue is preferred over a merge request, and + a merge request is preferred over an epic. ```markdown ## Feature name -> Introduced in GitLab 11.3. +> [Introduced]() in GitLab 11.3. This feature does something. -``` -Whenever possible, version text should have a link to the completed issue, merge -request, or epic that introduced the feature. An issue is preferred over a merge -request, and a merge request is preferred over an epic. For example: +## Feature name 2 -```markdown -> [Introduced]() in GitLab 11.3. -``` - -If you're adding information about new features or changes in a release, update -the blockquote to use a bulleted list: - -```markdown > - [Introduced]() in GitLab 11.3. -> - Enabled by default in GitLab 11.4. +> - [Enabled by default]() in GitLab 11.4. + +This feature does something else. ``` If a feature is moved to another tier: diff --git a/doc/development/documentation/testing.md b/doc/development/documentation/testing.md index f3d6e0a5c71..eed544911cb 100644 --- a/doc/development/documentation/testing.md +++ b/doc/development/documentation/testing.md @@ -272,7 +272,11 @@ To configure Vale in your editor, install one of the following as appropriate: In the extension's settings: - Select the **Use CLI** checkbox. - - In the **Config** setting, enter an absolute path to [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/blob/master/.vale.ini) in one of the cloned GitLab repositories on your computer. + - In the **Config** setting, enter an absolute + path to [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/blob/master/.vale.ini) + in one of the cloned GitLab repositories on your computer. + + - In the **Path** setting, enter the absolute path to the Vale binary. In most cases, `vale` should work. To find the location, run `which vale` in a terminal. diff --git a/doc/development/elasticsearch.md b/doc/development/elasticsearch.md index 3392bd1fbf6..3f14ca454fe 100644 --- a/doc/development/elasticsearch.md +++ b/doc/development/elasticsearch.md @@ -184,7 +184,7 @@ If the current version is `v12p1`, and we need to create a new version for `v12p 1. Change the namespace for files under `v12p1` folder from `Latest` to `V12p1` 1. Make changes to files under the `latest` folder as needed -## Creating a new Global Search migration +## Creating a new Advanced Search migration > This functionality was introduced by [#234046](https://gitlab.com/gitlab-org/gitlab/-/issues/234046). @@ -219,7 +219,9 @@ Any update to the Elastic index mappings should be replicated in [`Elastic::Late Migrations can be built with a retry limit and have the ability to be [failed and marked as halted](https://gitlab.com/gitlab-org/gitlab/-/blob/66e899b6637372a4faf61cfd2f254cbdd2fb9f6d/ee/lib/elastic/migration.rb#L40). Any data or index cleanup needed to support migration retries should be handled within the migration. -### Migration options supported by the [`Elastic::MigrationWorker`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/workers/elastic/migration_worker.rb) +### Migration options supported by the `Elastic::MigrationWorker` + +[`Elastic::MigrationWorker`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/workers/elastic/migration_worker.rb) supports the following migration options: - `batched!` - Allow the migration to run in batches. If set, the [`Elastic::MigrationWorker`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/workers/elastic/migration_worker.rb) will re-enqueue itself with a delay which is set using the `throttle_delay` option described below. The batching @@ -230,6 +232,9 @@ enough time to finish. Additionally, the time should be less than 30 minutes sin [`Elastic::MigrationWorker`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/workers/elastic/migration_worker.rb) cron worker runs. Default value is 5 minutes. +- `pause_indexing!` - Pause indexing while the migration runs. This setting will record the indexing setting before +the migration runs and set it back to that value when the migration is completed. + ```ruby # frozen_string_literal: true @@ -242,6 +247,102 @@ class BatchedMigrationName < Elastic::Migration end ``` +### Multi-version compatibility + +These Advanced Search migrations, like any other GitLab changes, need to support the case where +[multiple versions of the application are running at the same time](multi_version_compatibility.md). + +Depending on the order of deployment, it's possible that the migration +has started or finished and there's still a server running the application code from before the +migration. We need to take this into consideration until we can [ensure all Advanced Search migrations +start after the deployment has finished](https://gitlab.com/gitlab-org/gitlab/-/issues/321619). + +### Reverting a migration + +Because Elasticsearch does not support transactions, we always need to design our +migrations to accommodate a situation where the application +code is reverted after the migration has started or after it is finished. + +For this reason we generally defer destructive actions (for example, deletions after +some data is moved) to a later merge request after the migrations have +completed successfully. To be safe, for self-managed customers we should also +defer it to another release if there is risk of important data loss. + +### Best practices for Advanced Search migrations + +Follow these best practices for best results: + +- When working in batches, keep the batch size under 9,000 documents + and `throttle_delay` over 3 minutes. The bulk indexer is set to run + every 1 minute and process a batch of 10,000 documents. These limits + allow the bulk indexer time to process records before another migration + batch is attempted. +- To ensure that document counts are up to date, it is recommended to refresh + the index before checking if a migration is completed. +- Add logging statements to each migration when the migration starts, when a + completion check occurs, and when the migration is completed. These logs + are helpful when debugging issues with migrations. +- Pause indexing if you're using any Elasticsearch Reindex API operations. +- Consider adding a retry limit if there is potential for the migration to fail. + This ensures that migrations can be halted if an issue occurs. + +## Deleting Advanced Search migrations in a major version upgrade + +Since our Advanced Search migrations usually require us to support multiple +code paths for a long period of time, it's important to clean those up when we +safely can. + +We choose to use GitLab major version upgrades as a safe time to remove +backwards compatibility for indices that have not been fully migrated. We +[document this in our upgrade +documentation](../update/index.md#upgrading-to-a-new-major-version). We also +choose to remove the migration code and tests so that: + +- We don't need to maintain any code that is called from our Advanced Search + migrations. +- We don't waste CI time running tests for migrations that we don't support + anymore. + +To be extra safe, we will not delete migrations that were created in the last +minor version before the major upgrade. So, if the we are upgrading to `%14.0`, +we should not delete migrations that were only added in `%13.11`. This is an +extra safety net as we expect there are migrations that get merged that may +take multiple weeks to finish on GitLab.com. It would be bad if we upgraded +GitLab.com to `%14.0` before the migrations in `%13.11` were finished. Since +our deployments to GitLab.com are automated and we currently don't have +automated checks to prevent this, the extra precaution is warranted. +Additionally, even if we did have automated checks to prevent it, we wouldn't +actually want to hold up GitLab.com deployments on Advanced Search migrations, +as they may still have another week to go, and that's too long to block +deployments. + +### Process for removing migrations + +For every migration that was created 2 minor versions before the major version +being upgraded to, we do the following: + +1. Confirm the migration has actually completed successfully for GitLab.com. +1. Replace the content of `migrate` and `completed?` methods as follows: + + ```ruby + def migrate + log_raise "Migration has been deleted in the last major version upgrade." \ + "Migrations are supposed to be finished before upgrading major version https://docs.gitlab.com/ee/update/#upgrading-to-a-new-major-version ." \ + "In order to correct this issue you will need to reacreate your index from scratch https://docs.gitlab.com/ee/integration/elasticsearch.html#last-resort-to-recreate-an-index ." + end + + def completed? + false + end + ``` + +1. Delete any spec files to support this migration. +1. Remove any logic handling backwards compatibility for this migration. You + can find this by looking for + `Elastic::DataMigrationService.migration_has_finished?(:migration_name_in_lowercase)`. +1. Create a merge request with these changes. Noting that we should not + accidentally merge this before the major release is started. + ## Performance Monitoring ### Prometheus diff --git a/doc/development/emails.md b/doc/development/emails.md index 0b1f3c5b74c..1de1da33dc2 100644 --- a/doc/development/emails.md +++ b/doc/development/emails.md @@ -4,7 +4,7 @@ group: unassigned info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Dealing with email in development +# Working with email in development ## Ensuring compatibility with mailer Sidekiq jobs @@ -54,7 +54,8 @@ See the [Rails guides](https://guides.rubyonrails.org/action_mailer_basics.html# incoming_email: enabled: true - # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to. + # The email address including the %{key} placeholder that will be replaced to reference the item being replied to. This %{key} should be included in its entirety within the email address and not replaced by another value. + # For example: emailadress+%key@gmail.com. # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`). address: "gitlab-incoming+%{key}@gmail.com" diff --git a/doc/development/experiment_guide/experimentation.md b/doc/development/experiment_guide/experimentation.md new file mode 100644 index 00000000000..7135f8acd9b --- /dev/null +++ b/doc/development/experiment_guide/experimentation.md @@ -0,0 +1,399 @@ +--- +stage: Growth +group: Activation +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Create an A/B test with `Experimentation Module` + +## Implement the experiment + +1. Add the experiment to the `Gitlab::Experimentation::EXPERIMENTS` hash in + [`experimentation.rb`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib%2Fgitlab%2Fexperimentation.rb): + + ```ruby + EXPERIMENTS = { + other_experiment: { + #... + }, + # Add your experiment here: + signup_flow: { + tracking_category: 'Growth::Activation::Experiment::SignUpFlow' # Used for providing the category when setting up tracking data + } + }.freeze + ``` + +1. Use the experiment in the code. + + Experiments can be performed on a `subject`. The provided `subject` should + respond to `to_global_id` or `to_s`. + The resulting string is bucketed and assigned to either the control or the + experimental group, so you must always provide the same `subject` + for an experiment to have the same experience. + + 1. Use this standard for the experiment in a controller: + + - Experiment run for a user: + + ```ruby + class ProjectController < ApplicationController + def show + # experiment_enabled?(:experiment_key) is also available in views and helpers + if experiment_enabled?(:signup_flow, subject: current_user) + # render the experiment + else + # render the original version + end + end + end + ``` + + - Experiment run for a namespace: + + ```ruby + if experiment_enabled?(:signup_flow, subject: namespace) + # experiment code + else + # control code + end + ``` + + When no subject is given, it falls back to a cookie that gets set and is consistent until + the cookie gets deleted. + + ```ruby + class RegistrationController < ApplicationController + def show + # falls back to a cookie + if experiment_enabled?(:signup_flow) + # render the experiment + else + # render the original version + end + end + end + ``` + + 1. Make the experiment available to the frontend in a controller. This example + checks whether the experiment is enabled and pushes the result to the frontend: + + ```ruby + before_action do + push_frontend_experiment(:signup_flow, subject: current_user) + end + ``` + + You can check the state of the feature flag in JavaScript: + + ```javascript + import { isExperimentEnabled } from '~/experimentation'; + + if ( isExperimentEnabled('signupFlow') ) { + // ... + } + ``` + +You can also run an experiment outside of the controller scope, such as in a worker: + +```ruby +class SomeWorker + def perform + # Check if the experiment is active at all (the percentage_of_time_value > 0) + return unless Gitlab::Experimentation.active?(:experiment_key) + + # Since we cannot access cookies in a worker, we need to bucket models + # based on a unique, unchanging attribute instead. + # It is therefore necessery to always provide the same subject. + if Gitlab::Experimentation.in_experiment_group?(:experiment_key, subject: user) + # execute experimental code + else + # execute control code + end + end +end +``` + +## Implement tracking events + +To determine whether the experiment is a success or not, we must implement tracking events +to acquire data for analyzing. We can send events to Snowplow via either the backend or frontend. +Read the [product intelligence guide](https://about.gitlab.com/handbook/product/product-intelligence-guide/) for more details. + +### Track backend events + +The framework provides a helper method that is available in controllers: + +```ruby +before_action do + track_experiment_event(:signup_flow, 'action', 'value', subject: current_user) +end +``` + +To test it: + +```ruby +context 'when the experiment is active and the user is in the experimental group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_subject(signup_flow: true) + end + + it 'tracks an event', :snowplow do + subject + + expect_snowplow_event( + category: 'Growth::Activation::Experiment::SignUpFlow', + action: 'action', + value: 'value', + label: 'experimentation_subject_id', + property: 'experimental_group' + ) + end +end +``` + +### Track frontend events + +The framework provides a helper method that is available in controllers: + +```ruby +before_action do + push_frontend_experiment(:signup_flow, subject: current_user) + frontend_experimentation_tracking_data(:signup_flow, 'action', 'value', subject: current_user) +end +``` + +This pushes tracking data to `gon.experiments` and `gon.tracking_data`. + +```ruby +expect(Gon.experiments['signupFlow']).to eq(true) + +expect(Gon.tracking_data).to eq( + { + category: 'Growth::Activation::Experiment::SignUpFlow', + action: 'action', + value: 'value', + label: 'experimentation_subject_id', + property: 'experimental_group' + } +) +``` + +To track it: + +```javascript +import { isExperimentEnabled } from '~/lib/utils/experimentation'; +import Tracking from '~/tracking'; + +document.addEventListener('DOMContentLoaded', () => { + const signupFlowExperimentEnabled = isExperimentEnabled('signupFlow'); + + if (signupFlowExperimentEnabled && gon.tracking_data) { + const { category, action, ...data } = gon.tracking_data; + + Tracking.event(category, action, data); + } +} +``` + +To test it in Jest: + +```javascript +import { withGonExperiment } from 'helpers/experimentation_helper'; +import Tracking from '~/tracking'; + +describe('event tracking', () => { + describe('with tracking data', () => { + withGonExperiment('signupFlow'); + + beforeEach(() => { + jest.spyOn(Tracking, 'event').mockImplementation(() => {}); + + gon.tracking_data = { + category: 'Growth::Activation::Experiment::SignUpFlow', + action: 'action', + value: 'value', + label: 'experimentation_subject_id', + property: 'experimental_group' + }; + }); + + it('should track data', () => { + performAction() + + expect(Tracking.event).toHaveBeenCalledWith( + 'Growth::Activation::Experiment::SignUpFlow', + 'action', + { + value: 'value', + label: 'experimentation_subject_id', + property: 'experimental_group' + }, + ); + }); + }); +}); +``` + +## Record experiment user + +In addition to the anonymous tracking of events, we can also record which users +have participated in which experiments, and whether they were given the control +experience or the experimental experience. + +The `record_experiment_user` helper method is available to all controllers, and it +enables you to record these experiment participants (the current user) and which +experience they were given: + +```ruby +before_action do + record_experiment_user(:signup_flow) +end +``` + +Subsequent calls to this method for the same experiment and the same user have no +effect unless the user is then enrolled into a different experience. This happens +when we roll out the experimental experience to a greater percentage of users. + +This data is completely separate from the [events tracking data](#implement-tracking-events). +They are not linked together in any way. + +### Add context + +You can add arbitrary context data in a hash which gets stored as part of the experiment +user record. New calls to the `record_experiment_user` with newer contexts are merged +deeply into the existing context. + +This data can then be used by data analytics dashboards. + +```ruby +before_action do + record_experiment_user(:signup_flow, foo: 42, bar: { a: 22}) + # context is { "foo" => 42, "bar" => { "a" => 22 }} +end + +# Additional contexts for newer record calls are merged deeply +record_experiment_user(:signup_flow, foo: 40, bar: { b: 2 }, thor: 3) +# context becomes { "foo" => 40, "bar" => { "a" => 22, "b" => 2 }, "thor" => 3} +``` + +## Record experiment conversion event + +Along with the tracking of backend and frontend events and the +[recording of experiment participants](#record-experiment-user), we can also record +when a user performs the desired conversion event action. For example: + +- **Experimental experience:** Show an in-product nudge to test if the change causes more + people to sign up for trials. +- **Conversion event:** The user starts a trial. + +The `record_experiment_conversion_event` helper method is available to all controllers. +Use it to record the conversion event for the current user, regardless of whether +the user is in the control or experimental group: + +```ruby +before_action do + record_experiment_conversion_event(:signup_flow) +end +``` + +Note that the use of this method requires that we have first +[recorded the user](#record-experiment-user) as being part of the experiment. + +## Enable the experiment + +After all merge requests have been merged, use [ChatOps](../../ci/chatops/index.md) in the +[appropriate channel](../feature_flags/controls.md#communicate-the-change) to start the experiment for 10% of the users. +The feature flag should have the name of the experiment with the `_experiment_percentage` suffix appended. +For visibility, share any commands run against production in the `#s_growth` channel: + + ```shell + /chatops run feature set signup_flow_experiment_percentage 10 + ``` + + If you notice issues with the experiment, you can disable the experiment by removing the feature flag: + + ```shell + /chatops run feature delete signup_flow_experiment_percentage + ``` + +## Add user to experiment group manually + +To force the application to add your current user into the experiment group, +add a query string parameter to the path where the experiment runs. If you add the +query string parameter, the experiment works only for this request, and doesn't work +after following links or submitting forms. + +For example, to forcibly enable the `EXPERIMENT_KEY` experiment, add `force_experiment=EXPERIMENT_KEY` +to the URL: + +```shell +https://gitlab.com/?force_experiment= +``` + +## Add user to experiment group with a cookie + +You can force the current user into the experiment group for `` +during the browser session by using your browser's developer tools: + +```javascript +document.cookie = "force_experiment=; path=/"; +``` + +Use a comma to list more than one experiment to be forced: + +```javascript +document.cookie = "force_experiment=,; path=/"; +``` + +To clear the experiments, unset the `force_experiment` cookie: + +```javascript +document.cookie = "force_experiment=; path=/"; +``` + +## Testing and test helpers + +### RSpec + +Use the following in RSpec to mock the experiment: + +```ruby +context 'when the experiment is active' do + before do + stub_experiment(signup_flow: true) + end + + context 'when the user is in the experimental group' do + before do + stub_experiment_for_subject(signup_flow: true) + end + + it { is_expected.to do_experimental_thing } + end + + context 'when the user is in the control group' do + before do + stub_experiment_for_subject(signup_flow: false) + end + + it { is_expected.to do_control_thing } + end +end +``` + +### Jest + +Use the following in Jest to mock the experiment: + +```javascript +import { withGonExperiment } from 'helpers/experimentation_helper'; + +describe('given experiment is enabled', () => { + withGonExperiment('signupFlow'); + + it('should do the experimental thing', () => { + expect(wrapper.find('.js-some-experiment-triggered-element')).toEqual(expect.any(Element)); + }); +}); +``` diff --git a/doc/development/experiment_guide/gitlab_experiment.md b/doc/development/experiment_guide/gitlab_experiment.md new file mode 100644 index 00000000000..6b15449b812 --- /dev/null +++ b/doc/development/experiment_guide/gitlab_experiment.md @@ -0,0 +1,547 @@ +--- +stage: Growth +group: Adoption +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Implementing an A/B/n experiment using GLEX + +## Introduction + +`Gitlab::Experiment` (GLEX) is tightly coupled with the concepts provided by +[Feature flags in development of GitLab](../feature_flags/index.md). Here, we refer +to this layer as feature flags, and may also use the term Flipper, because we +built our development and experiment feature flags atop it. + +You're strongly encouraged to read and understand the +[Feature flags in development of GitLab](../feature_flags/index.md) portion of the +documentation before considering running experiments. Experiments add additional +concepts which may seem confusing or advanced without understanding the underpinnings +of how GitLab uses feature flags in development. One concept: GLEX supports multivariate +experiments, which are sometimes referred to as A/B/n tests. + +The [`gitlab-experiment` project](https://gitlab.com/gitlab-org/gitlab-experiment) +exists in a separate repository, so it can be shared across any GitLab property that uses +Ruby. You should feel comfortable reading the documentation on that project as well +if you want to dig into more advanced topics. + +## Glossary of terms + +To ensure a shared language, you should understand these fundamental terms we use +when communicating about experiments: + +- `experiment`: Any deviation of code paths we want to run at some times, but not others. +- `context`: A consistent experience we provide in an experiment. +- `control`: The default, or "original" code path. +- `candidate`: Defines an experiment with only one code path. +- `variant(s)`: Defines an experiment with multiple code paths. + +### How it works + +Use this decision tree diagram to understand how GLEX works. When an experiment runs, +the following logic is executed to determine what variant should be provided, +given how the experiment has been defined and using the provided context: + +```mermaid +graph TD + GP[General Pool/Population] --> Running? + Running? -->|Yes| Cached?[Cached? / Pre-segmented?] + Running? -->|No| Excluded[Control / No Tracking] + Cached? -->|No| Excluded? + Cached? -->|Yes| Cached[Cached Value] + Excluded? -->|Yes / Cached| Excluded + Excluded? -->|No| Segmented? + Segmented? -->|Yes / Cached| VariantA + Segmented? -->|No| Included?[Experiment Group?] + Included? -->|Yes| Rollout + Included? -->|No| Control + Rollout -->|Cached| VariantA + Rollout -->|Cached| VariantB + Rollout -->|Cached| VariantC + +classDef included fill:#380d75,color:#ffffff,stroke:none +classDef excluded fill:#fca121,stroke:none +classDef cached fill:#2e2e2e,color:#ffffff,stroke:none +classDef default fill:#fff,stroke:#6e49cb + +class VariantA,VariantB,VariantC included +class Control,Excluded excluded +class Cached cached +``` + +## Implement an experiment + +Start by generating a feature flag using the `bin/feature-flag` command as you +normally would for a development feature flag, making sure to use `experiment` for +the type. For the sake of documentation let's name our feature flag (and experiment) +"pill_color". + +```shell +bin/feature-flag pill_color -t experiment +``` + +After you generate the desired feature flag, you can immediately implement an +experiment in code. An experiment implementation can be as simple as: + +```ruby +experiment(:pill_color, actor: current_user) do |e| + e.use { 'control' } + e.try(:red) { 'red' } + e.try(:blue) { 'blue' } +end +``` + +When this code executes, the experiment is run, a variant is assigned, and (if within a +controller or view) a `window.gon.experiment.pillColor` object will be available in the +client layer, with details like: + +- The assigned variant. +- The context key for client tracking events. + +In addition, when an experiment runs, an event is tracked for +the experiment `:assignment`. We cover more about events, tracking, and +the client layer later. + +In local development, you can make the experiment active by using the feature flag +interface. You can also target specific cases by providing the relevant experiment +to the call to enable the feature flag: + +```ruby +# Enable for everyone +Feature.enable(:pill_color) + +# Get the `experiment` method -- already available in controllers, views, and mailers. +include Gitlab::Experiment::Dsl +# Enable for only the first user +Feature.enable(:pill_color, experiment(:pill_color, actor: User.first)) +``` + +To roll out your experiment feature flag on an environment, run +the following command using ChatOps (which is covered in more depth in the +[Feature flags in development of GitLab](../feature_flags/index.md) documentation). +This command creates a scenario where half of everyone who encounters +the experiment would be assigned the _control_, 25% would be assigned the _red_ +variant, and 25% would be assigned the _blue_ variant: + +```slack +/chatops run feature set pill_color 50 --actors +``` + +For an even distribution in this example, change the command to set it to 66% instead +of 50. + +NOTE: +To immediately stop running an experiment, use the +`/chatops run feature set pill_color false` command. + +WARNING: +We strongly recommend using the `--actors` flag when using the ChatOps commands, +as anything else may give odd behaviors due to how the caching of variant assignment is +handled. + +We can also implement this experiment in a HAML file with HTML wrappings: + +```haml +#cta-interface + - experiment(:pill_color, actor: current_user) do |e| + - e.use do + .pill-button control + - e.try(:red) do + .pill-button.red red + - e.try(:blue) do + .pill-button.blue blue +``` + +### The importance of context + +In our previous example experiment, our context (this is an important term) is a hash +that's set to `{ actor: current_user }`. Context must be unique based on how you +want to run your experiment, and should be understood at a lower level. + +It's expected, and recommended, that you use some of these +contexts to simplify reporting: + +- `{ actor: current_user }`: Assigns a variant and is "sticky" to each user + (or "client" if `current_user` is nil) who enters the experiment. +- `{ project: project }`: Assigns a variant and is "sticky" to the project currently + being viewed. If running your experiment is more useful when viewing a project, + rather than when a specific user is viewing any project, consider this approach. +- `{ group: group }`: Similar to the project example, but applies to a wider + scope of projects and users. +- `{ actor: current_user, project: project }`: Assigns a variant and is "sticky" + to the user who is viewing the given project. This creates a different variant + assignment possibility for every project that `current_user` views. Understand this + can create a large cache size if an experiment like this in a highly trafficked part + of the application. +- `{ wday: Time.current.wday }`: Assigns a variant based on the current day of the + week. In this example, it would consistently assign one variant on Friday, and a + potentially different variant on Saturday. + +Context is critical to how you define and report on your experiment. It's usually +the most important aspect of how you choose to implement your experiment, so consider +it carefully, and discuss it with the wider team if needed. Also, take into account +that the context you choose affects our cache size. + +After the above examples, we can state the general case: *given a specific +and consistent context, we can provide a consistent experience and track events for +that experience.* To dive a bit deeper into the implementation details: a context key +is generated from the context that's provided. Use this context key to: + +- Determine the assigned variant. +- Identify events tracked against that context key. + +We can think about this as the experience that we've rendered, which is both dictated +and tracked by the context key. The context key is used to track the interaction and +results of the experience we've rendered to that context key. These concepts are +somewhat abstract and hard to understand initially, but this approach enables us to +communicate about experiments as something that's wider than just user behavior. + +NOTE: +Using `actor:` utilizes cookies if the `current_user` is nil. If you don't need +cookies though - meaning that the exposed functionality would only be visible to +signed in users - `{ user: current_user }` would be just as effective. + +WARNING: +The caching of variant assignment is done by using this context, and so consider +your impact on the cache size when defining your experiment. If you use +`{ time: Time.current }` you would be inflating the cache size every time the +experiment is run. Not only that, your experiment would not be "sticky" and events +wouldn't be resolvable. + +### Advanced experimentation + +GLEX allows for two general implementation styles: + +1. The simple experiment style described previously. +1. A more advanced style where an experiment class can be provided. + +The advanced style is handled by naming convention, and works similar to what you +would expect in Rails. + +To generate a custom experiment class that can override the defaults in +`ApplicationExperiment` (our base GLEX implementation), use the rails generator: + +```shell +rails generate gitlab:experiment pill_color control red blue +``` + +This generates an experiment class in `app/experiments/pill_color_experiment.rb` +with the variants (or _behaviors_) we've provided to the generator. Here's an example +of how that class would look after migrating the previous example into it: + +```ruby +class PillColorExperiment < ApplicationExperiment + def control_behavior + 'control' + end + + def red_behavior + 'red' + end + + def blue_behavior + 'blue' + end +end +``` + +We can now simplify where we run our experiment to the following call, instead of +providing the block we were initially providing, by explicitly calling `run`: + +```ruby +experiment(:pill_color, actor: current_user).run +``` + +The _behavior_ methods we defined in our experiment class represent the default +implementation. You can still use the block syntax to override these _behavior_ +methods however, so the following would also be valid: + +```ruby +experiment(:pill_color, actor: current_user) do |e| + e.use { 'control' } +end +``` + +NOTE: +When passing a block to the `experiment` method, it is implicitly invoked as +if `run` has been called. + +#### Segmentation rules + +You can use runtime segmentation rules to, for instance, segment contexts into a specific +variant. The `segment` method is a callback (like `before_action`) and so allows providing +a block or method name. + +In this example, any user named `'Richard'` would always be assigned the _red_ +variant, and any account older than 2 weeks old would be assigned the _blue_ variant: + +```ruby +class PillColorExperiment < ApplicationExperiment + segment(variant: :red) { context.actor.first_name == 'Richard' } + segment :old_account?, variant: :blue + + # ...behaviors + + private + + def old_account? + context.actor.created_at < 2.weeks.ago + end +end +``` + +When an experiment runs, the segmentation rules are executed in the order they're +defined. The first segmentation rule to produce a truthy result assigns the variant. + +In our example, any user named `'Richard'`, regardless of account age, will always +be assigned the _red_ variant. If you want the opposite logic, flip the order. + +NOTE: +Keep in mind when defining segmentation rules: after a truthy result, the remaining +segmentation rules are skipped to achieve optimal performance. + +#### Exclusion rules + +Exclusion rules are similar to segmentation rules, but are intended to determine +if a context should even be considered as something we should include in the experiment +and track events toward. Exclusion means we don't care about the events in relation +to the given context. + +These examples exclude all users named `'Richard'`, *and* any account +older than 2 weeks old. Not only are they given the control behavior - which could +be nothing - but no events are tracked in these cases as well. + +```ruby +class PillColorExperiment < ApplicationExperiment + exclude :old_account?, ->{ context.actor.first_name == 'Richard' } + + # ...behaviors + + private + + def old_account? + context.actor.created_at < 2.weeks.ago + end +end +``` + +We can also do exclusion when we run the experiment. For instance, +if we wanted to prevent the inclusion of non-administrators in an experiment, consider +the following experiment. This type of logic enables us to do complex experiments +while preventing us from passing things into our experiments, because +we want to minimize passing things into our experiments: + +```ruby +experiment(:pill_color, actor: current_user) do |e| + e.exclude! unless can?(current_user, :admin_project, project) +end +``` + +You may also need to check exclusion in custom tracking logic by calling `should_track?`: + +```ruby +class PillColorExperiment < ApplicationExperiment + # ...behaviors + + def expensive_tracking_logic + return unless should_track? + + track(:my_event, value: expensive_method_call) + end +end +``` + +Exclusion rules aren't the best way to determine if an experiment is active. Override +the `enabled?` method for a high-level way of determining if an experiment should +run and track. Make the `enabled?` check as efficient as possible because it's the +first early opt-out path an experiment can implement. + +### Tracking events + +One of the most important aspects of experiments is gathering data and reporting on +it. GLEX provides an interface that allows tracking events across an experiment. +You can implement it consistently if you provide the same context between +calls to your experiment. If you do not yet understand context, you should read +about contexts now. + +We can assume we run the experiment in one or a few places, but +track events potentially in many places. The tracking call remains the same, with +the arguments you would normally use when +[tracking events using snowplow](../snowplow.md). The easiest example +of tracking an event in Ruby would be: + +```ruby +experiment(:pill_color, actor: current_user).track(:created) +``` + +When you run an experiment with any of these examples, an `:assigned` event +is tracked automatically by default. All events that are tracked from an +experiment have a special +[experiment context](https://gitlab.com/gitlab-org/iglu/-/blob/master/public/schemas/com.gitlab/gitlab_experiment/jsonschema/1-0-0) +added to the event. This can be used - typically by the data team - to create a connection +between the events on a given experiment. + +If our current user hasn't encountered the experiment yet (meaning where the experiment +is run), and we track an event for them, they are assigned a variant and see +that variant if they ever encountered the experiment later, when an `:assignment` +event would be tracked at that time for them. + +NOTE: +GitLab tries to be sensitive and respectful of our customers regarding tracking, +so GLEX allows us to implement an experiment without ever tracking identifying +IDs. It's not always possible, though, based on experiment reporting requirements. +You may be asked from time to time to track a specific record ID in experiments. +The approach is largely up to the PM and engineer creating the implementation. +No recommendations are provided here at this time. + +## Test with RSpec + +This gem provides some RSpec helpers and custom matchers. These are in flux as of GitLab 13.10. + +First, require the RSpec support file to mix in some of the basics: + +```ruby +require 'gitlab/experiment/rspec' +``` + +You still need to include matchers and other aspects, which happens +automatically for files in `spec/experiments`, but for other files and specs +you want to include it in, you can specify the `:experiment` type: + +```ruby +it "tests", :experiment do +end +``` + +### Stub helpers + +You can stub experiments using `stub_experiments`. Pass it a hash using experiment +names as the keys, and the variants you want each to resolve to, as the values: + +```ruby +# Ensures the experiments named `:example` & `:example2` are both +# "enabled" and that each will resolve to the given variant +# (`:my_variant` & `:control` respectively). +stub_experiments(example: :my_variant, example2: :control) + +experiment(:example) do |e| + e.enabled? # => true + e.variant.name # => 'my_variant' +end + +experiment(:example2) do |e| + e.enabled? # => true + e.variant.name # => 'control' +end +``` + +### Exclusion and segmentation matchers + +You can also test the exclusion and segmentation matchers. + +```ruby +class ExampleExperiment < ApplicationExperiment + exclude { context.actor.first_name == 'Richard' } + segment(variant: :candidate) { context.actor.username == 'jejacks0n' } +end + +excluded = double(username: 'rdiggitty', first_name: 'Richard') +segmented = double(username: 'jejacks0n', first_name: 'Jeremy') + +# exclude matcher +expect(experiment(:example)).to exclude(actor: excluded) +expect(experiment(:example)).not_to exclude(actor: segmented) + +# segment matcher +expect(experiment(:example)).to segment(actor: segmented).into(:candidate) +expect(experiment(:example)).not_to segment(actor: excluded) +``` + +### Tracking matcher + +Tracking events is a major aspect of experimentation. We try +to provide a flexible way to ensure your tracking calls are covered. + +You can do this on the instance level or at an "any instance" level: + +```ruby +subject = experiment(:example) + +expect(subject).to track(:my_event) + +subject.track(:my_event) +``` + +You can use the `on_any_instance` chain method to specify that it could happen on +any instance of the experiment. This helps you if you're calling +`experiment(:example).track` downstream: + +```ruby +expect(experiment(:example)).to track(:my_event).on_any_instance + +experiment(:example).track(:my_event) +``` + +A full example of the methods you can chain onto the `track` matcher: + +```ruby +expect(experiment(:example)).to track(:my_event, value: 1, property: '_property_') + .on_any_instance + .with_context(foo: :bar) + .for(:variant_name) + +experiment(:example, :variant_name, foo: :bar).track(:my_event, value: 1, property: '_property_') +``` + +## Experiments in the client layer + +This is in flux as of GitLab 13.10, and can't be documented just yet. + +Any experiment that's been run in the request lifecycle surfaces in `window.gon.experiment`, +and matches [this schema](https://gitlab.com/gitlab-org/iglu/-/blob/master/public/schemas/com.gitlab/gitlab_experiment/jsonschema/1-0-0) +so you can use it when resolving some concepts around experimentation in the client layer. + +## Notes on feature flags + +NOTE: +We use the terms "enabled" and "disabled" here, even though it's against our +[documentation style guide recommendations](../documentation/styleguide/index.md#avoid-ableist-language) +because these are the terms that the feature flag documentation uses. + +You may already be familiar with the concept of feature flags in GitLab, but using +feature flags in experiments is a bit different. While in general terms, a feature flag +is viewed as being either `on` or `off`, this isn't accurate for experiments. + +Generally, `off` means that when we ask if a feature flag is enabled, it will always +return `false`, and `on` means that it will always return `true`. An interim state, +considered `conditional`, also exists. GLEX takes advantage of this trinary state of +feature flags. To understand this `conditional` aspect: consider that either of these +settings puts a feature flag into this state: + +- Setting a `percentage_of_actors` of any percent greater than 0%. +- Enabling it for a single user or group. + +Conditional means that it returns `true` in some situations, but not all situations. + +When a feature flag is disabled (meaning the state is `off`), the experiment is +considered _inactive_. You can visualize this in the [decision tree diagram](#how-it-works) +as reaching the first [Running?] node, and traversing the negative path. + +When a feature flag is rolled out to a `percentage_of_actors` or similar (meaning the +state is `conditional`) the experiment is considered to be _running_ +where sometimes the control is assigned, and sometimes the candidate is assigned. +We don't refer to this as being enabled, because that's a confusing and overloaded +term here. In the experiment terms, our experiment is _running_, and the feature flag is +`conditional`. + +When a feature flag is enabled (meaning the state is `on`), the candidate will always be +assigned. + +We should try to be consistent with our terms, and so for experiments, we have an +_inactive_ experiment until we set the feature flag to `conditional`. After which, +our experiment is then considered _running_. If you choose to "enable" your feature flag, +you should consider the experiment to be _resolved_, because everyone is assigned +the candidate unless they've opted out of experimentation. + +As of GitLab 13.10, work is being done to improve this process and how we communicate +about it. diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md index 21c61324dc1..15430831f4a 100644 --- a/doc/development/experiment_guide/index.md +++ b/doc/development/experiment_guide/index.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w Experiments can be conducted by any GitLab team, most often the teams from the [Growth Sub-department](https://about.gitlab.com/handbook/engineering/development/growth/). Experiments are not tied to releases because they primarily target GitLab.com. -Experiments are run as an A/B test and are behind a feature flag to turn the test on or off. Based on the data the experiment generates, the team decides if the experiment had a positive impact and should be made the new default or rolled back. +Experiments are run as an A/B/n test, and are behind a feature flag to turn the test on or off. Based on the data the experiment generates, the team decides if the experiment had a positive impact and should be made the new default, or rolled back. ## Experiment tracking issue @@ -36,386 +36,27 @@ and link to the issue that resolves the experiment. If the experiment is successful and becomes part of the product, any follow up issues should be addressed. -## Experiments using `gitlab-experiment` +## Implementing an experiment -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/300383) in GitLab 13.7. -> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. -> - It's enabled on GitLab.com. -> - It is not yet intended for use in GitLab self-managed instances. +There are currently two options when implementing an experiment. -[GitLab Experiment](https://gitlab.com/gitlab-org/gitlab-experiment/) is a gem included -in GitLab that can be used for running experiments. +One is built into GitLab directly and has been around for a while (this is called +`Exerimentation Module`), and the other is provided by +[`gitlab-experiment`](https://gitlab.com/gitlab-org/gitlab-experiment) and is referred +to as `Gitlab::Experiment` -- GLEX for short. -## How to create an A/B test using `experimentation.rb` +Both approaches use [experiment](../feature_flags/index.md#experiment-type) +feature flags, and there is currently no strong suggestion to use one over the other. -### Implement the experiment +| Feature | `Experimentation Module` | GLEX | +| -------------------- |------------------------- | ---- | +| Record user grouping | Yes | No | +| Uses feature flags | Yes | Yes | +| Multivariate (A/B/n) | No | Yes | -1. Add the experiment to the `Gitlab::Experimentation::EXPERIMENTS` hash in [`experimentation.rb`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib%2Fgitlab%2Fexperimentation.rb): +- [Implementing an A/B experiment using `Experimentation Module`](experimentation.md) +- [Implementing an A/B/n experiment using GLEX](gitlab_experiment.md) - ```ruby - EXPERIMENTS = { - other_experiment: { - #... - }, - # Add your experiment here: - signup_flow: { - tracking_category: 'Growth::Activation::Experiment::SignUpFlow' # Used for providing the category when setting up tracking data - } - }.freeze - ``` - -1. Use the experiment in the code. - - Experiments can be performed on a `subject`. The `subject` that gets provided needs to respond to `to_global_id` or `to_s`. - The resulting string is bucketed and assigned to either the control or the experimental group. It's therefore necessary to always provide the same `subject` for an experiment to have the same experience. - - - Use this standard for the experiment in a controller: - - Experiment run for a user: - - ```ruby - class ProjectController < ApplicationController - def show - # experiment_enabled?(:experiment_key) is also available in views and helpers - if experiment_enabled?(:signup_flow, subject: current_user) - # render the experiment - else - # render the original version - end - end - end - ``` - - or experiment run for a namespace: - - ```ruby - if experiment_enabled?(:signup_flow, subject: namespace) - # experiment code - else - # control code - end - ``` - - When no subject is given, it falls back to a cookie that gets set and is consistent until - the cookie gets deleted. - - ```ruby - class RegistrationController < ApplicationController - def show - # falls back to a cookie - if experiment_enabled?(:signup_flow) - # render the experiment - else - # render the original version - end - end - end - ``` - - - Make the experiment available to the frontend in a controller: - - ```ruby - before_action do - push_frontend_experiment(:signup_flow, subject: current_user) - end - ``` - - The above checks whether the experiment is enabled and pushes the result to the frontend. - - You can check the state of the feature flag in JavaScript: - - ```javascript - import { isExperimentEnabled } from '~/experimentation'; - - if ( isExperimentEnabled('signupFlow') ) { - // ... - } - ``` - - - It is also possible to run an experiment outside of the controller scope, for example in a worker: - - ```ruby - class SomeWorker - def perform - # Check if the experiment is active at all (the percentage_of_time_value > 0) - return unless Gitlab::Experimentation.active?(:experiment_key) - - # Since we cannot access cookies in a worker, we need to bucket models based on a unique, unchanging attribute instead. - # It is therefore necessery to always provide the same subject. - if Gitlab::Experimentation.in_experiment_group?(:experiment_key, subject: user) - # execute experimental code - else - # execute control code - end - end - end - ``` - -### Implement the tracking events - -To determine whether the experiment is a success or not, we must implement tracking events -to acquire data for analyzing. We can send events to Snowplow via either the backend or frontend. -Read the [product intelligence guide](https://about.gitlab.com/handbook/product/product-intelligence-guide/) for more details. - -#### Track backend events - -The framework provides the following helper method that is available in controllers: - -```ruby -before_action do - track_experiment_event(:signup_flow, 'action', 'value', subject: current_user) -end -``` - -Which can be tested as follows: - -```ruby -context 'when the experiment is active and the user is in the experimental group' do - before do - stub_experiment(signup_flow: true) - stub_experiment_for_subject(signup_flow: true) - end - - it 'tracks an event', :snowplow do - subject - - expect_snowplow_event( - category: 'Growth::Activation::Experiment::SignUpFlow', - action: 'action', - value: 'value', - label: 'experimentation_subject_id', - property: 'experimental_group' - ) - end -end -``` - -#### Track frontend events - -The framework provides the following helper method that is available in controllers: - -```ruby -before_action do - push_frontend_experiment(:signup_flow, subject: current_user) - frontend_experimentation_tracking_data(:signup_flow, 'action', 'value', subject: current_user) -end -``` - -This pushes tracking data to `gon.experiments` and `gon.tracking_data`. - -```ruby -expect(Gon.experiments['signupFlow']).to eq(true) - -expect(Gon.tracking_data).to eq( - { - category: 'Growth::Activation::Experiment::SignUpFlow', - action: 'action', - value: 'value', - label: 'experimentation_subject_id', - property: 'experimental_group' - } -) -``` - -Which can then be used for tracking as follows: - -```javascript -import { isExperimentEnabled } from '~/lib/utils/experimentation'; -import Tracking from '~/tracking'; - -document.addEventListener('DOMContentLoaded', () => { - const signupFlowExperimentEnabled = isExperimentEnabled('signupFlow'); - - if (signupFlowExperimentEnabled && gon.tracking_data) { - const { category, action, ...data } = gon.tracking_data; - - Tracking.event(category, action, data); - } -} -``` - -Which can be tested in Jest as follows: - -```javascript -import { withGonExperiment } from 'helpers/experimentation_helper'; -import Tracking from '~/tracking'; - -describe('event tracking', () => { - describe('with tracking data', () => { - withGonExperiment('signupFlow'); - - beforeEach(() => { - jest.spyOn(Tracking, 'event').mockImplementation(() => {}); - - gon.tracking_data = { - category: 'Growth::Activation::Experiment::SignUpFlow', - action: 'action', - value: 'value', - label: 'experimentation_subject_id', - property: 'experimental_group' - }; - }); - - it('should track data', () => { - performAction() - - expect(Tracking.event).toHaveBeenCalledWith( - 'Growth::Activation::Experiment::SignUpFlow', - 'action', - { - value: 'value', - label: 'experimentation_subject_id', - property: 'experimental_group' - }, - ); - }); - }); -}); -``` - -### Record experiment user - -In addition to the anonymous tracking of events, we can also record which users have participated in which experiments and whether they were given the control experience or the experimental experience. - -The `record_experiment_user` helper method is available to all controllers, and it enables you to record these experiment participants (the current user) and which experience they were given: - -```ruby -before_action do - record_experiment_user(:signup_flow) -end -``` - -Subsequent calls to this method for the same experiment and the same user have no effect unless the user has gets enrolled into a different experience. This happens when we roll out the experimental experience to a greater percentage of users. - -Note that this data is completely separate from the [events tracking data](#implement-the-tracking-events). They are not linked together in any way. - -#### Add context - -You can add arbitrary context data in a hash which gets stored as part of the experiment user record. New calls to the `record_experiment_user` with newer contexts get merged deeply into the existing context. - -This data can then be used by data analytics dashboards. - -```ruby -before_action do - record_experiment_user(:signup_flow, foo: 42, bar: { a: 22}) - # context is { "foo" => 42, "bar" => { "a" => 22 }} -end - -# Additional contexts for newer record calls are merged deeply -record_experiment_user(:signup_flow, foo: 40, bar: { b: 2 }, thor: 3) -# context becomes { "foo" => 40, "bar" => { "a" => 22, "b" => 2 }, "thor" => 3} -``` - -### Record experiment conversion event - -Along with the tracking of backend and frontend events and the [recording of experiment participants](#record-experiment-user), we can also record when a user performs the desired conversion event action. For example: - -- **Experimental experience:** Show an in-product nudge to see if it causes more people to sign up for trials. -- **Conversion event:** The user starts a trial. - -The `record_experiment_conversion_event` helper method is available to all controllers. It enables us to record the conversion event for the current user, regardless of whether they are in the control or experimental group: - -```ruby -before_action do - record_experiment_conversion_event(:signup_flow) -end -``` - -Note that the use of this method requires that we have first [recorded the user as being part of the experiment](#record-experiment-user). - -### Enable the experiment - -After all merge requests have been merged, use [`chatops`](../../ci/chatops/index.md) in the -[appropriate channel](../feature_flags/controls.md#communicate-the-change) to start the experiment for 10% of the users. -The feature flag should have the name of the experiment with the `_experiment_percentage` suffix appended. -For visibility, please also share any commands run against production in the `#s_growth` channel: - - ```shell - /chatops run feature set signup_flow_experiment_percentage 10 - ``` - - If you notice issues with the experiment, you can disable the experiment by removing the feature flag: - - ```shell - /chatops run feature delete signup_flow_experiment_percentage - ``` - -### Manually force the current user to be in the experiment group - -You may force the application to put your current user in the experiment group. To do so -add a query string parameter to the path where the experiment runs. If you do so, -the experiment will work only for this request and won't work after following links or submitting forms. - -For example, to forcibly enable the `EXPERIMENT_KEY` experiment, add `force_experiment=EXPERIMENT_KEY` -to the URL: - -```shell -https://gitlab.com/?force_experiment= -``` - -### A cookie-based approach to force an experiment - -It's possible to force the current user to be in the experiment group for `` -during the browser session by using your browser's developer tools: - -```javascript -document.cookie = "force_experiment=; path=/"; -``` - -Use a comma to list more than one experiment to be forced: - -```javascript -document.cookie = "force_experiment=,; path=/"; -``` - -To clear the experiments, unset the `force_experiment` cookie: - -```javascript -document.cookie = "force_experiment=; path=/"; -``` - -### Testing and test helpers - -#### RSpec - -Use the following in RSpec to mock the experiment: - -```ruby -context 'when the experiment is active' do - before do - stub_experiment(signup_flow: true) - end - - context 'when the user is in the experimental group' do - before do - stub_experiment_for_subject(signup_flow: true) - end - - it { is_expected.to do_experimental_thing } - end - - context 'when the user is in the control group' do - before do - stub_experiment_for_subject(signup_flow: false) - end - - it { is_expected.to do_control_thing } - end -end -``` - -#### Jest - -Use the following in Jest to mock the experiment: - -```javascript -import { withGonExperiment } from 'helpers/experimentation_helper'; - -describe('given experiment is enabled', () => { - withGonExperiment('signupFlow'); - - it('should do the experimental thing', () => { - expect(wrapper.find('.js-some-experiment-triggered-element')).toEqual(expect.any(Element)); - }); -}); -``` +Historical Context: `Experimentation Module` was built iteratively with the needs that +appeared while implementing Growth sub-department experiments, while GLEX was built +with the learnings of the team and an easier to use API. diff --git a/doc/development/export_csv.md b/doc/development/export_csv.md index 0bf12149779..c301b6b9d66 100644 --- a/doc/development/export_csv.md +++ b/doc/development/export_csv.md @@ -14,7 +14,7 @@ This document lists the different implementations of CSV export in GitLab codeba | Downloading | - Query and write data in batches to a temporary file.
- Loads the file into memory.
- Sends the file to the client. | - Report available immediately. | - Large amount of data might cause request timeout.
- Memory intensive.
- Request expires when user navigates to a different page. | [Export Chain of Custody Report](../user/compliance/compliance_dashboard/#chain-of-custody-report) | | As email attachment | - Asynchronously process the query with background job.
- Email uses the export as an attachment. | - Asynchronous processing. | - Requires users use a different app (email) to download the CSV.
- Email providers may limit attachment size. | - [Export Issues](../user/project/issues/csv_export.md)
- [Export Merge Requests](../user/project/merge_requests/csv_export.md) | | As downloadable link in email (*) | - Asynchronously process the query with background job.
- Email uses an export link. | - Asynchronous processing.
- Bypasses email provider attachment size limit. | - Requires users use a different app (email).
- Requires additional storage and cleanup. | [Export User Permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/1772) | -| Polling (non-persistent state) | - Asynchronously processes the query with the background job.
- Frontend(FE) polls every few seconds to check if CSV file is ready. | - Asynchronous processing.
- Automatically downloads to local machine on completion.
- In-app solution. | - Non-persistable request - request expires when user navigates to a different page.
- API is processed for each polling request. | [Export Vulnerabilities](../user/application_security/vulnerability_report/#export-vulnerabilities) | +| Polling (non-persistent state) | - Asynchronously processes the query with the background job.
- Frontend(FE) polls every few seconds to check if CSV file is ready. | - Asynchronous processing.
- Automatically downloads to local machine on completion.
- In-app solution. | - Non-persistable request - request expires when user navigates to a different page.
- API is processed for each polling request. | [Export Vulnerabilities](../user/application_security/vulnerability_report/#export-vulnerability-details) | | Polling (persistent state) (*) | - Asynchronously processes the query with background job.
- Backend (BE) maintains the export state
- FE polls every few seconds to check status.
- FE shows 'Download link' when export is ready.
- User can download or regenerate a new report. | - Asynchronous processing.
- No database calls made during the polling requests (HTTP 304 status is returned until export status changes).
- Does not require user to stay on page until export is complete.
- In-app solution.
- Can be expanded into a generic CSV feature (such as dashboard / CSV API). | - Requires to maintain export states in DB.
- Does not automatically download the CSV export to local machine, requires users to click 'Download' button. | [Export Merge Commits Report](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43055) | NOTE: diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index 5ad1a701fac..5f22c13ca06 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -4,14 +4,195 @@ group: unassigned info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Accessibility & Readability +# Accessibility + +Accessibility is important for users who use screen readers or rely on keyboard-only functionality +to ensure they have an equivalent experience to sighted mouse users. + +This page contains guidelines we should follow. + +## Quick summary + +Since [no ARIA is better than bad ARIA](https://www.w3.org/TR/wai-aria-practices/#no_aria_better_bad_aria), +review the following recommendations before using `aria-*`, `role`, and `tabindex`. +Use semantic HTML, which typically has accessibility semantics baked in, but always be sure to test with +[relevant combinations of screen readers and browsers](https://www.accessibility-developer-guide.com/knowledge/screen-readers/relevant-combinations/). + +In [WebAIM's accessibility analysis of the top million home pages](https://webaim.org/projects/million/#aria), +they found that "ARIA correlated to higher detectable errors". +It is likely that *misuse* of ARIA is a big cause of increased errors, +so when in doubt don't use `aria-*`, `role`, and `tabindex`, and stick with semantic HTML. + +## Provide accessible names to screen readers + +To provide markup with accessible names, ensure every: + +- `input` has an associated `label`. +- `button` and `a` have child text, or `aria-label` when text isn’t present. + For example, an icon button with no visible text. +- `img` has an `alt` attribute. +- `fieldset` has `legend` as its first child. +- `figure` has `figcaption` as its first child. +- `table` has `caption` as its first child. + +If the `label`, child text, or child element is not visually desired, +use `.gl-sr-only` to hide the element from everything but screen readers. + +Ensure the accessible name is descriptive enough to be understood in isolation. + +```html +// bad + +page + +// good + +GitLab's accessibility page +``` + +## Role + +In general, avoid using `role`. +Use semantic HTML elements that implicitly have a `role` instead. + +| Bad | Good | +| --- | --- | +| `
` | `