diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 12:26:25 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 12:26:25 +0000 |
| commit | a09983ae35713f5a2bbb100981116d31ce99826e (patch) | |
| tree | 2ee2af7bd104d57086db360a7e6d8c9d5d43667a /doc/development | |
| parent | 18c5ab32b738c0b6ecb4d0df3994000482f34bd8 (diff) | |
| download | gitlab-ce-a09983ae35713f5a2bbb100981116d31ce99826e.tar.gz | |
Add latest changes from gitlab-org/gitlab@13-2-stable-ee
Diffstat (limited to 'doc/development')
92 files changed, 3003 insertions, 956 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index d330d6d466e..ab86c252948 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -99,6 +99,9 @@ Complementary reads: - [Code comments](code_comments.md) - [Renaming features](renaming_features.md) - [Windows Development on GCP](windows.md) +- [Code Intelligence](code_intelligence/index.md) +- [Approval Rules](approval_rules.md) +- [Feature categorization](feature_categorization/index.md) ## Performance guides diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 43e96340d10..92e6add9f17 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -571,7 +571,8 @@ module Types end ``` -NOTE: **Note:** If the field's type already [has a particular +NOTE: **Note:** +If the field's type already [has a particular authorization](#type-authorization) then there is no need to add that same authorization to the field. @@ -717,11 +718,48 @@ will be returned as the result of the mutation. ### Naming conventions -Always provide a consistent GraphQL-name to the mutation, this name is -used to generate the input types and the field the mutation is mounted -on. The name should look like `<Resource being modified><Mutation -class name>`, for example the `Mutations::MergeRequests::SetWip` -mutation has GraphQL name `MergeRequestSetWip`. +Each mutation must define a `graphql_name`, which is the name of the mutation in the GraphQL schema. + +Example: + +```ruby +class UserUpdateMutation < BaseMutation + graphql_name 'UserUpdate' +end +``` + +Our GraphQL mutation names are historically inconsistent, but new mutation names should follow the +convention `'{Resource}{Action}'` or `'{Resource}{Action}{Attribute}'`. + +Mutations that **create** new resources should use the verb `Create`. + +Example: + +- `CommitCreate` + +Mutations that **update** data should use: + +- The verb `Update`. +- A domain-specific verb like `Set`, `Add`, or `Toggle` if more appropriate. + +Examples: + +- `EpicTreeReorder` +- `IssueSetWeight` +- `IssueUpdate` +- `TodoMarkDone` + +Mutations that **remove** data should use: + +- The verb `Delete` rather than `Destroy`. +- A domain-specific verb like `Remove` if more appropriate. + +Examples: + +- `AwardEmojiRemove` +- `NoteDelete` + +If you need advice for mutation naming, canvass the Slack `#graphql` channel for feedback. ### Arguments @@ -975,6 +1013,34 @@ to make sure the error information we are passing back is useful. See also the [frontend GraphQL guide](../development/fe_guide/graphql.md#handling-errors). +### Aliasing and deprecating mutations + +The `#mount_aliased_mutation` helper allows us to alias a mutation as +another name within `MutationType`. + +For example, to alias a mutation called `FooMutation` as `BarMutation`: + +```ruby +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) argument. + +Example: + +```ruby +mount_aliased_mutation 'UpdateFoo', + Mutations::Foo::Update, + deprecated: { reason: 'Use fooUpdate', milestone: '13.2' } +``` + +Deprecated mutations should be added to `Types::DeprecatedMutations` and +tested for within the unit test of `Types::MutationType`. The merge request +[!34798](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34798) +can be referred to as an example of this, including the method of testing +deprecated aliased mutations. + ## Validating arguments For validations of single arguments, use the diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index 6a044004926..327f919d7f4 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -98,6 +98,46 @@ For instance: Model.create(foo: params[:foo]) ``` +## Array types + +With Grape v1.3+, Array types must be defined with a `coerce_with` +block, or parameters will fail to validate when passed a string from an +API request. See the [Grape upgrading +documentation](https://github.com/ruby-grape/grape/blob/master/UPGRADING.md#ensure-that-array-types-have-explicit-coercions) +for more details. + +### Automatic coercion of nil inputs + +Prior to Grape v1.3.3, Array parameters with `nil` values would +automatically be coerced to an empty Array. However, due to [this pull +request in v1.3.3](https://github.com/ruby-grape/grape/pull/2040), this +is no longer the case. For example, suppose you define a PUT `/test` +request that has an optional parameter: + +```ruby +optional :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The user ids for this rule' +``` + +Normally, a request to PUT `/test?user_ids` would cause Grape to pass +`params` of `{ user_ids: nil }`. + +This may introduce errors with endpoints that expect a blank array and +do not handle `nil` inputs properly. To preserve the previous behavior, +there is a helper method `coerce_nil_params_to_array!` that is used +in the `before` block of all API calls: + +```ruby +before do + coerce_nil_params_to_array! +end +``` + +With this change, a request to PUT `/test?user_ids` will cause Grape to +pass `params` to be `{ user_ids: [] }`. + +There is [an open issue in the Grape tracker](https://github.com/ruby-grape/grape/issues/2068) +to make this easier. + ## Using HTTP status helpers For non-200 HTTP responses, use the provided helpers in `lib/api/helpers.rb` to ensure correct behavior (`not_found!`, `no_content!` etc.). These will `throw` inside Grape and abort the execution of your endpoint. diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index 1f7a9ff09b9..4d296451add 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -11,7 +11,7 @@ coordinate with others to [document](../administration/instance_limits.md) and communicate those limits. There is a guide about [introducing application -limits](https://about.gitlab.com/handbook/product/product-management/process/#introducing-application-limits). +limits](https://about.gitlab.com/handbook/product/product-processes/#introducing-application-limits). ## Development @@ -27,7 +27,8 @@ limit values. It's recommended to create separate migration script files. add_column(:plan_limits, :project_hooks, :integer, default: 100, null: false) ``` - NOTE: **Note:** Plan limits entries set to `0` mean that limits are not + NOTE: **Note:** + Plan limits entries set to `0` mean that limits are not enabled. You should use this setting only in special and documented circumstances. 1. (Optionally) Create the database migration that fine-tunes each level with @@ -57,7 +58,8 @@ limit values. It's recommended to create separate migration script files. end ``` -NOTE: **Note:** Some plans exist only on GitLab.com. This will be no-op +NOTE: **Note:** +Some plans exist only on GitLab.com. This will be no-op for plans that do not exist. ### Plan limits validation @@ -95,7 +97,8 @@ can be used to validate that a model does not exceed the limits. It ensures that the count of the records for the current model does not exceed the defined limit. -NOTE: **Note:** You must specify the limit scope of the object being validated +NOTE: **Note:** +You must specify the limit scope of the object being validated and the limit name if it's different from the pluralized model name. ```ruby @@ -143,4 +146,5 @@ GitLab.com: - `silver` - Namespaces and projects with a Silver subscription - `gold` - Namespaces and projects with a Gold subscription -NOTE: **Note:** The test environment doesn't have any plans. +NOTE: **Note:** +The test environment doesn't have any plans. diff --git a/doc/development/application_secrets.md b/doc/development/application_secrets.md new file mode 100644 index 00000000000..24755586cf8 --- /dev/null +++ b/doc/development/application_secrets.md @@ -0,0 +1,41 @@ +# Application secrets + +This page is a development guide for application secrets. + +## Secret entries + +|Entry |Description | +|--- |--- | +|`secret_key_base` | The base key to be used for generating a various secrets | +| `otp_key_base` | The base key for One Time Passwords, described in [User management](../raketasks/user_management.md#rotate-two-factor-authentication-encryption-key) | +|`db_key_base` | The base key to encrypt the data for `attr_encrypted` columns | +|`openid_connect_signing_key` | The singing key for OpenID Connect | + +## Where the secrets are stored + +|Installation type |Location | +|--- |--- | +|Omnibus |[`/etc/gitlab/gitlab-secrets.json`](https://docs.gitlab.com/omnibus/settings/backups.html#backup-and-restore-omnibus-gitlab-configuration) | +|Cloud Native GitLab Charts |[Kubernets Secrets](https://gitlab.com/gitlab-org/charts/gitlab/-/blob/f65c3d37fc8cf09a7987544680413552fb666aac/doc/installation/secrets.md#gitlab-rails-secret)| +|Source |`<path-to-gitlab-rails>/config/secrets.yml` (Automatically generated by [01_secret_token.rb](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/01_secret_token.rb)) | + +## Warning: Before you add a new secret to application secrets + +Before you add a new secret to [`config/initializers/01_secret_token.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/initializers/01_secret_token.rb), +make sure you also update Omnibus GitLab or updates will fail. Omnibus is responsible for writing the `secrets.yml` file. +If Omnibus doesn't know about a secret, Rails will attempt to write to the file, but this will fail because Rails doesn't have write access. +The same rules apply to Cloud Native GitLab charts, you must update the charts at first. +In case you need the secret to have same value on each node (which is usually the case) you need to make sure it's configured for all +GitLab.com environments prior to changing this file. + +**Examples** + +- [Change for source installation](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/27581) +- [Change for omnibus installation](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/3267) +- [Change for omnibus installation](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/4158) +- [Change for Cloud Native installation](https://gitlab.com/gitlab-org/charts/gitlab/-/merge_requests/1318) + +## Further iteration + +We might deprecate/remove this automatic secret generation '01_secret_token.rb' in the future. +Please see [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/222690) for more information. diff --git a/doc/development/approval_rules.md b/doc/development/approval_rules.md new file mode 100644 index 00000000000..65df82721de --- /dev/null +++ b/doc/development/approval_rules.md @@ -0,0 +1,280 @@ +# Approval Rules **(STARTER)** + +This document explains the backend design and flow of all related functionality +about [merge request approval rules](../user/project/merge_requests/merge_request_approvals.md). + +This should help contributors to understand the code design easier and to also +help see if there are parts to improve as the feature and its implementation +evolves. + +It's intentional that it doesn't contain too much implementation detail as they +can change often. The code should explain those things better. The components +mentioned here are the major parts of the application for the approval rules +feature to work. + +NOTE: **Note:** +This is a living document and should be updated accordingly when parts +of the codebase touched in this document changed/removed or when new components +are added. + +## Data Model + +```mermaid +erDiagram + Project ||--o{ MergeRequest: " " + Project ||--o{ ApprovalProjectRule: " " + ApprovalProjectRule }o--o{ User: " " + ApprovalProjectRule }o--o{ Group: " " + ApprovalProjectRule }o--o{ ProtectedBranch: " " + MergeRequest ||--|| ApprovalState: " " + ApprovalState ||--o{ ApprovalWrappedRule: " " + MergeRequest ||--o{ Approval: " " + MergeRequest ||--o{ ApprovalMergeRequestRule: " " + ApprovalMergeRequestRule }o--o{ User: " " + ApprovalMergeRequestRule }o--o{ Group: " " + ApprovalMergeRequestRule ||--o| ApprovalProjectRule: " " +``` + +### `Project` and `MergeRequest` + +`Project` and `MergeRequest` models are defined in `ee/app/models/ee/project.rb` +and `ee/app/models/ee/merge_request.rb`. They extend the non-EE versions since +approval rules is an EE only feature. Associations and other related stuff to +merge request approvals are defined here. + +### `ApprovalState` + +```mermaid +erDiagram + MergeRequest ||--|| ApprovalState: " " +``` + +`ApprovalState` class is defined in `ee/app/models/approval_state.rb`. It's not +an actual `ActiveRecord` model. This class encapsulates all logic related to the +state of the approvals for a certain merge request like: + +- Knowing the approval rules that are applicable to the merge request based on + its target branch. +- Knowing the approval rules that are applicable to a certain target branch. +- Checking if all rules were approved. +- Checking if approval is required. +- Knowing how many approvals were given or still required. + +It gets the approval rules data from the project (`ApprovalProjectRule`) or the +merge request (`ApprovalMergeRequestRule`) and wrap it as `ApprovalWrappedRule`. + +### `ApprovalProjectRule` + +```mermaid +erDiagram + Project ||--o{ ApprovalProjectRule: " " + ApprovalProjectRule }o--o{ User: " " + ApprovalProjectRule }o--o{ Group: " " + ApprovalProjectRule }o--o{ ProtectedBranch: " " +``` + +`ApprovalProjectRule` model is defined in `ee/app/models/approval_project_rule.rb`. + +A record is created/updated/deleted when an approval rule is added/edited/removed +via project settings or the [project level approvals API](../api/merge_request_approvals.md#project-level-mr-approvals). +The `ApprovalState` model get these records when approval rules are not +overwritten. + +The `protected_branches` attribute is set and used when a rule is scoped to +protected branches. See [Scoped to Protected Branch doc](../user/project/merge_requests/merge_request_approvals.md#scoped-to-protected-branch-premium) +for more information about the feature. + +### `ApprovalMergeRequestRule` + +```mermaid +erDiagram + MergeRequest ||--o{ ApprovalMergeRequestRule: " " + ApprovalMergeRequestRule }o--o{ User: " " + ApprovalMergeRequestRule }o--o{ Group: " " + ApprovalMergeRequestRule ||--o| ApprovalProjectRule: " " +``` + +`ApprovalMergeRequestRule` model is defined in `ee/app/models/approval_merge_request_rule.rb`. + +A record is created/updated/deleted when a rule is added/edited/removed via merge +request create/edit form or the [merge request level approvals API](../api/merge_request_approvals.md#merge-request-level-mr-approvals). + +The `approval_project_rule` is set when it is based from an existing `ApprovalProjectRule`. + +An `ApprovalMergeRequestRule` doesn't have `protected_branches` as it inherits +them from the `approval_project_rule` if not overridden. + +### `ApprovalWrappedRule` + +```mermaid +erDiagram + ApprovalState ||--o{ ApprovalWrappedRule: " " +``` + +`ApprovalWrappedRule` is defined in `ee/app/modes/approval_wrapped_rule.rb` and +is not an `ActiveRecord` model. It's used to wrap an `ApprovalProjectRule` or +`ApprovalMergeRequestRule` for common interface. It also has the following sub +types: + +- `ApprovalWrappedAnyApprovalRule` - for wrapping an `any_approver` rule. +- `ApprovalWrappedCodeOwnerRule` - for wrapping a `code_owner` rule. + +This class delegates most of the responsibilities to the approval rule it wraps +but it's also responsible for: + +- Checking if the approval rule is approved. +- Knowing how many approvals were given or still required for the approval rule. + +It gets this information from the approval rule and the `Approval` records from +the merge request. + +### `Approval` + +```mermaid +erDiagram + MergeRequest ||--o{ Approval: " " +``` + +`Approval` model is defined in `ee/app/models/approval.rb`. This model is +responsible for storing information about an approval made on a merge request. +Whenever an approval is given/revoked, a record is created/deleted. + +## Controllers and Services + +The following controllers and services below are being utilized for the approval +rules feature to work. + +### `API::ProjectApprovalSettings` + +This private API is defined in `ee/lib/api/project_approval_settings.rb`. + +This is used for the following: + +- Listing the approval rules in project settings. +- Creating/updating/deleting rules in project settings. +- Listing the approval rules on create merge request form. + +### `Projects::MergeRequests::CreationsController` + +This controller is defined in `app/controllers/projects/merge_requests/creations_controller.rb`. + +The `create` action of this controller is used when create merge request form is +submitted. It accepts the `approval_rules_attributes` parameter for creating/updating/deleting +`ApprovalMergeRequestRule` records. It passes the parameter along when it executes +`MergeRequests::CreateService`. + +### `Projects::MergeRequestsController` + +This controller is defined in `app/controllers/projects/merge_requests_controller.rb`. + +The `update` action of this controller is used when edit merge request form is +submitted. It's like `Projects::MergeRequests::CreationsController` but it executes +`MergeRequests::UpdateService` instead. + +### `API::MergeRequestApprovals` + +This API is defined in `ee/lib/api/merge_request_approvals.rb`. + +The [Approvals API endpoint](../api/merge_request_approvals.md#get-configuration-1) +is requested when merge request page loads. + +The `/projects/:id/merge_requests/:merge_request_iid/approval_settings` is a +private API endpoint used for the following: + +- Listing the approval rules on edit merge request form. +- Listing the approval rules on the merge request page. + +When approving/unapproving MR via UI and API, the [Approve Merge Request](../api/merge_request_approvals.md#approve-merge-request) +API endpoint and the [Unapprove Merge Request](../api/merge_request_approvals.md#unapprove-merge-request) +API endpoint are requested. They execute `MergeRequests::ApprovalService` and +`MergeRequests::RemoveApprovalService` accordingly. + +### `API::ProjectApprovalRules` and `API::MergeRequestApprovalRules` + +These APIs are defined in `ee/lib/api/project_approval_rules.rb` and +`ee/lib/api/merge_request_approval_rules.rb`. + +Used to list/create/update/delete project and merge request level rules via +[Merge request approvals API](../api/merge_request_approvals.md). + +Executes `ApprovalRules::CreateService`, `ApprovalRules::UpdateService`, +`ApprovalRules::ProjectRuleDestroyService`, and `ApprovalRules::MergeRequestRuleDestroyService` +accordingly. + +### `ApprovalRules::ParamsFilteringService` + +This service is defined in `ee/app/services/approval_rules/params_filtering_service.rb`. + +It is called only when `MergeRequests::CreateService` and +`MergeRequests::UpdateService` are executed. + +It is responsible for parsing `approval_rules_attributes` parameter to: + +- Remove it when user can't update approval rules. +- Filter the user IDs whether they are members of the project or not. +- Filter the group IDs whether they are visible to user. +- Identify the `any_approver` rule. +- Append hidden groups to it when specified. +- Append user defined inapplicable (rules that does not apply to MR's target + branch) approval rules. + +## Flow + +These flowcharts should help explain the flow from the controllers down to the +models for different functionalities. + +Some CRUD API endpoints are intentionally skipped because they are pretty +straightforward. + +### Creating a merge request with approval rules via web UI + +```mermaid +graph LR + Projects::MergeRequests::CreationsController --> MergeRequests::CreateService + MergeRequests::CreateService --> ApprovalRules::ParamsFilteringService + ApprovalRules::ParamsFilteringService --> MergeRequests::CreateService + MergeRequests::CreateService --> MergeRequest + MergeRequest --> db[(Database)] + MergeRequest --> User + MergeRequest --> Group + MergeRequest --> ApprovalProjectRule + User --> db[(Database)] + Group --> db[(Database)] + ApprovalProjectRule --> db[(Database)] +``` + +When updating, same flow is followed but it starts at `Projects::MergeRequestsController` +and executes `MergeRequests::UpdateService` instead. + +### Viewing the merge request approval rules on an MR page + +```mermaid +graph LR + API::MergeRequestApprovals --> MergeRequest + MergeRequest --> ApprovalState + ApprovalState --> id1{approval rules are overridden} + id1{approval rules are overridden} --> |No| ApprovalProjectRule & ApprovalMergeRequestRule + id1{approval rules are overridden} --> |Yes| ApprovalMergeRequestRule + ApprovalState --> ApprovalWrappedRule + ApprovalWrappedRule --> Approval +``` + +This flow gets initiated by the frontend component. The data returned will +then be used to display information on the MR widget. + +### Approving a merge request + +```mermaid +graph LR + API::MergeRequestApprovals --> MergeRequests::ApprovalService + MergeRequests::ApprovalService --> Approval + Approval --> db[(Database)] +``` + +When unapproving, same flow is followed but the `MergeRequests::RemoveApprovalService` +is executed instead. + +## TODO + +1. Add information related to other rule types (e.g. `code_owner` and `report_approver`). +1. Add information about side effects of approving/unapproving merge request. diff --git a/doc/development/architecture.md b/doc/development/architecture.md index f0ce033587d..8b28dd03017 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -140,29 +140,29 @@ Table description links: | [Elasticsearch](#elasticsearch) | Improved search within GitLab | ⤓ | ⤓ | ⤓ | ❌ | ⤓ | ⤓ | EE Only | | [Gitaly](#gitaly) | Git RPC service for handling all Git calls made by GitLab | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | | [GitLab Exporter](#gitlab-exporter) | Generates a variety of GitLab metrics | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab Geo Node](#gitlab-geo) | Geographically distributed GitLab nodes | ⚙ | ❌ | ❌ | ✅ | ❌ | ⚙ | EE Only | +| [GitLab Geo Node](#gitlab-geo) | Geographically distributed GitLab nodes | ⚙ | ⚙ | ❌ | ✅ | ❌ | ⚙ | EE Only | | [GitLab Managed Apps](#gitlab-managed-apps) | Deploy Helm, Ingress, Cert-Manager, Prometheus, a Runner, JupyterHub, or Knative to a cluster | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | CE & EE | | [GitLab Pages](#gitlab-pages) | Hosts static websites | ⚙ | ❌ | ❌ | ✅ | ⚙ | ⚙ | CE & EE | | [GitLab self-monitoring: Alertmanager](#alertmanager) | Deduplicates, groups, and routes alerts from Prometheus | ⚙ | ✅ | ⚙ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab self-monitoring: Grafana](#grafana) | Metrics dashboard | ✅ | ⤓ | ⤓ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab self-monitoring: Jaeger](#jaeger) | View traces generated by the GitLab instance | ❌ | ❌ | ❌ | ❌ | ⤓ | ⚙ | CE & EE | +| [GitLab self-monitoring: Grafana](#grafana) | Metrics dashboard | ✅ | ⚙ | ⤓ | ✅ | ❌ | ❌ | CE & EE | +| [GitLab self-monitoring: Jaeger](#jaeger) | View traces generated by the GitLab instance | ❌ | ⚙ | ❌ | ❌ | ⤓ | ⚙ | CE & EE | | [GitLab self-monitoring: Prometheus](#prometheus) | Time-series database, metrics collection, and query service | ✅ | ✅ | ⚙ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab self-monitoring: Sentry](#sentry) | Track errors generated by the GitLab instance | ⤓ | ❌ | ❌ | ✅ | ⤓ | ⤓ | CE & EE | +| [GitLab self-monitoring: Sentry](#sentry) | Track errors generated by the GitLab instance | ⤓ | ⤓ | ❌ | ✅ | ⤓ | ⤓ | CE & EE | | [GitLab Shell](#gitlab-shell) | Handles `git` over SSH sessions | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | | [GitLab Workhorse](#gitlab-workhorse) | Smart reverse proxy, handles large HTTP requests | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | -| [Inbound email (SMTP)](#inbound-email) | Receive messages to update issues | ⤓ | ⤓ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | +| [Inbound email (SMTP)](#inbound-email) | Receive messages to update issues | ⤓ | ⚙ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | | [Jaeger integration](#jaeger) | Distributed tracing for deployed apps | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | EE Only | | [LDAP Authentication](#ldap-authentication) | Authenticate users against centralized LDAP directory | ⤓ | ⤓ | ⤓ | ❌ | ⤓ | ⤓ | CE & EE | | [Mattermost](#mattermost) | Open-source Slack alternative | ⚙ | ⤓ | ⤓ | ⤓ | ❌ | ❌ | CE & EE | | [MinIO](#minio) | Object storage service | ⤓ | ✅ | ✅ | ✅ | ❌ | ⚙ | CE & EE | | [NGINX](#nginx) | Routes requests to appropriate components, terminates SSL | ✅ | ✅ | ⚙ | ✅ | ⤓ | ❌ | CE & EE | | [Node Exporter](#node-exporter) | Prometheus endpoint with system metrics | ✅ | N/A | N/A | ✅ | ❌ | ❌ | CE & EE | -| [Outbound email (SMTP)](#outbound-email) | Send email messages to users | ⤓ | ⤓ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | +| [Outbound email (SMTP)](#outbound-email) | Send email messages to users | ⤓ | ⚙ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | | [PgBouncer Exporter](#pgbouncer-exporter) | Prometheus endpoint with PgBouncer metrics | ⚙ | ❌ | ❌ | ✅ | ❌ | ❌ | CE & EE | | [PgBouncer](#pgbouncer) | Database connection pooling, failover | ⚙ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | | [PostgreSQL Exporter](#postgresql-exporter) | Prometheus endpoint with PostgreSQL metrics | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | | [PostgreSQL](#postgresql) | Database | ✅ | ✅ | ✅ | ✅ | ⤓ | ✅ | CE & EE | -| [Praefect](#praefect) | A transparent proxy between any Git client and Gitaly storage nodes. | ✅ | ❌ | ❌ | ✅ | ⚙ | ✅ | CE & EE | +| [Praefect](#praefect) | A transparent proxy between any Git client and Gitaly storage nodes. | ✅ | ⚙ | ❌ | ✅ | ⚙ | ✅ | CE & EE | | [Redis Exporter](#redis-exporter) | Prometheus endpoint with Redis metrics | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | | [Redis](#redis) | Caching service | ✅ | ✅ | ✅ | ✅ | ⤓ | ✅ | CE & EE | | [Registry](#registry) | Container registry, allows pushing and pulling of images | ⚙ | ✅ | ✅ | ✅ | ⤓ | ⚙ | CE & EE | @@ -196,7 +196,7 @@ GitLab can be considered to have two layers from a process perspective: - Process: `alertmanager` - GitLab.com: [Monitoring of GitLab.com](https://about.gitlab.com/handbook/engineering/monitoring/) -[Alert manager](https://prometheus.io/docs/alerting/alertmanager/) is a tool provided by Prometheus that _"handles alerts sent by client applications such as the Prometheus server. It takes care of deduplicating, grouping, and routing them to the correct receiver integration such as email, PagerDuty, or OpsGenie. It also takes care of silencing and inhibition of alerts."_ You can read more in [issue #45740](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/45740) about what we will be alerting on. +[Alert manager](https://prometheus.io/docs/alerting/latest/alertmanager/) is a tool provided by Prometheus that _"handles alerts sent by client applications such as the Prometheus server. It takes care of deduplicating, grouping, and routing them to the correct receiver integration such as email, PagerDuty, or Opsgenie. It also takes care of silencing and inhibition of alerts."_ You can read more in [issue #45740](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/45740) about what we will be alerting on. #### Certificate management @@ -273,7 +273,7 @@ repository updates to secondary nodes. - Configuration: - [Omnibus](../administration/geo/replication/index.md#setup-instructions) - - [Charts](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/8) + - [Charts](https://docs.gitlab.com/charts/advanced/geo/) - [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/geo.md) - Layer: Core Service (Processor) @@ -349,7 +349,7 @@ GitLab CI/CD is the open-source continuous integration service included with Git - [Project page](https://github.com/grafana/grafana/blob/master/README.md) - Configuration: - [Omnibus](../administration/monitoring/performance/grafana_configuration.md) - - [Charts](https://github.com/helm/charts/tree/master/stable/grafana) + - [Charts](https://docs.gitlab.com/charts/charts/globals#configure-grafana-integration) - Layer: Monitoring - GitLab.com: [GitLab triage Grafana dashboard](https://dashboards.gitlab.com/d/RZmbBr7mk/gitlab-triage?refresh=30s) @@ -360,7 +360,7 @@ Grafana is an open source, feature rich metrics dashboard and graph editor for G - [Project page](https://github.com/jaegertracing/jaeger/blob/master/README.md) - Configuration: - [Omnibus](https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/4104) - - [Charts](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/1320) + - [Charts](https://docs.gitlab.com/charts/charts/globals#tracing) - [Source](../development/distributed_tracing.md#enabling-distributed-tracing) - [GDK](../development/distributed_tracing.md#using-jaeger-in-the-gitlab-development-kit) - Layer: Monitoring @@ -369,7 +369,7 @@ Grafana is an open source, feature rich metrics dashboard and graph editor for G Jaeger, inspired by Dapper and OpenZipkin, is a distributed tracing system. It can be used for monitoring microservices-based distributed systems. -For monitoring deployed apps, see [Jaeger tracing documentation](../user/project/operations/tracing.md) +For monitoring deployed apps, see [Jaeger tracing documentation](../operations/tracing.md) #### Logrotate @@ -458,7 +458,7 @@ Prometheus exporter for PgBouncer. Exports metrics at 9127/metrics. - [Project page](https://github.com/postgres/postgres/blob/master/README) - Configuration: - [Omnibus](https://docs.gitlab.com/omnibus/settings/database.html) - - [Charts](https://github.com/helm/charts/tree/master/stable/postgresql) + - [Charts](https://docs.gitlab.com/charts/installation/deployment.html#postgresql) - [Source](../install/installation.md#6-database) - Layer: Core Service (Data) - Process: `postgresql` @@ -471,7 +471,7 @@ GitLab packages the popular Database to provide storage for Application meta dat - [Project page](https://github.com/wrouesnel/postgres_exporter/blob/master/README.md) - Configuration: - [Omnibus](../administration/monitoring/prometheus/postgres_exporter.md) - - [Charts](https://github.com/helm/charts/tree/master/stable/postgresql) + - [Charts](https://docs.gitlab.com/charts/installation/deployment.html#postgresql) - Layer: Monitoring - Process: `postgres-exporter` - GitLab.com: [Monitoring of GitLab.com](https://about.gitlab.com/handbook/engineering/monitoring/) @@ -483,7 +483,7 @@ GitLab packages the popular Database to provide storage for Application meta dat - [Project page](https://github.com/prometheus/prometheus/blob/master/README.md) - Configuration: - [Omnibus](../administration/monitoring/prometheus/index.md) - - [Charts](https://github.com/helm/charts/tree/master/stable/prometheus) + - [Charts](https://docs.gitlab.com/charts/installation/deployment.html#prometheus) - Layer: Monitoring - Process: `prometheus` - GitLab.com: [Prometheus](../user/gitlab_com/index.md#prometheus) @@ -495,7 +495,7 @@ Prometheus is a time-series tool that helps GitLab administrators expose metrics - [Project page](https://github.com/antirez/redis/blob/unstable/README.md) - Configuration: - [Omnibus](https://docs.gitlab.com/omnibus/settings/redis.html) - - [Charts](https://docs.gitlab.com/charts/charts/redis/) + - [Charts](https://docs.gitlab.com/charts/installation/deployment.html#redis) - [Source](../install/installation.md#7-redis) - Layer: Core Service (Data) - Process: `redis` @@ -512,7 +512,7 @@ Redis is packaged to provide a place to store: - [Project page](https://github.com/oliver006/redis_exporter/blob/master/README.md) - Configuration: - [Omnibus](../administration/monitoring/prometheus/redis_exporter.md) - - [Charts](https://docs.gitlab.com/charts/charts/redis/) + - [Charts](https://docs.gitlab.com/charts/installation/deployment.html#redis) - Layer: Monitoring - Process: `redis-exporter` - GitLab.com: [Monitoring of GitLab.com](https://about.gitlab.com/handbook/engineering/monitoring/) @@ -545,7 +545,7 @@ An external registry can also be configured to use GitLab as an auth endpoint. - [Project page](https://github.com/getsentry/sentry/) - Configuration: - [Omnibus](https://docs.gitlab.com/omnibus/settings/configuration.html#error-reporting-and-logging-with-sentry) - - [Charts](https://gitlab.com/gitlab-org/charts/gitlab/-/issues/1319) + - [Charts](https://docs.gitlab.com/charts/charts/globals#sentry-settings) - [Source](https://gitlab.com/gitlab-org/gitlab/blob/master/config/gitlab.yml.example) - [GDK](https://gitlab.com/gitlab-org/gitlab/blob/master/config/gitlab.yml.example) - Layer: Monitoring @@ -567,7 +567,7 @@ For monitoring deployed apps, see the [Sentry integration docs](../user/project/ - [GDK](https://gitlab.com/gitlab-org/gitlab/blob/master/config/gitlab.yml.example) - Layer: Core Service (Processor) - Process: `sidekiq` -- GitLab.com: [Sidekiq](../user/gitlab_com/index.md#Sidekiq) +- GitLab.com: [Sidekiq](../user/gitlab_com/index.md#sidekiq) Sidekiq is a Ruby background job processor that pulls jobs from the Redis queue and processes them. Background jobs allow GitLab to provide a faster request/response cycle by moving work into the background. @@ -576,7 +576,7 @@ Sidekiq is a Ruby background job processor that pulls jobs from the Redis queue - [Project page](https://gitlab.com/gitlab-org/gitlab/blob/master/README.md) - Configuration: - [Omnibus](https://docs.gitlab.com/omnibus/settings/unicorn.html) - - [Charts](https://docs.gitlab.com/charts/charts/gitlab/unicorn/) + - [Charts](https://docs.gitlab.com/charts/charts/gitlab/webservice/) - [Source](../install/installation.md#configure-it) - [GDK](https://gitlab.com/gitlab-org/gitlab/blob/master/config/gitlab.yml.example) - Layer: Core Service (Processor) @@ -773,7 +773,7 @@ response back to the user directly. When referring to `~git` in the pictures it means the home directory of the Git user which is typically `/home/git`. -GitLab is primarily installed within the `/home/git` user home directory as `git` user. Within the home directory is where the gitlabhq server software resides as well as the repositories (though the repository location is configurable). +GitLab is primarily installed within the `/home/git` user home directory as `git` user. Within the home directory is where the GitLab server software resides as well as the repositories (though the repository location is configurable). The bare repositories are located in `/home/git/repositories`. GitLab is a Ruby on rails application so the particulars of the inner workings can be learned by studying how a Ruby on rails application works. @@ -849,7 +849,7 @@ Usage: /etc/init.d/postgresql {start|stop|restart|reload|force-reload|status} [v ### Log locations of the services -gitlabhq (includes Unicorn and Sidekiq logs): +GitLab (includes Unicorn and Sidekiq logs): - `/home/git/gitlab/log/` contains `application.log`, `production.log`, `sidekiq.log`, `unicorn.stdout.log`, `git_json.log` and `unicorn.stderr.log` normally. diff --git a/doc/development/changelog.md b/doc/development/changelog.md index 5a8e05f888c..00a0573a8ba 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -14,12 +14,12 @@ following format: --- title: "Change[log]s" merge_request: 1972 -author: Black Sabbath +author: Black Sabbath @bsabbath type: added ``` The `merge_request` value is a reference to a merge request that adds this -entry, and the `author` key is used to give attribution to community +entry, and the `author` key (format: `<full name> <GitLab username>`) is used to give attribution to community contributors. **Both are optional**. The `type` field maps the category of the change, valid options are: added, fixed, changed, deprecated, removed, security, performance, other. **Type field is mandatory**. @@ -42,10 +42,9 @@ the `author` field. GitLab team members **should not**. Example: "Fixed a typo on the search results page." - Any docs-only changes **should not** have a changelog entry. - Any change behind a feature flag **should not** have a changelog entry - unless - the feature flag has been defaulted to true. The entry should be added - [in the merge request removing the feature flags](feature_flags/development.md). - If the change includes a database migration (regular, post, or data migration), - there should be a changelog entry for the migration change. + the feature flag has been defaulted to true. +- 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 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 60056c2f65c..a3a4f1d7adc 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -14,9 +14,11 @@ tasks such as: To request access to Chatops on GitLab.com: 1. Log into <https://ops.gitlab.net/users/sign_in> **using the same username** as for GitLab.com (you may have to rename it). + 1. You could also use the "Sign in with" Google button to sign in, with your GitLab.com email address. 1. Ask in the [#production](https://gitlab.slack.com/messages/production) channel for an existing member to add you to the `chatops` project in Ops. They can do it by running `/chatops run member add <username> gitlab-com/chatops --ops` command in that channel. -NOTE: **Note:** If you had to change your username for GitLab.com on the first step, make sure [to reflect this information](https://gitlab.com/gitlab-com/www-gitlab-com#adding-yourself-to-the-team-page) on [the team page](https://about.gitlab.com/company/team/). +NOTE: **Note:** +If you had to change your username for GitLab.com on the first step, make sure [to reflect this information](https://gitlab.com/gitlab-com/www-gitlab-com#adding-yourself-to-the-team-page) on [the team page](https://about.gitlab.com/company/team/). ## See also diff --git a/doc/development/cicd/img/ci_template_selection_v13_1.png b/doc/development/cicd/img/ci_template_selection_v13_1.png Binary files differnew file mode 100644 index 00000000000..af9f6dd1a90 --- /dev/null +++ b/doc/development/cicd/img/ci_template_selection_v13_1.png diff --git a/doc/development/cicd/index.md b/doc/development/cicd/index.md index 42d4b494544..e0cca00fd69 100644 --- a/doc/development/cicd/index.md +++ b/doc/development/cicd/index.md @@ -2,6 +2,8 @@ Development guides that are specific to CI/CD are listed here. +If you are creating new CI/CD templates, please read [the development guide for GitLab CI/CD templates](templates.md). + ## CI Architecture overview The following is a simplified diagram of the CI architecture. Some details are left out in order to focus on @@ -90,7 +92,8 @@ A job with the `created` state won't be seen by the Runner yet. To make it possi When the Runner is connected, it requests the next `pending` job to run by polling the server continuously. -NOTE: **Note:** API endpoints used by the Runner to interact with GitLab are defined in [`lib/api/runner.rb`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/runner.rb) +NOTE: **Note:** +API endpoints used by the Runner to interact with GitLab are defined in [`lib/api/runner.rb`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/runner.rb) After the server receives the request it selects a `pending` job based on the [`Ci::RegisterJobService` algorithm](#ciregisterjobservice), then assigns and sends the job to the Runner. @@ -124,7 +127,8 @@ There are 3 top level queries that this service uses to gather the majority of t This list of jobs is then filtered further by matching tags between job and Runner tags. -NOTE: **Note:** If a job contains tags, the Runner will not pick the job if it does not match **all** the tags. +NOTE: **Note:** +If a job contains tags, the Runner will not pick the job if it does not match **all** the tags. The Runner may have more tags than defined for the job, but not vice-versa. Finally if the Runner can only pick jobs that are tagged, all untagged jobs are filtered out. diff --git a/doc/development/cicd/templates.md b/doc/development/cicd/templates.md new file mode 100644 index 00000000000..904e7adffe2 --- /dev/null +++ b/doc/development/cicd/templates.md @@ -0,0 +1,66 @@ +# Development guide for GitLab CI/CD templates + +This document explains how to develop [GitLab CI/CD templates](../../ci/examples/README.md). + +## Place the template file in a relevant directory + +All template files reside in the `lib/gitlab/ci/templates` directory, and are categorized by the following sub-directories: + +| Sub-directroy | Content | [Selectable in UI](#make-sure-the-new-template-can-be-selected-in-ui) | +|---------------|--------------------------------------------------------------|-----------------------------------------------------------------------| +| `/Jobs/*` | Auto DevOps related jobs | Yes | +| `/Pages/*` | Static site generators for GitLab Pages (for example Jekyll) | Yes | +| `/Security/*` | Security related jobs | Yes | +| `/Verify/*` | Verify/testing related jobs | Yes | +| `/Worklows/*` | Common uses of the `workflow:` keyword | No | +| `/*` (root) | General templates | Yes | + +## Criteria + +The file must follow the [`.gitlab-ci.yml` syntax](../../ci/yaml/README.md). +Verify it's valid by pasting it into the CI lint tool at `https://gitlab.com/gitlab-org/gitlab/-/ci/lint`. + +Also, all templates must be named with the `*.gitlab-ci.yml` suffix. + +### Backward compatibility + +A template might be dynamically included with the `include:template:` keyword. If +you make a change to an *existing* template, you must make sure that it won't break +CI/CD in existing projects. + +## Testing + +Each CI/CD template must be tested in order to make sure that it's safe to be published. + +### Manual QA + +It's always good practice to test the template in a minimal demo project. +To do so, please follow the following steps: + +1. Create a public sample project on <https://gitlab.com>. +1. Add a `.gitlab-ci.yml` to the project with the proposed template. +1. Run pipelines and make sure that everything runs properly, in all possible cases + (merge request pipelines, schedules, and so on). +1. Link to the project in the description of the merge request that is adding a new template. + +This is useful information for reviewers to make sure the template is safe to be merged. + +### Make sure the new template can be selected in UI + +Templates located under some directories are also [selectable in the **New file** UI](#place-the-template-file-in-a-relevant-directory). +When you add a template into one of those directories, make sure that it correctly appears in the dropdown: + + + +### Write an RSpec test + +You should write an RSpec test to make sure that pipeline jobs will be generated correctly: + +1. Add a test file at `spec/lib/gitlab/ci/templates/<template-category>/<template-name>_spec.rb` +1. Test that pipeline jobs are properly created via `Ci::CreatePipelineService`. + +## 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. +If you're unsure if it's secure or not, you need to ask security experts for cross-validation. diff --git a/doc/development/code_intelligence/index.md b/doc/development/code_intelligence/index.md new file mode 100644 index 00000000000..bd11f0bff79 --- /dev/null +++ b/doc/development/code_intelligence/index.md @@ -0,0 +1,110 @@ +# Code Intelligence + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/1576) in GitLab 13.1. + +This document describes the design behind [Code Intelligence](../../user/project/code_intelligence.md). + +GitLab's built-in Code Intelligence is powered by +[LSIF](https://lsif.dev) and comes down to generating an LSIF document for a +project in a CI job, processing the data, uploading it as a CI artifact and +displaying this information for the files in the project. + +Here is a sequence diagram for uploading an LSIF artifact: + +```mermaid +sequenceDiagram + participant Runner + participant Workhorse + participant Rails + participant Object Storage + + Runner->>+Workhorse: POST /v4/jobs/:id/artifacts + Workhorse->>+Rails: POST /:id/artifacts/authorize + Rails-->>-Workhorse: Respond with ProcessLsif header + Note right of Workhorse: Process LSIF file + Workhorse->>+Object Storage: Put file + Object Storage-->>-Workhorse: request results + Workhorse->>+Rails: POST /:id/artifacts + Rails-->>-Workhorse: request results + Workhorse-->>-Runner: request results +``` + +1. The CI/CD job generates a document in an LSIF format (usually `dump.lsif`) using [an + indexer](https://lsif.dev) for the language of a project. The format + [describes](https://github.com/sourcegraph/sourcegraph/blob/master/doc/user/code_intelligence/writing_an_indexer.md) + interactions between a method or function and its definition(s) or references. The + document is marked to be stored as an LSIF report artifact. + +1. After receiving a request for storing the artifact, Workhorse asks + GitLab Rails to authorize the upload. + +1. GitLab Rails validates whether the artifact can be uploaded and sends + `ProcessLsif: true` header if the lsif artifact can be processed. + +1. Workhorse reads the LSIF document line by line and generates code intelligence + data for each file in the project. The output is a zipped directory of JSON + files which imitates the structure of the project: + + Project: + + ```code + app + controllers + application_controller.rb + models + application.rb + ``` + + Generated data: + + ```code + app + controllers + application_controller.rb.json + models + application.rb.json + ``` + +1. The zipped directory is stored as a ZIP artifact. Workhorse replaces the + original LSIF document with a set of JSON files in the ZIP artifact and + generates metadata for it. The metadata makes it possible to view a single + file in a ZIP file without unpacking or loading the whole file. That allows us + to access code intelligence data for a single file. + +1. When a file is viewed in the GitLab application, frontend fetches code + intelligence data for the file directly from the object storage. The file + contains information about code units in the file. For example: + + ```json + [ + { + "definition_path": "cmd/check/main.go#L4", + "hover": [ + { + "language": "go", + "tokens": [ + [ + { + "class": "kn", + "value": "package" + }, + { + "value": " " + }, + { + "class": "s", + "value": "\"fmt\"" + } + ] + ] + }, + { + "value": "Package fmt implements formatted I/O with functions analogous to C's printf and scanf. The format 'verbs' are derived from C's but are simpler. \n\n### hdr-PrintingPrinting\nThe verbs: \n\nGeneral: \n\n```\n%v\tthe value in a default format\n\twhen printing st..." + } + ], + "start_char": 2, + "start_line": 33 + } + ... + ] + ``` diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 9c6cb1d0237..fd53ce79534 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -95,17 +95,16 @@ with [domain expertise](#domain-experts). **approved by a [Distribution team member](https://about.gitlab.com/company/team/)**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/development/enablement/distribution/#how-to-work-with-distribution) for more details. 1. If your merge request includes documentation changes, it must be **approved by a [Technical writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers)**, based on - the appropriate [product category](https://about.gitlab.com/handbook/product/categories/). -1. If your merge request includes Quality and non-Quality-related changes (*3*), it must be **approved + the appropriate [product category](https://about.gitlab.com/handbook/product/product-categories/). +1. If your merge request includes end-to-end **and** non-end-to-end changes (*3*), it must be **approved by a [Software Engineer in Test](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors)**. -1. If your merge request includes _only_ Quality-related changes (*3*), it must be **approved - by a [Quality maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_qa)**. +1. If your merge request only includes end-to-end changes (*3*) **or** if the MR author is a [Software Engineer in Test](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors), it must be **approved by a [Quality maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_qa)** - (*1*): Please note that specs other than JavaScript specs are considered backend code. - (*2*): We encourage you to seek guidance from a database maintainer if your merge request is potentially introducing expensive queries. It is most efficient to comment on the line of code in question with the SQL queries so they can give their advice. -- (*3*): Quality-related changes include all files within the `qa` directory. +- (*3*): End-to-end changes include all files within the `qa` directory. #### Security requirements @@ -386,9 +385,12 @@ author. - Learning how to find the right balance takes time; that is why we have reviewers that become maintainers after some time spent on reviewing merge requests. -- Finding bugs and improving code style is important, but thinking about good - design is important as well. Building abstractions and good design is what - makes it possible to hide complexity and makes future changes easier. +- Finding bugs is important, but thinking about good design is important as + well. Building abstractions and good design is what makes it possible to hide + complexity and makes future changes easier. +- Enforcing and improving [code style](contributing/style_guides.md) should be primarily done through + [automation](https://about.gitlab.com/handbook/values/#cleanup-over-sign-off) + instead of review comments. - Asking the author to change the design sometimes means the complete rewrite of the contributed code. It's usually a good idea to ask another maintainer or reviewer before doing it, but have the courage to do it when you believe it is diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index f70299cbfc2..9feaa485bd2 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -81,7 +81,7 @@ already reserved for category labels). The descriptions on the [labels page](https://gitlab.com/groups/gitlab-org/-/labels) explain what falls under each type label. -The GitLab handbook documents [when something is a bug and when it is a feature request](https://about.gitlab.com/handbook/product/product-management/process/feature-or-bug.html). +The GitLab handbook documents [when something is a bug](https://about.gitlab.com/handbook/product/product-processes/#bug-issues) and [when it is a feature request](https://about.gitlab.com/handbook/product/product-processes/#feature-issues). ### Facet labels @@ -104,7 +104,7 @@ Following is a non-exhaustive list of facet labels: ### Stage labels -Stage labels specify which [stage](https://about.gitlab.com/handbook/product/categories/#hierarchy) the issue belongs to. +Stage labels specify which [stage](https://about.gitlab.com/handbook/product/product-categories/#hierarchy) the issue belongs to. #### Naming and color convention @@ -148,7 +148,7 @@ The current group labels can be found by [searching the labels list for `group:: These labels are [scoped labels](../../user/project/labels.md#scoped-labels-premium) and thus are mutually exclusive. -You can find the groups listed in the [Product Stages, Groups, and Categories](https://about.gitlab.com/handbook/product/categories/) page. +You can find the groups listed in the [Product Stages, Groups, and Categories](https://about.gitlab.com/handbook/product/product-categories/) page. We use the term group to map down product requirements from our product stages. As a team needs some way to collect the work their members are planning to be assigned to, we use the `~group::` labels to do so. @@ -167,7 +167,7 @@ Please read [Stage and Group labels in Throughput](https://about.gitlab.com/hand ### Category labels From the handbook's -[Product stages, groups, and categories](https://about.gitlab.com/handbook/product/categories/#hierarchy) +[Product stages, groups, and categories](https://about.gitlab.com/handbook/product/product-categories/#hierarchy) page: > Categories are high-level capabilities that may be a standalone product at @@ -199,7 +199,7 @@ in <https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/categories.yml ### Feature labels From the handbook's -[Product stages, groups, and categories](https://about.gitlab.com/handbook/product/categories/#hierarchy) +[Product stages, groups, and categories](https://about.gitlab.com/handbook/product/product-categories/#hierarchy) page: > Features: Small, discrete functionalities. e.g. Issue weights. Some common @@ -312,22 +312,13 @@ We want to avoid a situation when a contributor picks an because we realize that it does not fit our vision, or we want to solve it in a different way. -We add the ~"Accepting merge requests" label to: +We automatically add the ~"Accepting merge requests" label to issues +that match the [triage policy](https://about.gitlab.com/handbook/engineering/quality/triage-operations/#accepting-merge-requests). -- Low priority ~bug issues (i.e. we do not add it to the bugs that we want to - solve in the ~"Next Patch Release") -- Small ~feature -- Small ~"technical debt" issues - -After adding the ~"Accepting merge requests" label, we try to estimate the -[weight](#issue-weight) of the issue. We use issue weight to let contributors -know how difficult the issue is. Additionally: - -- We advertise [`Accepting merge requests` issues with weight < 5](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight) - as suitable for people that have never contributed to GitLab before on the - [Up For Grabs campaign](https://up-for-grabs.net/#/) -- We encourage people that have never contributed to any open source project to - look for [`Accepting merge requests` issues with a weight of 1](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight&weight=1) +We recommend people that have never contributed to any open source project to +look for issues labeled `~"Accepting merge requests"` with a [weight of 1](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight&weight=1) or the `~"Good for 1st time contributors"` [label](https://gitlab.com/gitlab-org/gitlab/-/issues?scope=all&utf8=%E2%9C%93&state=opened&label_name[]=Good%20for%201st%20time%20contributors&assignee_id=None) attached to it. +More experienced contributors are very welcome to tackle +[any of them](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None). If you've decided that you would like to work on an issue, please @-mention the [appropriate product manager](https://about.gitlab.com/handbook/product/#who-to-talk-to-for-what) diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 14c6b0b1073..e5a8bdad7b0 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -119,8 +119,7 @@ document from the Kubernetes team also has some great points regarding this. When writing commit messages, please follow the guidelines below: - The commit subject must contain at least 3 words. -- The commit subject should ideally contain up to 50 characters, - and must not be longer than 72 characters. +- The commit subject must not be longer than 72 characters. - The commit subject must start with a capital letter. - The commit subject must not end with a period. - The commit subject and body must be separated by a blank line. diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 9e4870eadc4..ed254052180 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -56,6 +56,16 @@ Additionally, we have a dedicated [newlines style guide](../newlines_styleguide.md), as well as dedicated [test-specific style guides and best practices](../testing_guide/index.md). +### Creating new RuboCop cops + +Typically it is better for the linting rules to be enforced programmatically as it +reduces the aforementioned [bike-shedding](https://en.wiktionary.org/wiki/bikeshedding). + +To that end, we encourage creation of new RuboCop rules in the codebase. + +When creating a new cop that could be applied to multiple applications, we encourage you +to add it to our [GitLab Styles](https://gitlab.com/gitlab-org/gitlab-styles) gem. + ## Database migrations See the dedicated [Database Migrations Style Guide](../migration_style_guide.md). @@ -82,7 +92,7 @@ See the dedicated [Shell scripting standards and style guidelines](../shell_scri ## Markdown -We're following [Ciro Santilli's Markdown Style Guide](https://cirosantilli.com/markdown-style-guide). +We're following [Ciro Santilli's Markdown Style Guide](https://cirosantilli.com/markdown-style-guide/). ## Documentation diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index eda9e1e21cc..93434479846 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -128,10 +128,18 @@ Here is a (non-exhaustive) list of the kinds of things Danger has been used for at GitLab so far: - Coding style -- Database review workflow -- Documentation review workflow +- Database review +- Documentation review - Merge request metrics -- Reviewer roulette workflow +- Reviewer roulette. Reviewers and maintainers are chosen based on: + - Their roles (backend, frontend, database, etc). + - Their availability: + - No "OOO"/"PTO"/"Parental Leave" in their GitLab or Slack status. + - No `:red_circle:`/`:palm_tree:`/`:beach:`/`:beach_umbrella:`/`:beach_with_umbrella:` emojis in GitLab or Slack status. + - [Experimental] Their timezone: people for which the local hour is between + 6 AM and 2 PM are eligible to be picked. This is to ensure they have a good + chance to get to perform a review during their current work day. The experimentation is tracked in + [this issue](https://gitlab.com/gitlab-org/quality/team-tasks/-/issues/563) - Single codebase effort ## Limitations diff --git a/doc/development/database/add_foreign_key_to_existing_column.md b/doc/development/database/add_foreign_key_to_existing_column.md index b8817eddeec..1b41a52c95e 100644 --- a/doc/development/database/add_foreign_key_to_existing_column.md +++ b/doc/development/database/add_foreign_key_to_existing_column.md @@ -113,7 +113,8 @@ end Validating the foreign key will scan the whole table and make sure that each relation is correct. -NOTE: **Note:** When using [background migrations](../background_migrations.md), foreign key validation should happen in the next GitLab release. +NOTE: **Note:** +When using [background migrations](../background_migrations.md), foreign key validation should happen in the next GitLab release. Migration file for validating the foreign key: diff --git a/doc/development/database/database_reviewer_guidelines.md b/doc/development/database/database_reviewer_guidelines.md new file mode 100644 index 00000000000..894b1ea15f0 --- /dev/null +++ b/doc/development/database/database_reviewer_guidelines.md @@ -0,0 +1,95 @@ +# Database Reviewer Guidelines + +This page includes introductory material for new database reviewers. + +If you are interested in getting an application update reviewed, +check the [database review guidelines](../database_review.md). + +## Scope of work done by a database reviewer + +Database reviewers are domain experts who have substantial experience with databases, +`SQL`, and query performance optimization. + +A database review is required whenever an application update [touches the database](../database_review.md#general-process). + +The database reviewer is tasked with reviewing the database specific updates and +making sure that any queries or modifications will perform without issues +at the scale of GitLab.com. + +For more information on the database review process, check the [database review guidelines](../database_review.md). + +## How to apply for becoming a database reviewer + +Team members are encouraged to self-identify as database domain experts and add it to their [team profile](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/data/team.yml) + +```yaml + projects: + gitlab: + - reviewer database +``` + +Assign the MR which adds your expertise to the `team.yml` file to a database maintainer +or the [Database Team's Engineering Manager](https://about.gitlab.com/handbook/engineering/development/enablement/database/). + +Once the `team.yml` update is merged, the [Reviewer roulette](../code_review.md#reviewer-roulette) +may recommend you as a database reviewer. + +## Resources for database reviewers + +As a database reviewer, join the internal `#database` Slack channel and ask questions or discuss +database related issues with other database reviewers and maintainers. + +There is also an optional database office hours call held bi-weekly, alternating between +European/US and APAC friendly hours. You can join the office hours call and bring topics +that require a more in-depth discussion between the database reviewers and maintainers: + +- [Database Office Hours Agenda](https://docs.google.com/document/d/1wgfmVL30F8SdMg-9yY6Y8djPSxWNvKmhR5XmsvYX1EI/edit). +- [Youtube playlist with past recordings](https://www.youtube.com/playlist?list=PL05JrBw4t0Kp-kqXeiF7fF7cFYaKtdqXM). + +You should also join the [#database-labs](../understanding_explain_plans.md#database-lab) +Slack channel and get familiar with how to use Joe, the slackbot that provides developers +with their own clone of the production database. + +Understanding and efficiently using `EXPLAIN` plans is at the core of the database review process. +The following guides provide a quick introduction and links to follow on more advanced topics: + +- Guide on [understanding EXPLAIN plans](../understanding_explain_plans.md). +- [Explaining the unexplainable series in depesz](https://www.depesz.com/tag/unexplainable/). + +Finally, you can find various guides in the [Database guides](index.md) page that cover more specific +topics and use cases. The most frequently required during database reviewing are the following: + +- [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations. +- [What requires downtime?](../what_requires_downtime.md). +- [SQL guidelines](../sql.md) for working with SQL queries. + +## How to apply for becoming a database maintainer + +Once a database reviewer feels confident on switching to a database maintainer, +they can update their [team profile](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/data/team.yml) +to a `trainee_maintainer database`: + +```yaml + projects: + gitlab: + - trainee_maintainer database +``` + +The first step is to a create a [Trainee Database Maintainer Issue](https://gitlab.com/gitlab-com/www-gitlab-com/-/issues/new?issuable_template=trainee-database-maintainer). +Use and follow the process described in the 'Trainee database maintainer' template. + +Note that [trainee maintainers](https://about.gitlab.com/handbook/engineering/workflow/code-review/#trainee-maintainer) +are three times as likely to be picked by the [Danger bot](../dangerbot.md) as other reviewers. + +## What to do if you feel overwhelmed + +Similar to all types of reviews, [unblocking others is always a top priority](https://about.gitlab.com/handbook/values/#global-optimization). +Database reviewers are expected to [review assigned merge requests in a timely manner](../code_review.md#review-turnaround-time) +or let the author know as soon as possible and help them find another reviewer or maintainer. + +We are doing reviews to help the rest of the GitLab team and, at the same time, get exposed +to more use cases, get a lot of insights and hone our database and data management skills. + +If you are feeling overwhelmed, think you are at capacity, and are unable to accept any more +reviews until some have been completed, communicate this through your GitLab status by setting +the `:red_circle:` emoji and mentioning that you are at capacity in the status text. diff --git a/doc/development/database/index.md b/doc/development/database/index.md index 665af623059..9ea5b6fcaac 100644 --- a/doc/development/database/index.md +++ b/doc/development/database/index.md @@ -1,5 +1,13 @@ # Database guides +## Database Reviews + +- If you're creating a database MR for review, check out our [Database review guidelines](../database_review.md). + + It provides an introduction on database-related changes, migrations, and complex SQL queries. + +- If you're a database reviewer or want to become one, check out our [introduction to reviewing database changes](database_reviewer_guidelines.md). + ## Tooling - [Understanding EXPLAIN plans](../understanding_explain_plans.md) diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index 629aea5b121..6da0455f392 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -1,4 +1,4 @@ -# Database Debugging and Troubleshooting +# Troubleshooting and Debugging Database This section is to help give some copy-pasta you can use as a reference when you run into some head-banging database problems. diff --git a/doc/development/database_review.md b/doc/development/database_review.md index 5405a8808f0..967df411db5 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -19,6 +19,10 @@ A database review is required for: generally up to the author of a merge request to decide whether or not complex queries are being introduced and if they require a database review. +- Changes in usage data metrics that use `count` and `distinct_count`. + These metrics could have complex queries over large tables. + See the [Telemetry Guide](telemetry/usage_ping.md#implementing-usage-ping) + for implementation details. A database reviewer is expected to look out for obviously complex queries in the change and review those closer. If the author does not @@ -190,7 +194,8 @@ In general, migrations for a single deploy shouldn't take longer than 1 hour for GitLab.com. The following guidelines are not hard rules, they were estimated to keep migration timing to a minimum. -NOTE: **Note:** Keep in mind that all runtimes should be measured against GitLab.com. +NOTE: **Note:** +Keep in mind that all runtimes should be measured against GitLab.com. | Migration Type | Execution Time Recommended | Notes | |----|----|---| diff --git a/doc/development/distributed_tracing.md b/doc/development/distributed_tracing.md index 7fc33380aba..15b3b8ba755 100644 --- a/doc/development/distributed_tracing.md +++ b/doc/development/distributed_tracing.md @@ -66,7 +66,7 @@ GITLAB_TRACING=opentracing://<driver>?<param_name>=<param_value>&<param_name_2>= In this example, we have the following hypothetical values: - `driver`: the driver. [GitLab supports - `jaeger`](../user/project/operations/tracing.md). In future, other + `jaeger`](../operations/tracing.md). In future, other tracing implementations may also be supported. - `param_name`, `param_value`: these are driver specific configuration values. Configuration parameters for Jaeger are documented [further on in this diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index f845a6ec592..2ea26985fcf 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -26,7 +26,7 @@ The source of the documentation exists within the codebase of each GitLab applic | --- | --- | | [GitLab](https://gitlab.com/gitlab-org/gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab/tree/master/doc) | | [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner/) | [`/docs`](https://gitlab.com/gitlab-org/gitlab-runner/tree/master/docs) | -| [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab/tree/master/doc) | +| [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/omnibus-gitlab/tree/master/doc) | | [Charts](https://gitlab.com/gitlab-org/charts/gitlab) | [`/doc`](https://gitlab.com/gitlab-org/charts/gitlab/tree/master/doc) | Documentation issues and merge requests are part of their respective repositories and all have the label `Documentation`. @@ -70,7 +70,28 @@ See the [Structure](styleguide.md#structure) section of the [Documentation Style ## Metadata To provide additional directives or useful information, we add metadata in YAML -format to the beginning of each product documentation page. +format to the beginning of each product documentation page (YAML front matter). +All values are treated as strings and are only used for the +[docs website](site_architecture/index.md). + +### Stage and group metadata + +Each page should ideally have metadata related to the stage and group it +belongs to, as well as an information block as described below: + +- `stage`: The [Stage](https://about.gitlab.com/handbook/product/product-categories/#devops-stages) + to which the majority of the page's content belongs. +- `group`: The [Group](https://about.gitlab.com/company/team/structure/#product-groups) + to which the majority of the page's content belongs. +- `info`: The following line, which provides direction to contributors regarding + how to contact the Technical Writer associated with the page's Stage and + Group: + + ```plaintext + To determine the technical writer assigned to the Stage/Group + associated with this page, see + https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers + ``` For example, the following metadata would be at the beginning of a product documentation page whose content is primarily associated with the Audit Events @@ -84,24 +105,64 @@ info: To determine the technical writer assigned to the Stage/Group associated w --- ``` -The following list describes the YAML parameters in use: +### Page type metadata + +Originally discussed in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/1280), +each page should have a `type` metadata. It can be one or more of the following: + +- `index`: Index/overview pages. They serve as a list to other pages. Doesn't + necessarily mean the page should be named `index.md`. [Example page](../../install/README.md). +- `concepts`: What you need to know before using product. Informational, not + instructional. For example, abstract ideas, explain meaning or benefit, support + understanding of tasks. They are read for background information, for example + "Why X is important". [Example page](../../topics/autodevops/index.md). +- `howto`: Specific use case instructions. [Example page](../../ssh/README.md). +- `tutorial`: Learn a process/concept by doing. [Example page](../../gitlab-basics/start-using-git.md). +- `reference`: Covers what things are/do. Things like specific settings, facts + without too much explanation that are read for detailed information. + [Example page](../../ci/yaml/README.md). + +### Redirection metadata + +The following metadata should be added when a page is moved to another location: - `redirect_to`: The relative path and filename (with an `.md` extension) of the location to which visitors should be redirected for a moved page. [Learn more](#changing-document-location). -- `stage`: The [Stage](https://about.gitlab.com/handbook/product/categories/#devops-stages) - to which the majority of the page's content belongs. -- `group`: The [Group](https://about.gitlab.com/company/team/structure/#product-groups) - to which the majority of the page's content belongs. -- `info`: The following line, which provides direction to contributors regarding - how to contact the Technical Writer associated with the page's Stage and - Group: `To determine the technical writer assigned to the Stage/Group - associated with this page, see - https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers` - `disqus_identifier`: Identifier for Disqus commenting system. Used to keep comments with a page that's been moved to a new URL. [Learn more](#redirections-for-pages-with-disqus-comments). +### Comments metadata + +The [docs website](site_architecture/index.md) has comments (provided by Disqus) +enabled by default. In case you want to disable them (for example in index pages), +set it to `false`: + +```yaml +--- +comments: false +--- +``` + +### Additional page metadata + +Each page can have additional (optional) metadata (set in the +[default.html](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/fc3577921343173d589dfa43d837b4307e4e620f/layouts/default.html#L30-52) +Nanoc layout), which will be shown to the top of the page if defined: + +- `author`: The name of the author of a page, usually a tutorial. It requires `author_gitlab` in order to be shown. +- `author_gitlab`: The username of the author on GitLab.com. It requires `author` in order to be shown. +- `date`: The date the page was created, usually for tutorials. +- `article_type`: The type of article. Can be either `tutorial` or `user guide`. +- `level`: The level of complexity of a how-to or tutorial. Can be either `beginner`, + `advanced`, or `intermediate`. +- `last_updated`: The date in ISO format when the page was last updated. For example `2020-02-14`. +- `reading_time`: If you want to add an indication of the approximate reading + time of a page, you can set `reading_time` to `true`. This uses a simple + [algorithm](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/master/lib/helpers/reading_time.rb) + to calculate the reading time based on the number of words. + ## Changing document location Changing a document's location requires specific steps to ensure that @@ -226,9 +287,6 @@ it increases the work of the release managers. Every GitLab instance includes the documentation, which is available at `/help` (`https://gitlab.example.com/help`). For example, <https://gitlab.com/help>. -There are [plans](https://gitlab.com/groups/gitlab-org/-/epics/693) to end this -practice and instead link out from the GitLab application to <https://docs.gitlab.com> URLs. - The documentation available online on <https://docs.gitlab.com> is deployed every four hours from the `master` branch of GitLab, Omnibus, and Runner. Therefore, after a merge request gets merged, it will be available online on the same day. However, it will be shipped (and available on `/help`) within the milestone assigned @@ -492,122 +550,138 @@ The output should be similar to: Note that this requires you to either have the required lint tools installed on your machine, or a working Docker installation, in which case an image with these tools pre-installed will be used. -### Local linting +### Local linters To help adhere to the [documentation style guidelines](styleguide.md), and improve the content -added to documentation, consider locally installing and running documentation linters. This will -help you catch common issues before raising merge requests for review of documentation. - -Running the following locally allows you to match the checks in the build pipeline: +added to documentation, [install documentation linters](#install-linters) and +[integrate them with your code editor](#configure-editors). -- [markdownlint](#markdownlint). -- [Vale](#vale). +At GitLab, we mostly use: -NOTE: **Note:** -This list does not limit what other linters you can add to your local documentation writing toolchain. +- [markdownlint](#markdownlint) +- [Vale](#vale) #### markdownlint -[markdownlint](https://github.com/DavidAnson/markdownlint) checks that Markdown -syntax follows [certain rules](https://github.com/DavidAnson/markdownlint/blob/master/doc/Rules.md#rules), -and is used by the [`docs-lint` test](#testing) with a [configuration file](#markdownlint-configuration). -Our [Documentation Style Guide](styleguide.md#markdown) and [Markdown Guide](https://about.gitlab.com/handbook/markdown-guide/) -elaborate on which choices must be made when selecting Markdown syntax for GitLab -documentation. This tool helps catch deviations from those guidelines. - -markdownlint can be used [on the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--), -either on a single Markdown file or on all Markdown files in a project. For example, to run -markdownlint on all documentation in the [`gitlab` project](https://gitlab.com/gitlab-org/gitlab), -run the following commands from within your `gitlab` project root directory, which will -automatically detect the [`.markdownlint.json`](#markdownlint-configuration) configuration -file in the root of the project, and test all files in `/doc` and its subdirectories: - -```shell -markdownlint 'doc/**/*.md' -``` +[markdownlint](https://github.com/DavidAnson/markdownlint) checks that Markdown syntax follows +[certain rules](https://github.com/DavidAnson/markdownlint/blob/master/doc/Rules.md#rules), and is +used by the [`docs-lint` test](#testing). -If you wish to use a different configuration file, use the `-c` flag: +Our [Documentation Style Guide](styleguide.md#markdown) and +[Markdown Guide](https://about.gitlab.com/handbook/markdown-guide/) elaborate on which choices must +be made when selecting Markdown syntax for GitLab documentation. This tool helps catch deviations +from those guidelines. -```shell -markdownlint -c <config-file-name> 'doc/**/*.md' -``` +markdownlint configuration is found in the following projects: -##### Run markdownlint in an editor +- [`gitlab`](https://gitlab.com/gitlab-org/gitlab/blob/master/.markdownlint.json) +- [`gitlab-runner`](https://gitlab.com/gitlab-org/gitlab-runner/blob/master/.markdownlint.json) +- [`omnibus-gitlab`](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/.markdownlint.json) +- [`charts`](https://gitlab.com/gitlab-org/charts/gitlab/-/blob/master/.markdownlint.json) +- [`gitlab-development-kit`](https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/.markdownlint.json) -markdownlint can also be run as a linter within text editors using [plugins/extensions](https://github.com/DavidAnson/markdownlint#related), -such as: +This configuration is also used within build pipelines. -- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint) -- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) -- [Atom](https://atom.io/packages/linter-node-markdownlint) +You can use markdownlint: -It is best to use the [same configuration file](#markdownlint-configuration) as what -is in use in the four repositories that are the sources for <https://docs.gitlab.com>. Each -plugin/extension has different requirements regarding the configuration file, which -is explained in each editor's docs. +- [On the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--). +- [Within a code editor](#configure-editors). -##### markdownlint configuration +#### Vale -Each formatting issue that markdownlint checks has an associated -[rule](https://github.com/DavidAnson/markdownlint/blob/master/doc/Rules.md#rules). -These rules are configured in the `.markdownlint.json` files located in the root of -four repositories that are the sources for <https://docs.gitlab.com>: +[Vale](https://errata-ai.gitbook.io/vale/) is a grammar, style, and word usage linter for the +English language. Vale's configuration is stored in the +[`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/blob/master/.vale.ini) file located in the root +directory of projects. -- [`gitlab`](https://gitlab.com/gitlab-org/gitlab/blob/master/.markdownlint.json) -- [`gitlab-runner`](https://gitlab.com/gitlab-org/gitlab-runner/blob/master/.markdownlint.json) -- [`omnibus-gitlab`](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/.markdownlint.json) -- [`charts`](https://gitlab.com/charts/gitlab/blob/master/.markdownlint.json) +Vale supports creating [custom tests](https://errata-ai.github.io/vale/styles/) that extend any of +several types of checks, which we store in the `.linting/vale/styles/gitlab` directory within the +documentation directory of projects. -By default all rules are enabled, so the configuration file is used to disable unwanted -rules, and also to configure optional parameters for enabled rules as needed. You can -also check [the issue](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/64352) that -tracked the changes required to implement these rules, and details which rules were -on or off when markdownlint was enabled on the docs. +Vale configuration is found in the following projects: -#### Vale +- [`gitlab`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/doc/.vale/gitlab) +- [`gitlab-runner`](https://gitlab.com/gitlab-org/gitlab-runner/-/tree/master/docs/.vale/gitlab) +- [`omnibus-gitlab`](https://gitlab.com/gitlab-org/omnibus-gitlab/-/tree/master/doc/.vale/gitlab) +- [`charts`](https://gitlab.com/gitlab-org/charts/gitlab/-/tree/master/doc/.vale/gitlab) +- [`gitlab-development-kit`](https://gitlab.com/gitlab-org/gitlab-development-kit/-/tree/master/doc/.vale/gitlab) -[Vale](https://errata-ai.gitbook.io/vale/) is a grammar, style, and word usage linter -for the English language. Vale's configuration is stored in the -[`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/blob/master/.vale.ini) file -located in the root directory of the [GitLab repository](https://gitlab.com/gitlab-org/gitlab). +This configuration is also used within build pipelines. -Vale supports creating [custom tests](https://errata-ai.github.io/vale/styles/), -stored in the `doc/.linting/vale/styles/gitlab` directory, that extend any of -several types of checks. +You can use Vale: -To view linting suggestions locally, you must install Vale on your own machine, -and from GitLab's root directory (where `.vale.ini` is located), run: +- [On the command line](https://errata-ai.gitbook.io/vale/getting-started/usage). +- [Within a code editor](#configure-editors). -```shell -vale --glob='*.{md}' doc -``` +#### Install linters + +At a minimum, install [markdownlint](#markdownlint) and [Vale](#vale) to match the checks run in +build pipelines: -Vale's error-level test results [are displayed](#testing) in CI pipelines. +1. Install `markdownlint-cli`, using either: -##### Run Vale in an editor + - `npm`: + + ```shell + npm install -g markdownlint-cli + ``` + + - `yarn`: + + ```shell + yarn global add markdownlint-cli + ``` + + We recommend installing the version of `markdownlint-cli` currently used in the documentation + linting [Docker image](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/master/dockerfiles/Dockerfile.gitlab-docs-lint#L38). + +1. Install [`vale`](https://github.com/errata-ai/vale/releases). For example, to install using + `brew` for macOS, run: + + ```shell + brew install vale + ``` + + We recommend installing the version of Vale currently used in the documentation linting + [Docker image](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/master/dockerfiles/Dockerfile.gitlab-docs-lint#L16). + +In addition to using markdownlint and Vale at the command line, these tools can be +[integrated with your code editor](#configure-editors). + +#### Configure editors + +To configure markdownlint within your editor, install one of the following as appropriate: + +- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint) +- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) +- [Atom](https://atom.io/packages/linter-node-markdownlint) -You can run Vale as a linter within your text editor of choice, with: +To configure Vale within your editor, install one of the following as appropriate: - The Sublime Text [`SublimeLinter-contrib-vale` plugin](https://packagecontrol.io/packages/SublimeLinter-contrib-vale) - The Visual Studio Code [`testthedocs.vale` extension](https://marketplace.visualstudio.com/items?itemName=testthedocs.vale) We don't use [Vale Server](https://errata-ai.github.io/vale/#using-vale-with-a-text-editor-or-another-third-party-application). -##### Disable a Vale test +#### Disable Vale tests -You can disable a specific Vale linting rule or all Vale linting rules for any portion of a document: +You can disable a specific Vale linting rule or all Vale linting rules for any portion of a +document: -- To disable a specific rule, add a `<!-- vale gitlab.rulename = NO -->` tag - before the text, and a `<!-- vale gitlab.rulename = YES -->` tag after the text, - replacing `rulename` with the filename of a test in the [GitLab styles](https://gitlab.com/gitlab-org/gitlab/-/tree/master/doc/.linting/vale/styles/gitlab) directory. -- To disable all Vale linting rules, add a `<!-- vale off -->` tag before the text, - and a `<!-- vale on -->` tag after the text. +- To disable a specific rule, add a `<!-- vale gitlab.rulename = NO -->` tag before the text, and a + `<!-- vale gitlab.rulename = YES -->` tag after the text, replacing `rulename` with the filename + of a test in the + [GitLab styles](https://gitlab.com/gitlab-org/gitlab/-/tree/master/doc/.linting/vale/styles/gitlab) + directory. +- To disable all Vale linting rules, add a `<!-- vale off -->` tag before the text, and a + `<!-- vale on -->` tag after the text. -Whenever possible, exclude only the problematic rule and line(s). -In some cases, such as list items, you may need to disable linting for the entire -list until ["Ignore comments are not honored in a Markdown file"](https://github.com/errata-ai/vale/issues/175) is resolved. +Whenever possible, exclude only the problematic rule and line(s). In some cases, such as list items, +you may need to disable linting for the entire list until +[Vale issue #175](https://github.com/errata-ai/vale/issues/175) is resolved. -For more information, see [Vale's documentation](https://errata-ai.gitbook.io/vale/getting-started/markup#markup-based-configuration). +For more information, see +[Vale's documentation](https://errata-ai.gitbook.io/vale/getting-started/markup#markup-based-configuration). ## Danger Bot diff --git a/doc/development/documentation/site_architecture/global_nav.md b/doc/development/documentation/site_architecture/global_nav.md index 71020e6054e..2625fbe0415 100644 --- a/doc/development/documentation/site_architecture/global_nav.md +++ b/doc/development/documentation/site_architecture/global_nav.md @@ -16,6 +16,22 @@ Global navigation (the left-most pane in our three pane documentation) provides: - The ability to refine landing pages, so they don't have to do all the work of surfacing every page contained within the documentation. +## Quick start + +To add a topic to the global nav, go to the directory that contains +[navigation files](https://gitlab.com/gitlab-org/gitlab-docs/blob/master/content/_data/) +and edit the `yaml` file for your product area. You can copy an existing nav entry and +edit it to point to your topic. + +The files are: + +| File | Document | Location | +|-----------------------|--------------------------------------------------------------------|-------------------------------------------------------| +| `charts-nav.yaml` | GitLab cloud native Helm Chart | `https://docs.gitlab.com/charts/` | +| `default-nav.yaml` | GitLab Docs | `https://docs.gitlab.com/ee/` | +| `omnibus-nav.yaml` | Omnibus GitLab Docs | `https://docs.gitlab.com/omnibus/` | +| `runner-nav.yaml` | GitLab Runner Docs | `https://docs.gitlab.com/runner/` | + ## Adding new items All new pages need a new navigation item. Without a navigation, the page becomes "orphaned". That @@ -98,7 +114,7 @@ for clarity. To see the improvements planned, check the [global nav epic](https://gitlab.com/groups/gitlab-com/-/epics/21). -CAUTION: **Attention!** +NOTE: **Note:** **Do not** [add items](#adding-new-items) to the global nav without the consent of one of the technical writers. @@ -275,7 +291,7 @@ and the following syntax rules. an "information" icon on the nav to make the user aware that the feature is EE-only. -DANGER: **Important!** +CAUTION: **Caution:** All links present on the data file must end in `.html`, not `.md`. Do not start any relative link with a forward slash `/`. @@ -290,7 +306,7 @@ Examples: docs: - doc_title: Service Desk doc_url: 'user/project/service_desk.html' - ee_only: true + ee_only: false # note that the URL above ends in html and, as the # document is EE-only, the attribute ee_only is set to true. ``` diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index 942b202a3ec..60e6d4bcb13 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -111,7 +111,7 @@ located in the [Dockerfiles directory](https://gitlab.com/gitlab-org/gitlab-docs If you need to rebuild the Docker images immediately (must have maintainer level permissions): -CAUTION: **Caution** +CAUTION: **Caution:** If you change the dockerfile configuration and rebuild the images, you can break the master 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. @@ -152,9 +152,9 @@ Suppose we have the `content/_data/versions.yaml` file with the content: ```yaml versions: -- 10.6 -- 10.5 -- 10.4 + - 10.6 + - 10.5 + - 10.4 ``` We can then loop over the `versions` array with something like: diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index eadcedfaac0..4cfc57aa57b 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -20,7 +20,7 @@ Every feature or use case document should include the following content in the f with exceptions and details noted below and in the template included on this page. - **Title**: Top-level heading with the feature name, or a use case name, which would start with - a verb, like Configuring, Enabling, and so on. + a verb, like "Configure", "Enable", and so on. - **Introduction**: A couple sentences about the subject matter and what's to be found on this page. Describe what the feature or topic is, what it does, and in what context it should be used. There is no need to add a title called "Introduction" or "Overview," because people rarely @@ -47,10 +47,13 @@ When done, remove all of this commented-out text, except a commented-out Trouble which, if empty, can be left in place to encourage future use.--> --- description: "Short document description." # Up to ~200 chars long. They will be displayed in Google Search snippets. It may help to write the page intro first, and then reuse it here. +stage: "Add the stage name here, and remove the quotation marks" +group: "Add the group name here, and remove the quotation marks" +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/#designated-technical-writers --- # Feature Name or Use Case Name **[TIER]** (1) -<!--If writing about a use case, drop the tier, and start with a verb, e.g. 'Configuring', 'Implementing', + the goal/scenario--> +<!--If writing about a use case, drop the tier, and start with a verb, e.g. "Configure", "Implement", + the goal/scenario--> <!--For pages on newly introduced features, add the following line. If only some aspects of the feature have been introduced, specify what parts of the feature.--> > [Introduced](link_to_issue_or_mr) in GitLab (Tier) X.Y (2). @@ -99,12 +102,12 @@ Larger instruction sets may have subsections covering specific phases of the pro Where appropriate, provide examples of code or configuration files to better clarify intended usage. - Write a step-by-step guide, with no gaps between the steps. -- Include example code or configurations as part of the relevant step. Use appropriate markdown to [wrap code blocks with syntax highlighting](../../user/markdown.md#colored-code-and-syntax-highlighting). +- Include example code or configurations as part of the relevant step. Use appropriate Markdown to [wrap code blocks with syntax highlighting](../../user/markdown.md#colored-code-and-syntax-highlighting). - Start with an h2 (`##`), break complex steps into small steps using subheadings h3 > h4 > h5 > h6. _Never skip a hierarchy level, such as h2 > h4_, as it will break the TOC and may affect the breadcrumbs. - Use short and descriptive headings (up to ~50 chars). You can use one -single heading like `## Configuring X` for instructions when the feature +single heading like `## Configure X` for instructions when the feature is simple and the document is short. <!-- ## Troubleshooting diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 9a93dbf4f94..f2c90e71bd5 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -88,7 +88,7 @@ New information that would be useful toward the future usage or troubleshooting The more we reflexively add useful information to the docs, the more (and more successfully) the docs will be used to efficiently accomplish tasks and solve problems. -If you have questions when considering, authoring, or editing docs, ask the Technical Writing team on Slack in `#docs` or in GitLab by mentioning the writer for the applicable [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). Otherwise, forge ahead with your best effort. It does not need to be perfect; the team is happy to review and improve upon your content. Please review the [Documentation guidelines](index.md) before you begin your first documentation MR. +If you have questions when considering, authoring, or editing docs, ask the Technical Writing team on Slack in `#docs` or in GitLab by mentioning the writer for the applicable [DevOps stage](https://about.gitlab.com/handbook/product/product-categories/#devops-stages). Otherwise, forge ahead with your best effort. It does not need to be perfect; the team is happy to review and improve upon your content. Please review the [Documentation guidelines](index.md) before you begin your first documentation MR. Having a knowledge base in any form that is separate from the documentation would be against the docs-first methodology because the content would overlap with the documentation. @@ -325,9 +325,22 @@ tenses, words, and phrases: - Avoid using the word *currently* when talking about the product or its features. The documentation describes the product as it is, and not as it will be at some indeterminate point in the future. +- Avoid the using the word *scalability* with increasing GitLab's performance + for additional users. Using the words *scale* or *scaling* in other cases is + acceptable, but references to increasing GitLab's performance for additional + users should direct readers to the GitLab + [reference architectures](../../administration/reference_architectures/index.md) + page. +- Avoid all forms of the phrases *high availability* and *HA*, and instead + direct readers to the GitLab [reference architectures](../../administration/reference_architectures/index.md) + for information about configuring GitLab to have the performance needed for + additional users over time. - Don't use profanity or obscenities. Doing so may negatively affect other users and contributors, which is contrary to GitLab's value of - [diversity and inclusion](https://about.gitlab.com/handbook/values/#diversity-inclusion). + [Diversity, Inclusion, and Belonging](https://about.gitlab.com/handbook/values/#diversity-inclusion). +- Avoid the use of [racially-insensitive terminology or phrases](https://www.marketplace.org/2020/06/17/tech-companies-update-language-to-avoid-offensive-terms/). For example: + - Use *primary* and *secondary* for database and server relationships. + - Use *allowlist* and *denylist* to describe access control lists. ### Word usage clarifications @@ -662,7 +675,7 @@ For other punctuation rules, please refer to the - Avoid adding things that show ephemeral statuses. For example, if a feature is considered beta or experimental, put this information in a note, not in the heading. - When introducing a new document, be careful for the headings to be - grammatically and syntactically correct. Mention an [assigned technical writer (TW)](https://about.gitlab.com/handbook/product/categories/) + grammatically and syntactically correct. Mention an [assigned technical writer (TW)](https://about.gitlab.com/handbook/product/product-categories/) for review. This is to ensure that no document with wrong heading is going live without an audit, thus preventing dead links and redirection issues when @@ -750,6 +763,15 @@ _Internal_ refers to documentation in the same project. When linking to document separate projects (for example, linking to Omnibus docs from GitLab docs), you must use absolute URLs. +Do not use absolute URLs like `https://docs.gitlab.com/ee/index.html` to crosslink +to other docs within the same project. Use relative links to the file, like `../index.md`. (These are converted to HTML when the site is rendered.) + +Relative linking enables crosslinks to work: + +- in Review Apps, local previews, and `/help`. +- when working on the docs locally, so you can verify that they work as early as possible in the process. +- within the GitLab UI when browsing doc files in their respective repositories. For example, the links displayed at `https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/README.md`. + To link to internal documentation: - Use relative links to Markdown files in the same repository. @@ -778,7 +800,7 @@ To link to internal documentation: - `../../issues/tags.md` - `../../issues/tags.md#stages` -NOTE: **Note**: +NOTE: **Note:** Using the Markdown extension is necessary for the [`/help`](index.md#gitlab-help) section of GitLab. ### Links to external documentation @@ -839,19 +861,42 @@ To indicate the steps of navigation through the UI: ## Images +Images, including screenshots, can help a reader better understand a concept. +However, they can be hard to maintain, and should be used sparingly. + +Before including an image in the documentation, ensure it provides value to the reader. + +### Capture the image + +Use images to help the reader understand where they are in a process, or how they need to +interact with the application. + +When you take screenshots: + +- *Capture the most relevant area of the page.* Don't include unnecessary white + space or areas of the page that don't help illustrate your point. Also, don't + include the entire page if you don't have to, but also ensure the image + contains enough information to allow the user to determine where things are. +- *Be consistent.* Find a browser window size that works for you that also + displays all areas of the product, including the left navigation (usually > + 1200px wide). For consistency, use this browser window size for your + screenshots by installing a browser extension for setting a window to a + specific size (for example, + [Window Resizer](https://chrome.google.com/webstore/detail/window-resizer/kkelicaakdanhinjdeammmilcgefonfh/related?hl=en) + for Google Chrome). + +### Save the image + +- Save the image with a lowercase file name that is descriptive of the feature + or concept in the image. If the image is of the GitLab interface, append the + GitLab version to the file name, based on the following format: + `image_name_vX_Y.png`. For example, for a screenshot taken from the pipelines + page of GitLab 11.1, a valid name is `pipelines_v11_1.png`. If you're adding an + illustration that doesn't include parts of the user interface, add the release + number corresponding to the release the image was added to; for an MR added to + 11.1's milestone, a valid name for an illustration is `devops_diagram_v11_1.png`. - Place images in a separate directory named `img/` in the same directory where the `.md` document that you're working on is located. -- Images should have a specific, non-generic name that will - differentiate and describe them properly. -- For screenshots of GitLab software, append the GitLab version the screenshot was taken from to the - file name. Use the following format: `image_name_vX_Y.png`. -- For example, for a screenshot taken from the pipelines page of - GitLab 11.1, a valid name is `pipelines_v11_1.png`. If you're - adding an illustration that does not include parts of the UI, - add the release number corresponding to the release the image - was added to. Example, for an MR added to 11.1's milestone, - a valid name for an illustration is `devops_diagram_v11_1.png`. -- Keep all file names in lower case. - Consider using PNG images instead of JPEG. - [Compress all PNG images](#compress-images). - Compress gifs with <https://ezgif.com/optimize> or similar tool. @@ -860,15 +905,20 @@ To indicate the steps of navigation through the UI: - Max image size: 100KB (gifs included). - See also how to link and embed [videos](#videos) to illustrate the docs. -Inside the document: +### Add the image link to content + +The Markdown code for including an image in a document is: +`` + +The image description is the alt text for the rendered image on the docs site. +For accessibility and SEO, use [descriptions](https://webaim.org/techniques/alttext/) +that: + +- Are accurate, succinct, and unique. +- Don't use *image of …* or *graphic of…* to describe the image. -- The Markdown way of using an image inside a document is: - `` -- Always use a proper description for what the image is about. That way, when a - browser fails to show the image, this text will be used as an alternative - description. -- If a heading is placed right after an image, always add three dashes (`---`) - between the image and the heading. +Also, if a heading immediately follows an image, be sure to add three dashes +(`---`) between the image and the heading. ### Remove image shadow @@ -1133,8 +1183,8 @@ The following are examples of source Markdown for menu items with their publishe Whenever you need to call special attention to particular sentences, use the following markup for highlighting. -_Note that the alert boxes only work for one paragraph only. Multiple paragraphs, -lists, headers and so on, will not render correctly. For multiple lines, use blockquotes instead._ +Note that the alert boxes only work for one paragraph only. Multiple paragraphs, +lists, headers and so on, will not render correctly. For multiple lines, use [blockquotes](#blockquotes) instead. Alert boxes only render on the GitLab Docs site (<https://docs.gitlab.com>). Within GitLab itself, they will appear as plain Markdown text (like the examples @@ -1271,20 +1321,20 @@ The following are styles to follow when describing UI elements on a screen: ### Verbs for UI elements -The following are recommended verbs for specific uses. +The following are recommended verbs for specific uses with UI elements: -| Recommended | Used for | Alternatives | -|:------------|:---------------------------|:---------------------------| -| "click" | buttons, links, menu items | "hit", "press", "select" | -| "check" | checkboxes | "enable", "click", "press" | -| "select" | dropdowns | "pick" | -| "expand" | expandable sections | "open" | +| Recommended | Used for | Replaces | +|:--------------------|:---------------------------|:---------------------------| +| *click* | buttons, links, menu items | "hit", "press", "select" | +| *select* or *clear* | checkboxes | "enable", "click", "press" | +| *select* | dropdowns | "pick" | +| *expand* | expandable sections | "open" | ### Other Verbs -| Recommended | Used for | Alternatives | -|:------------|:--------------------------------|:-------------------| -| "go" | making a browser go to location | "navigate", "open" | +| Recommended | Used for | Replaces | +|:------------|:--------------------------------|:----------------------| +| *go to* | making a browser go to location | "navigate to", "open" | ## GitLab versions and tiers @@ -1296,6 +1346,11 @@ Tagged and released versions of GitLab documentation are available: The version introducing a new feature is added to the top of the topic in the documentation to provide a helpful link back to how the feature was developed. +TIP: **Tip:** +Whenever you have documentation related to the `gitlab.rb` file, you're working with a self-managed installation. +The section or page is therefore likely to apply only to self-managed instances. +If so, the relevant "`TIER` ONLY" [Product badge](#product-badges) should be included at the highest applicable heading level. + ### Text for documentation requiring version text - For features that need to declare the GitLab version that the feature was introduced. Text similar @@ -1327,6 +1382,22 @@ a helpful link back to how the feature was developed. > - [Moved](<link-to-issue>) to GitLab Core in 12.0. ``` +- If a feature is deprecated, include a link to a replacement (when available): + + ```markdown + > - [Deprecated](<link-to-issue>) in GitLab 11.3. Replaced by [meaningful text](<link-to-appropriate-documentation>). + ``` + + It's also acceptable to describe the replacement in surrounding text, if available. + + If the deprecation is not obvious in existing text, you may want to include a warning such as: + + ```markdown + CAUTION: **Warning:** + This feature was [deprecated](link-to-issue) in GitLab 12.3 + and replaced by [Feature name](link-to-feature-documentation). + ``` + NOTE: **Note:** Version text must be on its own line and surrounded by blank lines to render correctly. @@ -1389,7 +1460,7 @@ lines with an inserted line break. Splitting product or feature names across lines makes searching for these items more difficult, and can cause problems if names change. -For example, the followng Markdown content is *not* formatted correctly: +For example, the following Markdown content is *not* formatted correctly: ```markdown When entering a product or feature name that includes a space (such as GitLab @@ -1430,7 +1501,7 @@ For GitLab.com only tiers (when the feature is not available for self-managed in The tier should be ideally added to headers, so that the full badge will be displayed. However, it can be also mentioned from paragraphs, list items, and table cells. For these cases, -the tier mention will be represented by an orange question mark that will show the tiers on hover. +the tier mention will be represented by an orange info icon **(information)** that will show the tiers on hover. Use the lowest tier at the page level, even if higher-level tiers exist on the page. For example, you might have a page that is marked as Starter but a section badged as Premium. @@ -1542,6 +1613,9 @@ can facilitate this by making sure the troubleshooting content addresses: 1. How the user can confirm they have the problem. 1. Steps the user can take towards resolution of the problem. +If the contents of each category can be summarized in one line and a list of steps aren't required, consider setting up a +[table](#tables) with headers of *Problem* \| *Cause* \| *Solution* (or *Workaround* if the fix is temporary), or *Error message* \| *Solution*. + ## Feature flags Learn how to [document features deployed behind flags](feature_flags.md). @@ -1607,6 +1681,13 @@ and email address of a user. Don't use real user information in API calls: - **Names**: Use strings like `Example Username`. Alternatively, use diverse or non-gendered names with common surnames, such as `Sidney Jones`, `Zhang Wei`. or `Maria Garcia`. +### Fake URLs + +When including sample URLs in the documentation, use: + +- `example.com` when the domain name is generic. +- `gitlab.example.com` when referring to self-managed instances of GitLab. + ### Fake tokens There may be times where a token is needed to demonstrate an API call using diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index e22e96b6f06..ac544113cbd 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -512,12 +512,12 @@ do that, so we'll follow regular object-oriented practices that we define the interface first here. For example, suppose we have a few more optional parameters for EE. We can move the -parameters out of the `Grape::API` class to a helper module, so we can inject it +parameters out of the `Grape::API::Instance` class to a helper module, so we can inject it before it would be used in the class. ```ruby module API - class Projects < Grape::API + class Projects < Grape::API::Instance helpers Helpers::ProjectsHelpers end end @@ -578,7 +578,7 @@ class definition to make it easy and clear: ```ruby module API - class JobArtifacts < Grape::API + class JobArtifacts < Grape::API::Instance # EE::API::JobArtifacts would override the following helpers helpers do def authorize_download_artifacts! @@ -622,7 +622,7 @@ route. Something like this: ```ruby module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance helpers do # EE::API::MergeRequests would override the following helpers def update_merge_request_ee(merge_request) @@ -691,7 +691,7 @@ least argument. We would approach this as follows: ```ruby # api/merge_requests/parameters.rb module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance module Parameters def self.update_params_at_least_one_of %i[ @@ -707,7 +707,7 @@ API::MergeRequests::Parameters.prepend_if_ee('EE::API::MergeRequests::Parameters # api/merge_requests.rb module API - class MergeRequests < Grape::API + class MergeRequests < Grape::API::Instance params do at_least_one_of(*Parameters.update_params_at_least_one_of) end @@ -900,27 +900,79 @@ export default { </template> ``` -#### For JS code that is EE only, like props, computed properties, methods, etc, we will keep the current approach +#### For JS code that is EE only, like props, computed properties, methods, etc -- Since we [can't async load a mixin](https://github.com/vuejs/vue-loader/issues/418#issuecomment-254032223) we will use the [`ee_else_ce`](../development/ee_features.md#javascript-code-in-assetsjavascripts) alias we already have for webpack. - - This means all the EE specific props, computed properties, methods, etc that are EE only should be in a mixin in the `ee/` folder and we need to create a CE counterpart of the mixin +- Please do not use mixins unless ABSOLUTELY NECESSARY. Please try to find an alternative pattern. -##### Example +##### Reccomended alternative approach (named/scoped slots) -```javascript -import mixin from 'ee_else_ce/path/mixin'; +- We can use slots and/or scoped slots to achieve the same thing as we did with mixins. If you only need an EE component there is no need to create the CE component. + +1. First, we have a CE component that can render a slot incase we need EE template and functionality to be decorated on top of the CE base. + +```vue +// ./ce/my_component.vue + +<script> +export default { + props: { + tooltipDefaultText: { + type: String, + }, + }, + computed: { + tooltipText() { + return this.tooltipDefaultText || "5 issues please"; + } + }, +} +</script> + +<template> + <span v-gl-tooltip :title="tooltipText" class="ce-text">Community Edition Only Text</span> + <slot name="ee-specific-component"> +</template> +``` + +1. Next, we render the EE component, and inside of the EE component we render the CE component and add additional content in the slot. -{ - mixins: [mixin] +```vue +// ./ee/my_component.vue + +<script> +export default { + computed: { + tooltipText() { + if (this.weight) { + return "5 issues with weight 10"; + } + } + }, + methods: { + submit() { + // do something. + } + }, } +</script> + +<template> + <my-component :tooltipDefaultText="tooltipText"> + <template #ee-specific-component> + <span class="some-ee-specific">EE Specific Value</span> + <button @click="submit">Click Me</button> + </template> + </my-component> +</template> ``` -- Computed Properties/methods and getters only used in the child import still need a counterpart in CE +1. Finally, wherever the component is needed we can require it like so + +`import MyComponent from 'ee_else_ce/path/my_component'.vue` -- For store modules, we will need a CE counterpart too. -- You can see an MR with an example [here](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/9762) +- this way the correct component will be included for either the ce or ee implementation -#### `template` tag +**For EE components that need different results for the same computed values, we can pass in props to the CE wrapper as seen in the example.** - **EE Child components** - Since we are using the async loading to check which component to load, we'd still use the component's name, check [this example](#child-component-only-used-in-ee). diff --git a/doc/development/elasticsearch.md b/doc/development/elasticsearch.md index 9f54386f1af..90debab3b5c 100644 --- a/doc/development/elasticsearch.md +++ b/doc/development/elasticsearch.md @@ -60,7 +60,7 @@ The `whitespace` tokenizer was selected in order to have more control over how t Please see the `code` filter for an explanation on how tokens are split. -NOTE: **Known Issues**: +NOTE: **Note:** Currently the [Elasticsearch code_analyzer doesn't account for all code cases](../integration/elasticsearch.md#known-issues). #### `code_search_analyzer` @@ -111,11 +111,8 @@ Patterns: - `'"((?:\\"|[^"]|\\")*)"'`: captures terms inside quotes, removing the quotes - `"'((?:\\'|[^']|\\')*)'"`: same as above, for single-quotes - `'\.([^.]+)(?=\.|\s|\Z)'`: separate terms with periods in-between -- `'\/?([^\/]+)(?=\/|\b)'`: separate path terms `like/this/one` - -#### `edgeNGram_filter` - -Uses an [Edge NGram token filter](https://www.elastic.co/guide/en/elasticsearch/reference/5.5/analysis-edgengram-tokenfilter.html) to allow inputs with only parts of a token to find the token. For example it would turn `glasses` into permutations starting with `gl` and ending with `glasses`, which would allow a search for "`glass`" to find the original token `glasses` +- `'([\p{L}_.-]+)'`: some common chars in file names to keep the whole filename intact (eg. `my_file-ñame.txt`) +- `'([\p{L}\d_]+)'`: letters, numbers and underscores are the most common tokens in programming. Always capture them greedily regardless of context. ## Gotchas @@ -160,7 +157,8 @@ The global configurations per version are now in the `Elastic::(Version)::Config ### Creating new version of schema -NOTE: **Note:** this is not applicable yet as multiple indices functionality is not fully implemented. +NOTE: **Note:** +This is not applicable yet as multiple indices functionality is not fully implemented. Folders like `ee/lib/elastic/v12p1` contain snapshots of search logic from different versions. To keep a continuous Git history, the latest version lives under `ee/lib/elastic/latest`, but its classes are aliased under an actual version (e.g. `ee/lib/elastic/v12p3`). When referencing these classes, never use the `Latest` namespace directly, but use the actual version (e.g. `V12p3`). @@ -222,6 +220,16 @@ be used both locally in development and on any deployed GitLab instance to diagnose poor search performance. This will show the exact queries being made, which is useful to diagnose why a search might be slow. +### Correlation ID and X-Opaque-Id + +Our [correlation +ID](./distributed_tracing.md#developer-guidelines-for-working-with-correlation-ids) +is forwarded by all requests from Rails to Elasticsearch as the +[`X-Opaque-Id`](https://www.elastic.co/guide/en/elasticsearch/reference/current/tasks.html#_identifying_running_tasks) +header which allows us to track any +[tasks](https://www.elastic.co/guide/en/elasticsearch/reference/current/tasks.html) +in the cluster back the request in GitLab. + ## Troubleshooting ### Getting `flood stage disk watermark [95%] exceeded` diff --git a/doc/development/emails.md b/doc/development/emails.md index 2477a51f78f..cf7f49ee834 100644 --- a/doc/development/emails.md +++ b/doc/development/emails.md @@ -115,7 +115,7 @@ Examples of valid email keys: - `1234567890abcdef1234567890abcdef-unsubscribe` (unsubscribe from a conversation) - `1234567890abcdef1234567890abcdef` (reply to a conversation) -Please note that the action `-issue-` is used in GitLab Premium as the handler for the Service Desk feature. +Please note that the action `-issue-` is used in GitLab as the handler for the Service Desk feature. ### Legacy format @@ -127,7 +127,7 @@ These are the only valid legacy formats for an email handler: - `namespace` - `namespace+action` -Please note that `path/to/project` is used in GitLab Premium as handler for the Service Desk feature. +Please note that `path/to/project` is used in GitLab as the handler for the Service Desk feature. --- diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md index f0e05139cba..b1a15c7749f 100644 --- a/doc/development/experiment_guide/index.md +++ b/doc/development/experiment_guide/index.md @@ -1,12 +1,12 @@ # Experiment Guide -Experiments will be conducted by teams from the [Growth Section](https://about.gitlab.com/handbook/engineering/development/growth/) and are not tied to releases, because they will primarily target GitLab.com. +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 will primarily target GitLab.com. Experiments will be run as an A/B test and will be behind a feature flag to turn the test on or off. Based on the data the experiment generates, the team will decide if the experiment had a positive impact and will be the new default or rolled back. -## Follow-up issue +## Experiment tracking issue -Each experiment requires a follow-up issue to resolve the experiment. Immediately after an experiment is deployed, the deadline of the issue needs to be set (this depends on the experiment but can be up to a few weeks in the future). +Each experiment should have an [Experiment tracking](https://gitlab.com/groups/gitlab-org/-/issues?scope=all&utf8=%E2%9C%93&state=opened&label_name[]=growth%20experiment&search=%22Experiment+tracking%22) issue to track the experiment from roll-out through to cleanup/removal. Immediately after an experiment is deployed, the due date of the issue should be set (this depends on the experiment but can be up to a few weeks in the future). After the deadline, the issue needs to be resolved and either: - It was successful and the experiment will be the new default. @@ -17,10 +17,10 @@ In either case, an outcome of the experiment should be posted to the issue with ## Code reviews Since the code of experiments will not be part of the codebase for a long time and we want to iterate fast to retrieve data,the code quality of experiments might sometimes not fulfill our standards but should not negatively impact the availability of GitLab whether the experiment is running or not. -Experiments will only be deployed to a fraction of users but we still want a flawless experience for those users. Therefore, experiments still require tests. +Initially experiments will only be deployed to a fraction of users but we still want a flawless experience for those users. Therefore, experiments still require tests. For reviewers and maintainers: if you find code that would usually not make it through the review, but is temporarily acceptable, please mention your concerns but note that it's not necessary to change. -The author then adds a comment to this piece of code and adds a link to the issue that resolves the experiment. +The author then adds a comment to this piece of code and adds a link to the issue that resolves the experiment. If the experiment is successful and becomes part of the product these follow up issues should be addressed. ## How to create an A/B test @@ -42,7 +42,7 @@ The author then adds a comment to this piece of code and adds a link to the issu - Use the experiment in a controller: ```ruby - class RegistrationController < Applicationcontroller + class RegistrationController < ApplicationController def show # experiment_enabled?(:feature_name) is also available in views and helpers if experiment_enabled?(:signup_flow) diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index 998c71135fb..669c93eb251 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -1,4 +1,4 @@ -# Accessibility +# Accessibility & Readability ## Resources diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index 892e931bf5b..ff36b8b5c6a 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -73,8 +73,8 @@ With the purpose of being [respectful of others' time](https://about.gitlab.com/ - Before assigning to a maintainer, assign to a reviewer. - If you assigned a merge request or pinged someone directly, be patient because we work in different timezones and asynchronously. Unless the merge request is urgent (like fixing a broken master), please don't DM or reassign the merge request before waiting for a 24-hour window. - If you have a question regarding your merge request/issue, make it on the merge request/issue. When we DM each other, we no longer have a SSOT and [no one else is able to contribute](https://about.gitlab.com/handbook/values/#public-by-default). -- When you have a big WIP merge request with many changes, you're advised to get the review started before adding/removing significant code. Make sure it is assigned well before the release cut-off, as the reviewer(s)/maintainer(s) would always prioritize reviewing finished MRs before WIP ones. -- Make sure to remove the WIP title before the last round of review. +- When you have a big **Draft** merge request with many changes, you're advised to get the review started before adding/removing significant code. Make sure it is assigned well before the release cut-off, as the reviewer(s)/maintainer(s) would always prioritize reviewing finished MRs before the **Draft** ones. +- Make sure to remove the `Draft:` title before the last round of review. ### Share your work early diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md index 4f814f3cdde..3c0845a9aaa 100644 --- a/doc/development/fe_guide/frontend_faq.md +++ b/doc/development/fe_guide/frontend_faq.md @@ -146,3 +146,20 @@ export const fetchFoos = ({ state }) => { return axios.get(state.settings.fooPath); }; ``` + +### 7. How can I test the production build locally? + +Sometimes it's necessary to test locally what the frontend production build would produce, to do so the steps are: + +1. Stop webpack: `gdk stop webpack`. +1. Open `gitlab.yaml` located in your `gitlab` installation folder, scroll down to the `webpack` section and change `dev_server` to `enabled: false`. +1. Run `yarn webpack-prod && gdk restart rails-web`. + +The production build takes a few minutes to be completed; any code change at this point will be +displayed only after executing the item 3 above again. +To return to the normal development mode: + +1. Open `gitlab.yaml` located in your `gitlab` installation folder, scroll down to the `webpack` section and change back `dev_server` to `enabled: true`. +1. Run `yarn clean` to remove the production assets and free some space (optional). +1. Start webpack again: `gdk start webpack`. +1. Restart GDK: `gdk-restart rails-web`. diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 191ebd2ff58..3d74ec94ae4 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -646,7 +646,7 @@ defaultClient.query({ query }) .then(result => console.log(result)); ``` -When [using Vuex](#Using-with-Vuex), disable the cache when: +When [using Vuex](#using-with-vuex), disable the cache when: - The data is being cached elsewhere - The use case does not need caching diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index 131324e6479..7078b5e9b2f 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -61,6 +61,7 @@ export default { <gl-icon name="issues" :size="24" + class="class-name" /> </template> ``` @@ -68,7 +69,7 @@ export default { - **name** Name of the Icon in the SVG Sprite ([Overview is available here](https://gitlab-org.gitlab.io/gitlab-svgs)). - **size (optional)** Number value for the size which is then mapped to a specific CSS class (Available Sizes: 8, 12, 16, 18, 24, 32, 48, 72 are mapped to `sXX` CSS classes) -- **css-classes (optional)** Additional CSS Classes to add to the SVG tag. +- **class (optional)** Additional CSS Classes to add to the SVG tag. ### Usage in HTML/JS diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 8fd6ba459b9..3a2b3cac9bf 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -17,7 +17,9 @@ Working with our frontend assets requires Node (v10.13.0 or greater) and Yarn For our currently-supported browsers, see our [requirements](../../install/requirements.md#supported-web-browsers). -Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers. Login to BrowserStack with the credentials saved in GitLab's [shared 1Password account](https://about.gitlab.com/handbook/security/#1password-for-teams). +Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers. +Sign in to BrowserStack with the credentials saved in the **Engineering** vault of GitLab's +[shared 1Password account](https://about.gitlab.com/handbook/security/#1password-guide). ## Initiatives diff --git a/doc/development/fe_guide/tooling.md b/doc/development/fe_guide/tooling.md index 585cd969c96..28deb7d95f9 100644 --- a/doc/development/fe_guide/tooling.md +++ b/doc/development/fe_guide/tooling.md @@ -4,6 +4,44 @@ We use ESLint to encapsulate and enforce frontend code standards. Our configuration may be found in the [`gitlab-eslint-config`](https://gitlab.com/gitlab-org/gitlab-eslint-config) project. +### Yarn Script + +This section describes yarn scripts that are available to validate and apply automatic fixes to files using ESLint. + +To check all currently staged files (based on `git diff`) with ESLint, run the following script: + +```shell +yarn eslint-staged +``` + +_A list of problems found will be logged to the console._ + +To apply automatic ESLint fixes to all currently staged files (based on `git diff`), run the following script: + +```shell +yarn eslint-staged-fix +``` + +_If manual changes are required, a list of changes will be sent to the console._ + +To check **all** files in the repository with ESLint, run the following script: + +```shell +yarn eslint +``` + +_A list of problems found will be logged to the console._ + +To apply automatic ESLint fixes to **all** files in the repository, run the following script: + +```shell +yarn eslint-fix +``` + +_If manual changes are required, a list of changes will be sent to the console._ + +**Caution:** Limit use to global rule updates. Otherwise, the changes can lead to huge Merge Requests. + ### Disabling ESLint in new files Do not disable ESLint when creating new files. Existing files may have existing rules @@ -56,13 +94,15 @@ When declaring multiple globals, always use one `/* global [name] */` line per v ## Formatting with Prettier -Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting `.js`, `.vue`, and `.scss` files based on the standard prettier rules. You can find all settings for Prettier in `.prettierrc`. +> Support for `.graphql` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/227280) in GitLab 13.2. + +Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting `.js`, `.vue`, `.graphql`, and `.scss` files based on the standard prettier rules. You can find all settings for Prettier in `.prettierrc`. ### Editor The easiest way to include prettier in your workflow is by setting up your preferred editor (all major editors are supported) accordingly. We suggest setting up prettier to run automatically when each file is saved. Find [here](https://prettier.io/docs/en/editors.html) the best way to set it up in your preferred editor. -Please take care that you only let Prettier format the same file types as the global Yarn script does (`.js`, `.vue`, and `.scss`). In VSCode by example you can easily exclude file formats in your settings file: +Please take care that you only let Prettier format the same file types as the global Yarn script does (`.js`, `.vue`, `.graphql`, and `.scss`). In VSCode by example you can easily exclude file formats in your settings file: ```json "prettier.disableLanguages": [ @@ -131,6 +171,9 @@ To select Prettier as a formatter, add the following properties to your User or }, "[vue]": { "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[graphql]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" } } ``` @@ -150,5 +193,8 @@ To automatically format your files with Prettier, add the following properties t "[vue]": { "editor.formatOnSave": true }, + "[graphql]": { + "editor.formatOnSave": true + }, } ``` diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 0d77e4d129b..2a0556c6cda 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -53,14 +53,14 @@ Be sure to read about [page-specific JavaScript](./performance.md#page-specific- #### Providing data from HAML to JavaScript -While mounting a Vue application may be a need to provide data from Rails to JavaScript. -To do that, provide the data through `data` attributes in the HTML element and query them while mounting the application. +While mounting a Vue application, you might need to provide data from Rails to JavaScript. +To do that, you can use the `data` attributes in the HTML element and query them while mounting the application. _Note:_ You should only do this while initializing the application, because the mounted element will be replaced with Vue-generated DOM. The advantage of providing data from the DOM to the Vue instance through `props` in the `render` function -instead of querying the DOM inside the main Vue component is that makes tests easier by avoiding the need to -create a fixture or an HTML element in the unit test. See the following example: +instead of querying the DOM inside the main Vue component is avoiding the need to create a fixture or an HTML element in the unit test, +which will make the tests easier. See the following example: ```javascript // haml @@ -134,7 +134,7 @@ This approach has a few benefits: intermediate components being aware of it (c.f. passing the flag down via props). - Good testability, since the flag can be provided to `mount`/`shallowMount` - from `vue-test-utils` as easily as a prop. + from `vue-test-utils` simply as a prop. ```javascript import { shallowMount } from '@vue/test-utils'; @@ -155,7 +155,7 @@ This folder holds all components that are specific of this new feature. If you need to use or create a component that will probably be used somewhere else, please refer to `vue_shared/components`. -A good thumb rule to know when you should create a component is to think if +A good rule of thumb to know when you should create a component is to think if it will be reusable elsewhere. For example, tables are used in a quite amount of places across GitLab, a table @@ -321,7 +321,7 @@ We should verify an event has been fired by asserting against the result of the ## Vue.js Expert Role -One should apply to be a Vue.js expert by opening an MR when the Merge Request's they create and review show: +You should only apply to be a Vue.js expert when your own merge requests and your reviews show: - Deep understanding of Vue and Vuex reactivity - Vue and Vuex code are structured according to both official and our guidelines diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index e7be67b8da5..02387c15951 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -201,6 +201,72 @@ By following this pattern we guarantee: 1. All data in the application follows the same lifecycle pattern 1. Unit tests are easier +#### Updating complex state + +Sometimes, especially when the state is complex, is really hard to traverse the state to precisely update what the mutation needs to update. +Ideally a `vuex` state should be as normalized/decoupled as possible but this is not always the case. + +It's important to remember that the code is much easier to read and maintain when the `portion of the mutated state` is selected and mutated in the mutation itself. + +Given this state: + +```javascript + export default () => ({ + items: [ + { + id: 1, + name: 'my_issue', + closed: false, + }, + { + id: 2, + name: 'another_issue', + closed: false, + } + ] +}); +``` + +It may be tempting to write a mutation like so: + +```javascript +// Bad +export default { + [types.MARK_AS_CLOSED](state, item) { + Object.assign(item, {closed: true}) + } +} +``` + +While this approach works it has several dependencies: + +- Correct selection of `item` in the component/action. +- The `item` property is already declared in the `closed` state. + - A new `confidential` property would not be reactive. +- Noting that `item` is referenced by `items` + +A mutation written like this is harder to maintain and more error prone. We should rather write a mutation like this: + +```javascript +// Good +export default { + [types.MARK_AS_CLOSED](state, itemId) { + const item = state.items.find(i => i.id == itemId); + Vue.set(item, 'closed', true) + + state.items.splice(index, 1, item) + } +} +``` + +This approach is better because: + +- It selects and updates the state in the mutation, which is more maintainable. +- It has no external dependencies, if the correct `itemId` is passed the state is correctly updated. +- It does not have reactivity caveats, as we generate a new `item` to avoid coupling to the initial state. + +A mutation written like this is easier to maintain. In addition, we avoid errors due to the limitation of the reactivity system. + ### `getters.js` Sometimes we may need to get derived state based on store state, like filtering for a specific prop. diff --git a/doc/development/feature_categorization/index.md b/doc/development/feature_categorization/index.md new file mode 100644 index 00000000000..bcd5e16cc2b --- /dev/null +++ b/doc/development/feature_categorization/index.md @@ -0,0 +1,130 @@ +# Feature Categorization + +> [Introduced](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/269) in GitLab 13.2. + +Each Sidekiq worker, controller action, or (eventually) API endpoint +must declare a `feature_category` attribute. This attribute maps each +of these to a [feature +category](https://about.gitlab.com/handbook/product/product-categories/). This +is done for error budgeting, alert routing, and team attribution. + +The list of feature categories can be found in the file `config/feature_categories.yml`. +This file is generated from the +[`stages.yml`](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml) +data file used in the GitLab Handbook and other GitLab resources. + +## Updating `config/feature_categories.yml` + +Occasionally new features will be added to GitLab stages, groups, and +product categories. When this occurs, you can automatically update +`config/feature_categories.yml` by running +`scripts/update-feature-categories`. This script will fetch and parse +[`stages.yml`](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml) +and generate a new version of the file, which needs to be committed to +the repository. + +The [Scalabilitity +team](https://about.gitlab.com/handbook/engineering/infrastructure/team/scalability) +currently maintains the `stages.yml` file. They will automatically be +notified on Slack when the file becomes outdated. + +## Sidekiq workers + +The declaration uses the `feature_category` class method, as shown below. + +```ruby +class SomeScheduledTaskWorker + include ApplicationWorker + + # Declares that this worker is part of the + # `continuous_integration` feature category + feature_category :continuous_integration + + # ... +end +``` + +The feature categories specified using `feature_category` should be +defined in +[`config/feature_categories.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/feature_categories.yml). If +not, the specs will fail. + +### Excluding Sidekiq workers from feature categorization + +A few Sidekiq workers, that are used across all features, cannot be mapped to a +single category. These should be declared as such using the `feature_category_not_owned!` +declaration, as shown below: + +```ruby +class SomeCrossCuttingConcernWorker + include ApplicationWorker + + # Declares that this worker does not map to a feature category + feature_category_not_owned! + + # ... +end +``` + +## Rails controllers + +Specifying feature categories on controller actions can be done using +the `feature_category` class method. + +A feature category can be specified on an entire controller +using: + +```ruby +class Projects::MergeRequestsController < ApplicationController + feature_category :source_code_management +end +``` + +The feature category can be limited to a list of actions using the +`only` argument, actions can be excluded using the `except` argument. + +```ruby +class Projects::MergeRequestsController < ApplicationController + feature_category :code_testing, only: [:metrics_reports] + feature_category :source_code_management, except: [:test_reports, :coverage_reports] +end +``` + +`except` and `only` arguments can not be combined. + +When specifying `except` all other actions will get the specified +category assigned. + +The assignment can also be scoped using `if` and `unless` procs: + +```ruby +class Projects::MergeRequestsController < ApplicationController + feature_category :source_code_management, + unless: -> (action) { action.include?("reports") } + if: -> (action) { action.include?("widget") } +end +``` + +In this case, both procs need to be satisfied for the action to get +the category assigned. + +### Excluding controller actions from feature categorization + +In the rare case an action cannot be tied to a feature category this +can be done using the `not_owned` feature category. + +```ruby +class Admin::LogsController < ApplicationController + feature_category :not_owned +end +``` + +### Ensuring feature categories are valid + +The `spec/controllers/every_controller_spec.rb` will iterate over all +defined routes, and check the controller to see if a category is +assigned to all actions. + +The spec also validates if the used feature categories are known. And +if the actions used in `only` and `except` configuration still exist +as routes. diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index d4d1ec74591..9e7ce74cc0c 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -210,11 +210,20 @@ actors. Feature.enabled?(:some_feature, group) ``` -NOTE: - +NOTE: **Note:** **Percentage of time** rollout is not a good idea if what you want is to make sure a feature is always on or off to the users. In that case, **Percentage of actors** rollout is a better method. +Lastly, to verify that the feature is deemed stable in as many cases as possible, +you should fully roll out the feature by enabling the flag **globally** by running: + +```shell +/chatops run feature set some_feature true +``` + +This changes the feature flag state to be **enabled** always, which overrides the +existing gates (e.g. `--group=gitlab-org`) in the above processes. + ### Feature flag change logging Any feature flag change that affects GitLab.com (production) will diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md index a44bc70439e..0b918478668 100644 --- a/doc/development/feature_flags/development.md +++ b/doc/development/feature_flags/development.md @@ -32,8 +32,8 @@ request removing the feature flag or the merge request where the default value o the feature flag is set to true. If the feature contains any DB migration it should include a changelog entry for DB changes. -In the rare case that you need the feature flag to be on automatically, use -`default_enabled: true` when checking: +If you need the feature flag to be on automatically, use `default_enabled: true` +when checking: ```ruby Feature.enabled?(:feature_flag, project, default_enabled: true) diff --git a/doc/development/feature_flags/index.md b/doc/development/feature_flags/index.md index bd0bd8f2018..0c1e34edc6f 100644 --- a/doc/development/feature_flags/index.md +++ b/doc/development/feature_flags/index.md @@ -1,6 +1,6 @@ # Feature flags in development of GitLab -[Feature Flags](../../user/project/operations/feature_flags.md) +[Feature Flags](../../operations/feature_flags.md) can be used to gradually roll out changes, be it a new feature, or a performance improvement. By using feature flags, we can comfortably measure the impact of our changes, while still being able to easily diff --git a/doc/development/feature_flags/process.md b/doc/development/feature_flags/process.md index 57360f5b771..b053838964b 100644 --- a/doc/development/feature_flags/process.md +++ b/doc/development/feature_flags/process.md @@ -4,7 +4,7 @@ This document only covers feature flags used in the development of GitLab itself. Feature flags in deployed user applications can be found at -[Feature Flags feature documentation](../../user/project/operations/feature_flags.md). +[Feature Flags feature documentation](../../operations/feature_flags.md). ## Feature flags in GitLab development @@ -21,6 +21,19 @@ should be leveraged: - Merge requests that make changes hidden behind a feature flag, or remove an existing feature flag because a feature is deemed stable must have the ~"feature flag" label assigned. +- When development of a feature will be spread across multiple merge + requests, you can use the following workflow: + + 1. Introduce a feature flag which is **off** by default, in the first merge request. + 1. Submit incremental changes via one or more merge requests, ensuring that any + new code added can only be reached if the feature flag is **on**. + You can keep the feature flag enabled on your local GDK during development. + 1. When the feature is ready to be tested, enable the feature flag for + a specific project and ensure that there are no issues with the implementation. + 1. When the feature is ready to be announced, create a merge request that adds + documentation about the feature, including [documentation for the feature flag itself](../documentation/feature_flags.md), + and a changelog entry. In the same merge request either flip the feature flag to + be **on by default** or remove it entirely in order to enable the new behavior. One might be tempted to think that feature flags will delay the release of a feature by at least one month (= one release). This is not the case. A feature @@ -29,6 +42,8 @@ flag does not have to stick around for a specific amount of time is deemed stable. Stable means it works on GitLab.com without causing any problems, such as outages. +Please also read the [development guide for feature flags](development.md). + ### When to use feature flags Starting with GitLab 11.4, developers are required to use feature flags for @@ -56,7 +71,9 @@ absolutely no way to use the feature until it is enabled. In order to build a final release and present the feature for self-managed users, the feature flag should be at least defaulted to **on**. If the feature is deemed stable and there is confidence that removing the feature flag is safe, -consider removing the feature flag altogether. +consider removing the feature flag altogether. It's _strongly_ recommended that +the feature flag is [enabled **globally** on **production**](./controls.md#enabling-a-feature-for-gitlabcom) for **at least one day** +before making this decision. Unexpected bugs are sometimes discovered during this period. The process for enabling features that are disabled by default can take 5-6 days from when the merge request is first reviewed to when the change is deployed to @@ -71,7 +88,7 @@ Take into consideration that such action can make the feature available on GitLab.com shortly after the change to the feature flag is merged. Changing the default state or removing the feature flag has to be done before -the 22nd of the month, _at least_ 2 working days before, in order for the change +the 22nd of the month, _at least_ 3-4 working days before, in order for the change to be included in the final self-managed release. In addition to this, the feature behind feature flag should: diff --git a/doc/development/foreign_keys.md b/doc/development/foreign_keys.md index 508e5665f08..8a81dc158a7 100644 --- a/doc/development/foreign_keys.md +++ b/doc/development/foreign_keys.md @@ -35,6 +35,17 @@ When adding a foreign key in PostgreSQL the column is not indexed automatically, thus you must also add a concurrent index. Not doing so will result in cascading deletes being very slow. +## Naming foreign keys + +By default Ruby on Rails uses the `_id` suffix for foreign keys. So we should +only use this suffix for associations between two tables. If you want to +reference an ID on a third party platform the `_xid` suffix is recommended. + +The spec `spec/db/schema_spec.rb` will test if all columns with the `_id` suffix +have a foreign key constraint. So if that spec fails, don't add the column to +`IGNORED_FK_COLUMNS`, but instead add the FK constraint, or consider naming it +differently. + ## Dependent Removals Don't define options such as `dependent: :destroy` or `dependent: :delete` when diff --git a/doc/development/geo/framework.md b/doc/development/geo/framework.md index 85fecc41cfb..de4d6fb869d 100644 --- a/doc/development/geo/framework.md +++ b/doc/development/geo/framework.md @@ -1,11 +1,13 @@ # Geo self-service framework (alpha) -NOTE: **Note:** This document might be subjected to change. It's a +NOTE: **Note:** +This document might be subjected to change. It's a proposal we're working on and once the implementation is complete this documentation will be updated. Follow progress in the [epic](https://gitlab.com/groups/gitlab-org/-/epics/2161). -NOTE: **Note:** The Geo self-service framework is currently in +NOTE: **Note:** +The Geo self-service framework is currently in alpha. If you need to replicate a new data type, reach out to the Geo team to discuss the options. You can contact them in `#g_geo` on Slack or mention `@geo-team` in the issue or merge request. @@ -178,13 +180,17 @@ For example, to add support for files referenced by a `Widget` model with a mount_uploader :file, WidgetUploader + def self.replicables_for_geo_node + # Should be implemented. The idea of the method is to restrict + # the set of synced items depending on synchronization settings + end ... end ``` 1. Create `ee/app/replicators/geo/widget_replicator.rb`. Implement the `#carrierwave_uploader` method which should return a `CarrierWave::Uploader`. - And implement the private `#model` method to return the `Widget` class. + And implement the class method `.model` to return the `Widget` class. ```ruby # frozen_string_literal: true @@ -193,14 +199,12 @@ For example, to add support for files referenced by a `Widget` model with a class WidgetReplicator < Gitlab::Geo::Replicator include ::Geo::BlobReplicatorStrategy - def carrierwave_uploader - model_record.file + def self.model + ::Widget end - private - - def model - ::Widget + def carrierwave_uploader + model_record.file end end end @@ -215,7 +219,7 @@ For example, to add support for files referenced by a `Widget` model with a require 'spec_helper' - describe Geo::WidgetReplicator do + RSpec.describe Geo::WidgetReplicator do let(:model_record) { build(:widget) } it_behaves_like 'a blob replicator' @@ -231,20 +235,32 @@ For example, to add support for files referenced by a `Widget` model with a class CreateWidgetRegistry < ActiveRecord::Migration[6.0] DOWNTIME = false - def change - create_table :widget_registry, id: :serial, force: :cascade do |t| - t.integer :widget_id, null: false - t.integer :state, default: 0, null: false - t.integer :retry_count, default: 0 - t.string :last_sync_failure, limit: 255 - t.datetime_with_timezone :retry_at - t.datetime_with_timezone :last_synced_at - t.datetime_with_timezone :created_at, null: false - - t.index :widget_id, name: :index_widget_registry_on_repository_id, using: :btree - t.index :retry_at, name: :index_widget_registry_on_retry_at, using: :btree - t.index :state, name: :index_widget_registry_on_state, using: :btree + disable_ddl_transaction! + + def up + unless table_exists?(:widget_registry) + ActiveRecord::Base.transaction do + create_table :widget_registry, id: :bigserial, force: :cascade do |t| + t.integer :widget_id, null: false + t.integer :state, default: 0, null: false, limit: 2 + t.integer :retry_count, default: 0, limit: 2 + t.text :last_sync_failure + t.datetime_with_timezone :retry_at + t.datetime_with_timezone :last_synced_at + t.datetime_with_timezone :created_at, null: false + + t.index :widget_id + t.index :retry_at + t.index :state + end + end end + + add_text_limit :widget_registry, :last_sync_failure, 255 + end + + def down + drop_table :widget_registry end end ``` @@ -255,21 +271,28 @@ For example, to add support for files referenced by a `Widget` model with a # frozen_string_literal: true class Geo::WidgetRegistry < Geo::BaseRegistry - include Geo::StateMachineRegistry + include Geo::ReplicableRegistry + MODEL_CLASS = ::Widget MODEL_FOREIGN_KEY = :widget_id belongs_to :widget, class_name: 'Widget' end ``` + The method `has_create_events?` should return `true` in most of the cases. + However, if the entity you add doesn't have the create event, don't add the + method at all. + +1. Update `REGISTRY_CLASSES` in `ee/app/workers/geo/secondary/registry_consistency_worker.rb`. + 1. Create `ee/spec/factories/geo/widget_registry.rb`: ```ruby # frozen_string_literal: true FactoryBot.define do - factory :widget_registry, class: 'Geo::WidgetRegistry' do + factory :geo_widget_registry, class: 'Geo::WidgetRegistry' do widget state { Geo::WidgetRegistry.state_value(:pending) } @@ -301,14 +324,18 @@ For example, to add support for files referenced by a `Widget` model with a require 'spec_helper' - describe Geo::WidgetRegistry, :geo, type: :model do - let_it_be(:registry) { create(:widget_registry) } + RSpec.describe Geo::WidgetRegistry, :geo, type: :model do + let_it_be(:registry) { create(:geo_widget_registry) } specify 'factory is valid' do expect(registry).to be_valid end include_examples 'a Geo framework registry' + + describe '.find_registry_differences' do + ... # To be implemented + end end ``` @@ -360,36 +387,58 @@ Widgets should now be replicated by Geo! end ``` -1. Add fields `widget_count`, `widget_checksummed_count`, `widget_checksum_failed_count`, - `widget_synced_count` and `widget_failed_count` - to `GeoNodeStatus#RESOURCE_STATUS_FIELDS` array in `ee/app/models/geo_node_status.rb`. +To do: Add verification on secondaries. This should be done as part of +[Geo: Self Service Framework - First Implementation for Package File verification](https://gitlab.com/groups/gitlab-org/-/epics/1817) + +Widgets should now be verified by Geo! + +#### Metrics + +Metrics are gathered by `Geo::MetricsUpdateWorker`, persisted in +`GeoNodeStatus` for display in the UI, and sent to Prometheus. + +1. Add fields `widget_count`, `widget_checksummed_count`, + `widget_checksum_failed_count`, `widget_synced_count`, + `widget_failed_count`, and `widget_registry_count` to + `GeoNodeStatus#RESOURCE_STATUS_FIELDS` array in + `ee/app/models/geo_node_status.rb`. 1. Add the same fields to `GeoNodeStatus#PROMETHEUS_METRICS` hash in `ee/app/models/geo_node_status.rb`. 1. Add the same fields to `Sidekiq metrics` table in `doc/administration/monitoring/prometheus/gitlab_metrics.md`. -1. Add the same fields to `GET /geo_nodes/status` example response in `doc/api/geo_nodes.md`. -1. Modify `GeoNodeStatus#load_verification_data` to make sure the fields mantioned above - are set: +1. Add the same fields to `GET /geo_nodes/status` example response in + `doc/api/geo_nodes.md`. +1. Add the same fields to `ee/spec/models/geo_node_status_spec.rb` and + `ee/spec/factories/geo_node_statuses.rb`. +1. Set `widget_count` in `GeoNodeStatus#load_data_from_current_node`: + + ```ruby + self.widget_count = Geo::WidgetReplicator.primary_total_count + ``` + +1. Add `GeoNodeStatus#load_widgets_data` to set `widget_synced_count`, + `widget_failed_count`, and `widget_registry_count`: ```ruby - self.widget_count = Geo::WidgetReplicator.model.count - self.widget_checksummed_count = Geo::WidgetReplicator.checksummed.count - self.widget_checksum_failed_count = Geo::WidgetReplicator.checksum_failed.count + def load_widget_data self.widget_synced_count = Geo::WidgetReplicator.synced_count self.widget_failed_count = Geo::WidgetReplicator.failed_count + self.widget_registry_count = Geo::WidgetReplicator.registry_count + end ``` -1. Make sure `Widget` model has `checksummed` and `checksum_failed` scopes. -1. Update `ee/spec/fixtures/api/schemas/public_api/v4/geo_node_status.json` with new fields. -1. Update `GeoNodeStatus#PROMETHEUS_METRICS` hash in `ee/app/models/geo_node_status.rb` with new fields. -1. Update `Sidekiq metrics` table in `doc/administration/monitoring/prometheus/gitlab_metrics.md` with new fields. -1. Update `GET /geo_nodes/status` example response in `doc/api/geo_nodes.md` with new fields. -1. Update `ee/spec/models/geo_node_status_spec.rb` and `ee/spec/factories/geo_node_statuses.rb` with new fields. +1. Call `GeoNodeStatus#load_widgets_data` in + `GeoNodeStatus#load_secondary_data`. -To do: Add verification on secondaries. This should be done as part of -[Geo: Self Service Framework - First Implementation for Package File verification](https://gitlab.com/groups/gitlab-org/-/epics/1817) +1. Set `widget_checksummed_count` and `widget_checksum_failed_count` in + `GeoNodeStatus#load_verification_data`: -Widgets should now be verified by Geo! + ```ruby + self.widget_checksummed_count = Geo::WidgetReplicator.checksummed_count self.widget_checksum_failed_count = Geo::WidgetReplicator.checksum_failed_count + ``` + +Widget replication and verification metrics should now be available in the API, +the Admin Area UI, and Prometheus! #### GraphQL API @@ -428,8 +477,8 @@ Widgets should now be verified by Geo! require 'spec_helper' - describe Resolvers::Geo::WidgetRegistriesResolver do - it_behaves_like 'a Geo registries resolver', :widget_registry + RSpec.describe Resolvers::Geo::WidgetRegistriesResolver do + it_behaves_like 'a Geo registries resolver', :geo_widget_registry end ``` @@ -452,8 +501,8 @@ Widgets should now be verified by Geo! require 'spec_helper' - describe Geo::WidgetRegistryFinder do - it_behaves_like 'a framework registry finder', :widget_registry + RSpec.describe Geo::WidgetRegistryFinder do + it_behaves_like 'a framework registry finder', :geo_widget_registry end ``` @@ -484,7 +533,7 @@ Widgets should now be verified by Geo! require 'spec_helper' - describe GitlabSchema.types['WidgetRegistry'] do + RSpec.describe GitlabSchema.types['WidgetRegistry'] do it_behaves_like 'a Geo registry type' it 'has the expected fields (other than those included in RegistryType)' do @@ -503,7 +552,7 @@ Widgets should now be verified by Geo! it_behaves_like 'gets registries for', { field_name: 'widgetRegistries', registry_class_name: 'WidgetRegistry', - registry_factory: :widget_registry, + registry_factory: :geo_widget_registry, registry_foreign_key_field_name: 'widgetId' } ``` @@ -511,6 +560,13 @@ Widgets should now be verified by Geo! Individual widget synchronization and verification data should now be available via the GraphQL API! +1. Take care of replicating "update" events. Geo Framework does not currently support + replicating "update" events because all entities added to the framework, by this time, + are immutable. If this is the case + for the entity you're going to add, please follow <https://gitlab.com/gitlab-org/gitlab/-/issues/118743> + and <https://gitlab.com/gitlab-org/gitlab/-/issues/118745> as examples to add the new event type. + Please also remove this notice when you've added it. + #### Admin UI To do: This should be done as part of diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index 417c96a44dd..8b4e5090abb 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -1,7 +1,14 @@ -# GitLab Developers Guide to Working with Gitaly +--- +stage: Create +group: Gitaly +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/#designated-technical-writers +type: reference +--- -[Gitaly](https://gitlab.com/gitlab-org/gitaly) is a high-level Git RPC service used by GitLab CE/EE, -Workhorse and GitLab-Shell. +# Gitaly developers guide + +[Gitaly](https://gitlab.com/gitlab-org/gitaly) is a high-level Git RPC service used by GitLab Rails, +Workhorse and GitLab Shell. ## Deep Dive @@ -114,7 +121,8 @@ bundle exec rake gitlab:features:disable_rugged Most of this code exists in the `lib/gitlab/git/rugged_impl` directory. -NOTE: **Note:** You should NOT need to add or modify code related to +NOTE: **Note:** +You should NOT need to add or modify code related to Rugged unless explicitly discussed with the [Gitaly Team](https://gitlab.com/groups/gl-gitaly/group_members). This code will NOT work on GitLab.com or other GitLab instances that do not use NFS. diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index 5a5e163e142..779dcd9b7a2 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -414,7 +414,7 @@ it will display its help message (if `cli` has been used). With the exception of [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner), which publishes its own binaries, our Go binaries are created by projects -managed by the [Distribution group](https://about.gitlab.com/handbook/product/categories/#distribution-group). +managed by the [Distribution group](https://about.gitlab.com/handbook/product/product-categories/#distribution-group). The [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab) project creates a single, monolithic operating system package containing all the binaries, while @@ -453,6 +453,28 @@ are: To reduce unnecessary differences between two distribution methods, Omnibus and CNG **should always use the same Go version**. +### Supporting multiple Go versions + +Individual Golang-projects need to support multiple Go versions for the following reasons: + +1. When a new Go release is out, we should start integrating it into the CI pipelines to verify compatibility with the new compiler. +1. We must support the [Omnibus official Go version](#updating-go-version), which may be behind the latest minor release. +1. When Omnibus switches Go version, we still may need to support the old one for security backports. + +These 3 requirements may easily be satisfied by keeping support for the 3 latest minor versions of Go. + +It's ok to drop support for the oldest Go version and support only 2 latest releases, +if this is enough to support backports to the last 3 GitLab minor releases. + +Example: + +In case we want to drop support for `go 1.11` in GitLab `12.10`, we need to verify which Go versions we are using in `12.9`, `12.8`, and `12.7`. + +We will not consider the active milestone, `12.10`, because a backport for `12.7` will be required in case of a critical security release. + +1. If both [Omnibus and CNG](#updating-go-version) were using Go `1.12` since GitLab `12.7`, then we safely drop support for `1.11`. +1. If Omnibus or CNG were using `1.11` in GitLab `12.7`, then we still need to keep support for Go `1.11` for easier backporting of security fixes. + ## Secure Team standards and style guidelines The following are some style guidelines that are specific to the Secure Team. diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md index 7e08da8162b..34fe3f11489 100644 --- a/doc/development/gotchas.md +++ b/doc/development/gotchas.md @@ -20,7 +20,7 @@ Consider the following API spec: ```ruby require 'spec_helper' -describe API::Labels do +RSpec.describe API::Labels do it 'creates a first label' do create(:label) @@ -71,7 +71,7 @@ Following is the fixed API spec: ```ruby require 'spec_helper' -describe API::Labels do +RSpec.describe API::Labels do it 'creates a first label' do create(:label, title: 'foo') diff --git a/doc/development/i18n/externalization.md b/doc/development/i18n/externalization.md index bdd372e90ed..bce95dfc24e 100644 --- a/doc/development/i18n/externalization.md +++ b/doc/development/i18n/externalization.md @@ -95,7 +95,8 @@ Active Record's `:message` option accepts a `Proc`, so we can do this instead: validates :group_id, uniqueness: { scope: [:project_id], message: -> (object, data) { _("already shared with this group") } } ``` -NOTE: **Note:** Messages in the API (`lib/api/` or `app/graphql`) do +NOTE: **Note:** +Messages in the API (`lib/api/` or `app/graphql`) do not need to be externalised. ### HAML files diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 8c1f62330f4..935a171f34c 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -105,6 +105,7 @@ are very appreciative of the work done by translators and proofreaders! - Proofreaders needed. - Turkish - Ali Demirtaş - [GitLab](https://gitlab.com/alidemirtas), [CrowdIn](https://crowdin.com/profile/alidemirtas) + - Rıfat Ünalmış (Rifat Unalmis) - [GitLab](https://gitlab.com/runalmis), [CrowdIn](https://crowdin.com/profile/runalmis) - Ukrainian - Volodymyr Sobotovych - [GitLab](https://gitlab.com/wheleph), [CrowdIn](https://crowdin.com/profile/wheleph) - Andrew Vityuk - [GitLab](https://gitlab.com/3_1_3_u), [CrowdIn](https://crowdin.com/profile/andruwa13) diff --git a/doc/development/import_export.md b/doc/development/import_export.md index ecdfd8a853e..556fa6c7db4 100644 --- a/doc/development/import_export.md +++ b/doc/development/import_export.md @@ -408,4 +408,5 @@ tree └── 4352.json ``` -CAUTION: **Caution:** When updating these fixtures, please ensure you update both `json` files and `tree` folder, as the tests apply to both. +CAUTION: **Caution:** +When updating these fixtures, please ensure you update both `json` files and `tree` folder, as the tests apply to both. diff --git a/doc/development/import_project.md b/doc/development/import_project.md index f222a6533e8..1fa6ea5d405 100644 --- a/doc/development/import_project.md +++ b/doc/development/import_project.md @@ -17,7 +17,8 @@ The first option is to simply [import the Project tarball file via the GitLab UI It should take up to 15 minutes for the project to fully import. You can head to the project's main page for the current status. -NOTE: **Note:** This method ignores all the errors silently (including the ones related to `GITALY_DISABLE_REQUEST_LIMITS`) and is used by GitLab's users. For development and testing, check the other methods below. +NOTE: **Note:** +This method ignores all the errors silently (including the ones related to `GITALY_DISABLE_REQUEST_LIMITS`) and is used by GitLab's users. For development and testing, check the other methods below. ### Importing via the `import-project` script @@ -47,7 +48,9 @@ This method will take longer to import than the other methods and will depend on ### Importing via a Rake task -[`import.rake`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/tasks/gitlab/import_export/import.rake) was introduced for importing large GitLab project exports. +> The [Rake task](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/tasks/gitlab/import_export/import.rake) was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20724) in GitLab 12.6, replacing a GitLab.com Ruby script. + +This script was introduced in GitLab 12.6 for importing large GitLab project exports. As part of this script we also disable direct and background upload to avoid situations where a huge archive is being uploaded to GCS (while being inside a transaction, which can cause idle transaction timeouts). @@ -63,9 +66,53 @@ Parameters: | `archive_path` | string | yes | Path to the exported project tarball you want to import | ```shell -bundle exec rake "gitlab:import_export:import[root, root, testingprojectimport, /path/to/file.tar.gz]" +bundle exec rake "gitlab:import_export:import[root, group/subgroup, testingprojectimport, /path/to/file.tar.gz]" +``` + +If you're running Omnibus, run the following Rake task: + +```shell +gitlab-rake "gitlab:import_export:import[root, group/subgroup, testingprojectimport, /path/to/file.tar.gz]" +``` + +#### Troubleshooting + +Check the common errors listed below, what they mean, and how to fix them. + +##### `Exception: undefined method 'name' for nil:NilClass` + +The `username` is not valid. + +##### `Exception: undefined method 'full_path' for nil:NilClass` + +The `namespace_path` does not exist. +For example, one of the groups or subgroups is mistyped or missing +or you've specified the project name in the path. + +The task will only create the project. +If you want to import it to a new group or subgroup then create it first. + +##### `Exception: No such file or directory @ rb_sysopen - (filename)` + +The specified project export file in `archive_path` is missing. + +##### `Name can contain only letters, digits, emojis ...` + +```plaintext +Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, +digit, emoji or '_'. and Path can contain only letters, digits, '_', '-' and '.'. Cannot start +with '-', end in '.git' or end in '.atom' ``` +The project name specified in `project_path` is not valid for one of the specified reasons. + +Only put the project name in `project_path`. If, for example, you put a path of subgroups in there +it will fail with this error as `/` is not a valid character in a project name. + +##### `Name has already been taken and Path has already been taken` + +A project with that name already exists. + ### Importing via the Rails console The last option is to import a project using a Rails console: diff --git a/doc/development/insert_into_tables_in_batches.md b/doc/development/insert_into_tables_in_batches.md index d8919789808..f65d2478d2e 100644 --- a/doc/development/insert_into_tables_in_batches.md +++ b/doc/development/insert_into_tables_in_batches.md @@ -121,10 +121,10 @@ These callbacks cannot be used with bulk insertions, since they are meant to be every instance that is saved or created. Since these events do not fire when records are inserted in bulk, we currently disallow their use. -The specifics around which callbacks are disallowed are defined in +The specifics around which callbacks are explicitly allowed are defined in [`BulkInsertSafe`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/concerns/bulk_insert_safe.rb). -Consult the module source code for details. If your class uses any of the blacklisted -functionality, and you `include BulkInsertSafe`, the application will fail with an error. +Consult the module source code for details. If your class uses callbacks that are not explicitly designated +safe and you `include BulkInsertSafe` the application will fail with an error. ### `BulkInsertSafe` versus `InsertAll` diff --git a/doc/development/integrations/elasticsearch_for_paid_tiers_on_gitlabcom.md b/doc/development/integrations/elasticsearch_for_paid_tiers_on_gitlabcom.md new file mode 100644 index 00000000000..8289be47253 --- /dev/null +++ b/doc/development/integrations/elasticsearch_for_paid_tiers_on_gitlabcom.md @@ -0,0 +1,28 @@ +# Elasticsearch for paid tiers on GitLab.com + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/220246) in GitLab 13.2 +> - It's deployed behind a feature flag, disabled by default. +> - It's disabled on GitLab.com. +> - It's not recommended for use in GitLab self-managed instances. + +This document describes how to enable Elasticsearch with GitLab for all paid tiers on GitLab.com. Once enabled, +all paid tiers will have access to the [Advanced Global Search feature](../../integration/elasticsearch.md) on GitLab.com. + +## Enable or disable Elasticsearch for all paid tiers on GitLab.com + +Since we're still in the process of rolling this out and want to control the timing this is behind a feature flag +which defaults to off. + +To enable it: + +```ruby +# Instance-wide +Feature.enable(:elasticsearch_index_only_paid_groups) +``` + +To disable it: + +```ruby +# Instance-wide +Feature.disable(:elasticsearch_index_only_paid_groups) +``` diff --git a/doc/development/integrations/jira_connect.md b/doc/development/integrations/jira_connect.md index 8e619c3b0a2..374cc976caa 100644 --- a/doc/development/integrations/jira_connect.md +++ b/doc/development/integrations/jira_connect.md @@ -11,7 +11,7 @@ The following are required to install and test the app: For the app to work, Jira Cloud should be able to connect to the GitLab instance through the internet. To easily expose your local development environment, you can use tools like - [serveo](https://medium.com/@osanda.deshan/how-to-forward-my-local-port-to-public-using-serveo-4979f352a3bf) + [serveo](https://medium.com/testautomator/how-to-forward-my-local-port-to-public-using-serveo-4979f352a3bf) or [ngrok](https://ngrok.com). These also take care of SSL for you because Jira requires all connections to the app host to be over SSL. diff --git a/doc/development/integrations/secure.md b/doc/development/integrations/secure.md index 1737daae0e0..22da57400e0 100644 --- a/doc/development/integrations/secure.md +++ b/doc/development/integrations/secure.md @@ -100,13 +100,12 @@ the project repository contains Java source code and the `dependency_scanning` f ```yaml mysec_dependency_scanning: - except: - variables: - - $DEPENDENCY_SCANNING_DISABLED - only: - variables: - - $GITLAB_FEATURES =~ /\bdependency_scanning\b/ && - $CI_PROJECT_REPOSITORY_LANGUAGES =~ /\bjava\b/ + rules: + - if: $DEPENDENCY_SCANNING_DISABLED + when: never + - if: $GITLAB_FEATURES =~ /\bdependency_scanning\b/ + exists: + - '**/*.java' ``` Any additional job policy should only be configured by users based on their needs. @@ -232,6 +231,32 @@ to colorize the messages they write to the Unix standard output and standard err We recommend using red to report errors, yellow for warnings, and green for notices. Also, we recommend prefixing error messages with `[ERRO]`, warnings with `[WARN]`, and notices with `[INFO]`. +#### Logging level + +The scanner should filter out a log message if its log level is lower than the +one set in the `SECURE_LOG_LEVEL` variable. For instance, `info` and `warn` +messages should be skipped when `SECURE_LOG_LEVEL` is set to `error`. Accepted +values are as follows, listed from highest to lowest: + +- `fatal` +- `error` +- `warn` +- `info` +- `debug` + +It is recommended to use the `debug` level for verbose logging that could be +useful when debugging. The default value for `SECURE_LOG_LEVEL` should be set +to `info`. + +#### common logutil package + +If you are using [go](https://golang.org/) and +[common](https://gitlab.com/gitlab-org/security-products/analyzers/common), +then it is suggested that you use [logrus](https://github.com/Sirupsen/logrus) +and [common's logutil package](https://gitlab.com/gitlab-org/security-products/analyzers/common/-/tree/master/logutil) +to configure the formatter for [logrus](https://github.com/Sirupsen/logrus). +See the [logutil README.md](https://gitlab.com/gitlab-org/security-products/analyzers/common/-/tree/master/logutil/README.md) + ## Report The report is a JSON document that combines vulnerabilities with possible remediations. @@ -547,3 +572,15 @@ remediation. `fixes[].id` contains a fixed vulnerability's [unique identifier](# The `diff` field is a base64-encoded remediation code diff, compatible with [`git apply`](https://git-scm.com/docs/git-format-patch#_discussion). This field is required. + +## Limitations + +### Container Scanning + +Container Scanning currently has these limitations: + +- Although the Security Dashboard can display scan results from multiple images, if multiple + vulnerabilities have the same fingerprint, only the first instance of that vulnerability is + displayed. We're working on removing this limitation. You can follow our progress on the issue + [Change location fingerprint for Container Scanning](https://gitlab.com/gitlab-org/gitlab/-/issues/215466). +- Different scanners may each report the same vulnerability, resulting in duplicate findings. diff --git a/doc/development/integrations/secure_partner_integration.md b/doc/development/integrations/secure_partner_integration.md index 22e1f8bf769..19a497641f9 100644 --- a/doc/development/integrations/secure_partner_integration.md +++ b/doc/development/integrations/secure_partner_integration.md @@ -11,6 +11,15 @@ with [onboarding as a partner](https://about.gitlab.com/partners/integrate/). The steps below are a high-level view of what needs to be done to complete an integration as well as linking to more detailed resources for how to do so. +## Integration Tiers + +GitLab's security offerings are designed for GitLab Gold and GitLab Ultimate users, and the +[DevSecOps](https://about.gitlab.com/handbook/use-cases/#4-devsecops-shift-left-security) +use case. All the features are in those tiers. This includes the APIs and standard reporting +framework needed to provide a consistent experience for users to easily bring their preferred +security tools into GitLab. We ask that our integration partners focus their work on those license +tiers so that we can provide the most value to our mutual customers. + ## What is the GitLab Developer Workflow? This workflow is how GitLab users interact with our product and expect it to diff --git a/doc/development/licensing.md b/doc/development/licensing.md index f92151e7a37..8cda3d5f361 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -12,7 +12,7 @@ Some gems may not include their license information in their `gemspec` file, and ### License Finder commands -> Note: License Finder currently uses GitLab misused terms of whitelist and blacklist. As a result, the commands below references those terms. We've created an [issue on their project](https://github.com/pivotal/LicenseFinder/issues/745) to propose that they rename their commands. +> Note: License Finder currently uses GitLab misused terms of `whitelist` and `blacklist`. As a result, the commands below reference those terms. We've created an [issue on their project](https://github.com/pivotal/LicenseFinder/issues/745) to propose that they rename their commands. There are a few basic commands License Finder provides that you'll need in order to manage license detection. diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 7d3d9dac174..d1f956f957e 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -88,7 +88,7 @@ body: For example: ```ruby -class MyMigration < ActiveRecord::Migration[4.2] +class MyMigration < ActiveRecord::Migration[6.0] DOWNTIME = true DOWNTIME_REASON = 'This migration requires downtime because ...' @@ -411,7 +411,7 @@ migration. For this to work your migration needs to include the module `Gitlab::Database::MultiThreadedMigration`: ```ruby -class MyMigration < ActiveRecord::Migration[4.2] +class MyMigration < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers include Gitlab::Database::MultiThreadedMigration end @@ -421,7 +421,7 @@ You can then use the method `with_multiple_threads` to perform work in separate threads. For example: ```ruby -class MyMigration < ActiveRecord::Migration[4.2] +class MyMigration < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers include Gitlab::Database::MultiThreadedMigration @@ -455,7 +455,7 @@ by calling the method `disable_ddl_transaction!` in the body of your migration class like so: ```ruby -class MyMigration < ActiveRecord::Migration[4.2] +class MyMigration < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers disable_ddl_transaction! @@ -495,9 +495,11 @@ by calling the method `disable_ddl_transaction!` in the body of your migration class like so: ```ruby -class MyMigration < ActiveRecord::Migration[4.2] +class MyMigration < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers + DOWNTIME = false + disable_ddl_transaction! def up @@ -534,7 +536,7 @@ Here's an example where we add a new column with a foreign key constraint. Note it includes `index: true` to create an index for it. ```ruby -class Migration < ActiveRecord::Migration[4.2] +class Migration < ActiveRecord::Migration[6.0] def change add_reference :model, :other_model, index: true, foreign_key: { on_delete: :cascade } @@ -602,7 +604,8 @@ In this particular case, the default value exists and we're just changing the me `request_access_enabled` column, which does not imply a rewrite of all the existing records in the `namespaces` table. Only when creating a new column with a default, all the records are going be rewritten. -NOTE: **Note:** A faster [ALTER TABLE ADD COLUMN with a non-null default](https://www.depesz.com/2018/04/04/waiting-for-postgresql-11-fast-alter-table-add-column-with-a-non-null-default/) +NOTE: **Note:** +A faster [ALTER TABLE ADD COLUMN with a non-null default](https://www.depesz.com/2018/04/04/waiting-for-postgresql-11-fast-alter-table-add-column-with-a-non-null-default/) was introduced on PostgresSQL 11.0, removing the need of rewriting the table when a new column with a default value is added. For the reasons mentioned above, it's safe to use `change_column_default` in a single-transaction migration @@ -808,6 +811,14 @@ class BuildMetadata end ``` +When using a `JSONB` column, use the [JsonSchemaValidator](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/validators/json_schema_validator.rb) to keep control of the data being inserted over time. + +```ruby +class BuildMetadata + validates :config_options, json_schema: { filename: 'build_metadata_config_option' } +end +``` + ## Testing See the [Testing Rails migrations](testing_guide/testing_migrations_guide.md) style guide. @@ -844,16 +855,33 @@ If you need more complex logic, you can define and use models local to a migration. For example: ```ruby -class MyMigration < ActiveRecord::Migration[4.2] +class MyMigration < ActiveRecord::Migration[6.0] class Project < ActiveRecord::Base self.table_name = 'projects' end + + def up + # Reset the column information of all the models that update the database + # to ensure the Active Record's knowledge of the table structure is current + Project.reset_column_information + + # ... ... + end end ``` When doing so be sure to explicitly set the model's table name, so it's not derived from the class name or namespace. +Finally, make sure that `reset_column_information` is run in the `up` method of +the migration for all local Models that update the database. + +The reason for that is that all migration classes are loaded at the beginning +(when `db:migrate` starts), so they can get out of sync with the table schema +they map to in case another migration updates that schema. That makes the data +migration fail when trying to insert or make updates to the underlying table, +as the new columns are reported as `unknown attribute` by `ActiveRecord`. + ### Renaming reserved paths When a new route for projects is introduced, it could conflict with any diff --git a/doc/development/multi_version_compatibility.md b/doc/development/multi_version_compatibility.md index 001517d44ea..aedd5c1ffb7 100644 --- a/doc/development/multi_version_compatibility.md +++ b/doc/development/multi_version_compatibility.md @@ -45,7 +45,7 @@ and set this column to `false`. The old servers were still updating the old colu that updated the new column from the old one. For the new servers though, they were only updating the new column and that same trigger was now working against us and setting it back to the wrong value. -For more information, see this [confidential issue](../user/project/issues/confidential_issues.md) `https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/9176`. +For more information, see [the relevant issue](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/9176). ### Sidebar wasn't loading for some users diff --git a/doc/development/new_fe_guide/development/accessibility.md b/doc/development/new_fe_guide/development/accessibility.md index f76fd72d4dc..b9ee5c3a549 100644 --- a/doc/development/new_fe_guide/development/accessibility.md +++ b/doc/development/new_fe_guide/development/accessibility.md @@ -43,4 +43,4 @@ In forms we should use the `for` attribute in the label statement: - [Chrome Accessibility Developer Tools](https://github.com/GoogleChrome/accessibility-developer-tools) for testing accessibility - [Audit Rules Page](https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules) for best practices -- [Lighthouse Accessibility Score](https://developers.google.com/web/tools/lighthouse/scoring#a11y) for accessibility audits +- [Lighthouse Accessibility Score](https://web.dev/performance-scoring/) for accessibility audits diff --git a/doc/development/ordering_table_columns.md b/doc/development/ordering_table_columns.md index b68602ea30d..18788d0b86e 100644 --- a/doc/development/ordering_table_columns.md +++ b/doc/development/ordering_table_columns.md @@ -1,5 +1,10 @@ # Ordering Table Columns in PostgreSQL +For GitLab we require that columns of new tables are ordered to use the +least amount of space. An easy way of doing this is to order them based on the +type size in descending order with variable sizes (`text`, `varchar`, arrays, +`json`, `jsonb`, and so on) at the end. + Similar to C structures the space of a table is influenced by the order of columns. This is because the size of columns is aligned depending on the type of the following column. Let's consider an example: @@ -40,10 +45,9 @@ In these examples, the `id` and `user_id` columns are packed together, which means we only need 8 bytes to store _both_ of them. This in turn means each row will require 8 bytes less space. -For GitLab we require that columns of new tables are ordered based to use the -least amount of space. An easy way of doing this is to order them based on the -type size in descending order with variable sizes (`text`, `varchar`, arrays, -`json`, `jsonb`, and so on) at the end. +Note: **NOTE:** +Since Ruby on Rails 5.1, the default data type for IDs is `bigint`, which uses 8 bytes. +We are using `integer` in the examples to showcase a more realistic reordering scenario. ## Type Sizes diff --git a/doc/development/packages.md b/doc/development/packages.md index edf01255e84..ae67af10d88 100644 --- a/doc/development/packages.md +++ b/doc/development/packages.md @@ -68,7 +68,8 @@ The current state of existing package registries availability is: | Go | Yes | No - [open issue](https://gitlab.com/gitlab-org/gitlab/-/issues/213900) | No - [open-issue](https://gitlab.com/gitlab-org/gitlab/-/issues/213902) | | Composer | Yes | Yes | No | -NOTE: **Note:** NPM is currently a hybrid of the instance level and group level. +NOTE: **Note:** +NPM is currently a hybrid of the instance level and group level. It is using the top-level group or namespace as the defining portion of the name (for example, `@my-group-name/my-package-name`). diff --git a/doc/development/performance.md b/doc/development/performance.md index 69ad524675d..16ea1aa27ff 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -36,7 +36,6 @@ graphs/dashboards. GitLab provides built-in tools to help improve performance and availability: - [Profiling](profiling.md). - - [Sherlock](profiling.md#sherlock). - [Distributed Tracing](distributed_tracing.md) - [GitLab Performance Monitoring](../administration/monitoring/performance/index.md). - [Request Profiling](../administration/monitoring/performance/request_profiling.md). @@ -108,16 +107,24 @@ In short: ## Profiling By collecting snapshots of process state at regular intervals, profiling allows -you to see where time is spent in a process. The [StackProf](https://github.com/tmm1/stackprof) -gem is included in GitLab's development environment, allowing you to investigate -the behavior of suspect code in detail. +you to see where time is spent in a process. The +[Stackprof](https://github.com/tmm1/stackprof) gem is included in GitLab, +allowing you to profile which code is running on CPU in detail. -It's important to note that profiling an application *alters its performance*, -and will generally be done *in an unrepresentative environment*. In particular, -a method is not necessarily troublesome just because it's executed many times, -or takes a long time to execute. Profiles are tools you can use to better -understand what is happening in an application - using that information wisely -is up to you! +It's important to note that profiling an application *alters its performance*. +Different profiling strategies have different overheads. Stackprof is a sampling +profiler. It will sample stack traces from running threads at a configurable +frequency (e.g. 100hz, that is 100 stacks per second). This type of profiling +has quite a low (albeit non-zero) overhead and is generally considered to be +safe for production. + +### Development + +A profiler can be a very useful tool during development, even if it does run *in +an unrepresentative environment*. In particular, a method is not necessarily +troublesome just because it's executed many times, or takes a long time to +execute. Profiles are tools you can use to better understand what is happening +in an application - using that information wisely is up to you! Keeping that in mind, to create a profile, identify (or create) a spec that exercises the troublesome code path, then run it using the `bin/rspec-stackprof` @@ -166,11 +173,30 @@ dot -Tsvg project_policy_spec.dot > project_policy_spec.svg To load the profile in [kcachegrind](https://kcachegrind.github.io/): ```shell -stackprof tmp/project_policy_spec.dump --callgrind > project_policy_spec.callgrind +stackprof tmp/project_policy_spec.rb.dump --callgrind > project_policy_spec.callgrind kcachegrind project_policy_spec.callgrind # Linux qcachegrind project_policy_spec.callgrind # Mac ``` +For flamegraphs, enable raw collection first. Note that raw +collection can generate a very large file, so increase the `INTERVAL`, or +run on a smaller number of specs for smaller file size: + +```shell +RAW=true bin/rspec-stackprof spec/policies/group_member_policy_spec.rb +``` + +You can then generate, and view the resultant flamegraph. It might take a +while to generate based on the output file size: + +```shell +# Generate +stackprof --flamegraph tmp/group_member_policy_spec.rb.dump > group_member_policy_spec.flame + +# View +stackprof --flamegraph-viewer=group_member_policy_spec.flame +``` + It may be useful to zoom in on a specific method, for example: ```shell @@ -211,11 +237,57 @@ application code, these profiles can be used to investigate slow tests as well. However, for smaller runs (like this example), this means that the cost of setting up the test suite will tend to dominate. -It's also possible to modify the application code in-place to output profiles -whenever a particular code path is triggered without going through the test -suite first. See the -[StackProf documentation](https://github.com/tmm1/stackprof/blob/master/README.md) -for details. +### Production + +Stackprof can also be used to profile production workloads. + +In order to enable production profiling for Ruby processes, you can set the `STACKPROF_ENABLED` environment variable to `true`. + +The following configuration options can be configured: + +- `STACKPROF_ENABLED`: Enables stackprof signal handler on SIGUSR2 signal. + Defaults to `false`. +- `STACKPROF_INTERVAL_US`: Sampling interval in microseconds. Defaults to + `10000` μs (100hz). +- `STACKPROF_FILE_PREFIX`: File path prefix where profiles are stored. Defaults + to `$TMPDIR` (often corresponds to `/tmp`). +- `STACKPROF_TIMEOUT_S`: Profiling timeout in seconds. Profiling will + automatically stop after this time has elapsed. Defaults to `30`. +- `STACKPROF_RAW`: Whether to collect raw samples or only aggregates. Raw + samples are needed to generate flamegraphs, but they do have a higher memory + and disk overhead. Defaults to `true`. + +Once enabled, profiling can be triggered by sending a `SIGUSR2` signal to the +Ruby process. The process will begin sampling stacks. Profiling can be stopped +by sending another `SIGUSR2`. Alternatively, it will automatically stop after +the timeout. + +Once profiling stops, the profile is written out to disk at +`$STACKPROF_FILE_PREFIX/stackprof.$PID.$RAND.profile`. It can then be inspected +further via the `stackprof` command line tool, as described in the previous +section. + +Currently supported profiling targets are: + +- Puma worker +- Sidekiq + +NOTE: **Note:** +The Puma master process is not supported. Neither is Unicorn. +Sending SIGUSR2 to either of those will trigger restarts. In the case of Puma, +take care to only send the signal to Puma workers. + +This can be done via `pkill -USR2 puma:`. The `:` disambiguates between `puma +4.3.3.gitlab.2 ...` (the master process) from `puma: cluster worker 0: ...` (the +worker processes), selecting the latter. + +Production profiles can be especially noisy. It can be helpful to visualize them +as a [flamegraph](https://github.com/brendangregg/FlameGraph). This can be done +via: + +```shell +bundle exec stackprof --stackcollapse /tmp/stackprof.55769.c6c3906452.profile | flamegraph.pl > flamegraph.svg +``` ## RSpec profiling @@ -254,6 +326,13 @@ These results can also be placed into a PostgreSQL database by setting the `RSPEC_PROFILING_POSTGRES_URL` variable. This is used to profile the test suite when running in the CI environment. +We store these results also when running CI jobs on the default branch on +`gitlab.com`. Statistics of these profiling data are [available +online](https://gitlab-org.gitlab.io/rspec_profiling_stats/). For example, +you can find which tests take longest to run or which execute the most +queries. This can be handy for optimizing our tests or identifying performance +issues in our code. + ## Memory profiling One of the reasons of the increased memory footprint could be Ruby memory fragmentation. diff --git a/doc/development/permissions.md b/doc/development/permissions.md index 06a4a03de38..e930345caec 100644 --- a/doc/development/permissions.md +++ b/doc/development/permissions.md @@ -13,9 +13,16 @@ Groups and projects can have the following visibility levels: - internal (`10`) - an entity is visible to logged in users - private (`0`) - an entity is visible only to the approved members of the entity +By default, subgroups can **not** have higher visibility levels. +For example, if you create a new private group, it can not include a public subgroup. + The visibility level of a group can be changed only if all subgroups and -sub-projects have the same or lower visibility level. (e.g., a group can be set -to internal only if all subgroups and projects are internal or private). +sub-projects have the same or lower visibility level. For example, a group can be set +to internal only if all subgroups and projects are internal or private. + +CAUTION: **Warning:** +If you migrate an existing group to a lower visibility level, that action does not migrate subgroups +in the same way. This is a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/22406). Visibility levels can be found in the `Gitlab::VisibilityLevel` module. @@ -48,10 +55,10 @@ levels are available (defined in the `Gitlab::Access` module): - Maintainer (`40`) - Owner (`50`) -If a user is the member of both a project and the project parent group, the +If a user is the member of both a project and the project parent group(s), the higher permission is taken into account for the project. -If a user is the member of a project, but not the parent group (or groups), they +If a user is the member of a project, but not the parent group(s), they can still view the groups and their entities (like epics). Project membership (where the group membership is already taken into account) diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 05b80cdb4a6..d3623529cd4 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -370,24 +370,28 @@ and are as follows: | MRs | 11 | | `master` (non-scheduled pipelines) | 11 | | 2-hourly scheduled pipelines | 11 | +| `nightly` scheduled pipelines | 11, 12 | #### Long-term plan We follow the [PostgreSQL versions shipped with Omnibus GitLab](https://docs.gitlab.com/omnibus/package-information/postgresql_versions.html): -| PostgreSQL version | 12.10 (April 2020) | 13.0 (May 2020) | 13.1 (June 2020) | 13.2 (July 2020) | 13.3 (August 2020) | 13.4, 13.5 | 13.6 (November 2020) | 14.0 (May 2021?) | -| ------ | ------------------ | --------------- | ---------------- | ---------------- | ------------------ | ------------ | -------------------- | ---------------- | -| PG9.6 | MRs/`master`/`2-hour`/`nightly` | - | - | - | - | - | - | - | -| PG10 | `nightly` | - | - | - | - | - | - | - | -| PG11 | `master`/`2-hour` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | `nightly` | - | -| PG12 | - | - | - | - | `master`/`2-hour` | `master`/`2-hour` | MRs/`master`/`2-hour`/`nightly` | `master`/`2-hour` | -| PG13 | - | - | - | - | - | - | - | MRs/`master`/`2-hour`/`nightly` | +| PostgreSQL version | 13.0 (May 2020) | 13.1 (June 2020) | 13.2 (July 2020) | 13.3 (August 2020) | 13.4, 13.5 | 13.6 (November 2020) | 14.0 (May 2021?) | +| ------ | --------------- | ---------------- | ---------------- | ------------------ | ------------ | -------------------- | ---------------- | +| PG11 | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | `nightly` | - | +| PG12 | - | - | `nightly` | `2-hour`/`nightly` | `2-hour`/`nightly` | MRs/`2-hour`/`nightly` | `2-hour`/`nightly` | +| PG13 | - | - | - | - | - | - | MRs/`2-hour`/`nightly` | ### Test jobs Consult [GitLab tests in the Continuous Integration (CI) context](testing_guide/ci.md) for more information. +We have dedicated jobs for each [testing level](testing_guide/testing_levels.md) and each job runs depending on the +changes made in your merge request. +If you want to force all the RSpec jobs to run regardless of your changes, you can include `RUN ALL RSPEC` in your merge +request title. + ### Review app jobs Consult the [Review Apps](testing_guide/review_apps.md) dedicated page for more information. diff --git a/doc/development/policies.md b/doc/development/policies.md index 62442de825a..8f05948cb41 100644 --- a/doc/development/policies.md +++ b/doc/development/policies.md @@ -158,6 +158,89 @@ end will include all rules from `ProjectPolicy`. The delegated conditions will be evaluated with the correct delegated subject, and will be sorted along with the regular rules in the policy. Note that only the relevant rules for a particular ability will actually be considered. +### Overrides + +We allow policies to opt-out of delegated abilities. + +Delegated policies may define some abilities in a way that is incorrect for the +delegating policy. Take for example a child/parent relationship, where some +abilities can be inferred, and some cannot: + +```ruby +class ParentPolicy < BasePolicy + condition(:speaks_spanish) { @subject.spoken_languages.include?(:es) } + condition(:has_license) { @subject.driving_license.present? } + condition(:enjoys_broccoli) { @subject.enjoyment_of(:broccoli) > 0 } + + rule { speaks_spanish }.enable :read_spanish + rule { has_license }.enable :drive_car + rule { enjoys_broccoli }.enable :eat_broccoli + rule { ~enjoys_broccoli }.prevent :eat_broccoli +end +``` + +Here, if we delegated the child policy to the parent policy, some values would be +incorrect - we might correctly infer that the child can speak their parent's +language, but it would be incorrect to infer that the child can drive or would +eat broccoli just because the parent can and does. + +Some of these things we can deal with - we can forbid driving universally in the +child policy, for example: + +```ruby +class ChildPolicy < BasePolicy + delegate { @subject.parent } + + rule { default }.prevent :drive_car +end +``` + +But the food preferences one is harder - because of the `prevent` call in the +parent policy, if the parent dislikes it, even calling `enable` in the child +will not enable `:eat_broccoli`. + +We could remove the `prevent` call in the parent policy, but that still doesn't +help us, since the rules are different: parents get to eat what they like, and +children eat what they are given, provided they are well behaved. Allowing +delegation would end up with only children whose parents enjoy green vegetables +eating it. But a parent may well give their child broccoli, even if they dislike +it themselves, because it is good for their child. + +The solution it to override the `:eat_broccoli` ability in the child policy: + +```ruby +class ChildPolicy < BasePolicy + delegate { @subject.parent } + + overrides :eat_broccoli + + condition(:good_kid) { @subject.behavior_level >= Child::GOOD } + + rule { good_kid }.enable :eat_broccoli +end +``` + +With this definition, the `ChildPolicy` will _never_ look in the `ParentPolicy` to +satisfy `:eat_broccoli`, but it _will_ use it for any other abilities. The child +policy can then define `:eat_broccoli` in a way that makes sense for `Child` and not +`Parent`. + +### Alternatives to using `overrides` + +Overriding policy delegation is complex, for the same reason delegation is +complex - it involves reasoning about logical inference, and being clear about +semantics. Misuse of `override` has the potential to duplicate code, and +potentially introduce security bugs, allowing things that should be prevented. +For this reason, it should be used only when other approaches are not feasible. + +Other approaches can include for example using different ability names. Choosing +to eat a food and eating foods you are given are semantically distinct, and they +could be named differently (perhaps `chooses_to_eat_broccoli` and +`eats_what_is_given` in this case). It can depend on how polymorphic the call +site is. If you know that we will always check the policy with a `Parent` or a +`Child`, then we can choose the appropriate ability name. If the call site is +polymorphic, then we cannot do that. + ## Specifying Policy Class You can also override the Policy used for a given subject: diff --git a/doc/development/profiling.md b/doc/development/profiling.md index 2cab6750b9b..f7ead94d725 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -10,7 +10,8 @@ There is a `Gitlab::Profiler.profile` method, and corresponding `bin/profile-url` script, that enable profiling a GET or POST request to a specific URL, either as an anonymous user (the default) or as a specific user. -NOTE: **Note:** The first argument to the profiler is either a full URL +NOTE: **Note:** +The first argument to the profiler is either a full URL (including the instance hostname) or an absolute path, including the leading slash. diff --git a/doc/development/prometheus_metrics.md b/doc/development/prometheus_metrics.md index 004b1884bf0..024da5cc943 100644 --- a/doc/development/prometheus_metrics.md +++ b/doc/development/prometheus_metrics.md @@ -11,16 +11,16 @@ The requirement for adding a new metric is to make each query to have an unique ```yaml - group: Response metrics (NGINX Ingress) metrics: - - title: "Throughput" - y_axis: - name: "Requests / Sec" - format: "number" - precision: 2 - queries: - - id: response_metrics_nginx_ingress_throughput_status_code - query_range: 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)' - unit: req / sec - label: Status Code + - title: "Throughput" + y_axis: + name: "Requests / Sec" + format: "number" + precision: 2 + queries: + - id: response_metrics_nginx_ingress_throughput_status_code + query_range: 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)' + unit: req / sec + label: Status Code ``` ### Update existing metrics diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index b28fdb1252b..f7d2c23e28d 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -115,6 +115,6 @@ There are multiple ways to find the source of queries. ## See also -- [Bullet](profiling.md#Bullet) For finding `N+1` query problems +- [Bullet](profiling.md#bullet) For finding `N+1` query problems - [Performance guidelines](performance.md) - [Merge request performance guidelines](merge_request_performance_guidelines.md#query-counts) diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index 3fa6ba40e6c..fd5dee69fc3 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -232,6 +232,9 @@ To see the full list of API routes, you can run: bundle exec rake grape:path_helpers ``` +The generated list includes a full list of API endpoints and functional +RESTful API verbs. + For the Rails controllers, run: ```shell diff --git a/doc/development/redis.md b/doc/development/redis.md index 6782ea96448..693b9e1ad0d 100644 --- a/doc/development/redis.md +++ b/doc/development/redis.md @@ -33,6 +33,8 @@ stop being consulted if the project is renamed. If the contents of the key are invalidated by a name change, it is better to include a hook that will expire the entry, instead of relying on the key changing. +### Multi-key commands + We don't use [Redis Cluster](https://redis.io/topics/cluster-tutorial) at the moment, but may wish to in the future: [#118820](https://gitlab.com/gitlab-org/gitlab/-/issues/118820). @@ -41,3 +43,8 @@ operations that require several keys to be held on the same Redis server - for instance, diffing two sets held in Redis - the keys should ensure that by enclosing the changeable parts in curly braces, such as, `project:{1}:set_a` and `project:{1}:set_b`. + +Currently, we validate this in the development and test environments +with the [`RedisClusterValidator`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/instrumentation/redis_cluster_validator.rb), +which is enabled for the `cache` and `shared_state` +[Redis instances](https://docs.gitlab.com/omnibus/settings/redis.html#running-with-multiple-redis-instances).. diff --git a/doc/development/scalability.md b/doc/development/scalability.md index c0c26df88b5..0fb54d89913 100644 --- a/doc/development/scalability.md +++ b/doc/development/scalability.md @@ -115,8 +115,7 @@ that backup, the database can apply the WAL logs in order until the database has reached the target time. On GitLab.com, Consul and Patroni work together to coordinate failovers with -the read replicas. [Omnibus ships with repmgr instead of -Patroni](../administration/postgresql/replication_and_failover.md). +the read replicas. [Omnibus ships with both repmgr and Patroni](../administration/postgresql/replication_and_failover.md). #### Load-balancing diff --git a/doc/development/secure_coding_guidelines.md b/doc/development/secure_coding_guidelines.md index 912b8fbf043..65953620ce6 100644 --- a/doc/development/secure_coding_guidelines.md +++ b/doc/development/secure_coding_guidelines.md @@ -213,7 +213,7 @@ the mitigations for a new feature. #### Feature-specific Mitigations -For situations in which an allowlist or GitLab:HTTP cannot be used, it will be necessary to implement mitigations directly in the feature. It is best to validate the destination IP addresses themselves, not just domain names, as DNS can be controlled by the attacker. Below are a list of mitigations that should be implemented. +For situtions in which an allowlist or GitLab:HTTP cannot be used, it will be necessary to implement mitigations directly in the feature. It is best to validate the destination IP addresses themselves, not just domain names, as DNS can be controlled by the attacker. Below are a list of mitigations that should be implemented. **Important Note:** There are many tricks to bypass common SSRF validations. If feature-specific mitigations are necessary, they should be reviewed by the AppSec team, or a developer who has worked on SSRF mitigations previously. @@ -278,6 +278,7 @@ For any and all input fields, ensure to define expectations on the type/format o - Validate the [input size limits](https://youtu.be/2VFavqfDS6w?t=7582). - Validate the input using an [allowlist approach](https://youtu.be/2VFavqfDS6w?t=7816) to only allow characters through which you are expecting to receive for the field. - Input which fails validation should be **rejected**, and not sanitized. +- When adding redirects or links to a user-controlled URL, ensure that the scheme is HTTP or HTTPS. Allowing other schemes like `javascript://` can lead to XSS and other security issues. Note that denylists should be avoided, as it is near impossible to block all [variations of XSS](https://owasp.org/www-community/xss-filter-evasion-cheatsheet). @@ -292,40 +293,60 @@ Once you've [determined when and where](#setting-expectations) the user submitte ### Additional info -#### Mitigating XSS in Rails +#### XSS mitigation and prevention in Rails + +By default, Rails automatically escapes strings when they are inserted into HTML templates. Avoid the +methods used to keep Rails from escaping strings, especially those related to user-controlled values. +Specifically, the following options are dangerous because they mark strings as trusted and safe: + +| Method | Avoid these options | +|----------------------|-------------------------------| +| HAML templates | `html_safe`, `raw`, `!=` | +| Embedded Ruby (ERB) | `html_safe`, `raw`, `<%== %>` | +In case you want to sanitize user-controlled values against XSS vulnerabilities, you can use +[`ActionView::Helpers::SanitizeHelper`](https://api.rubyonrails.org/classes/ActionView/Helpers/SanitizeHelper.html). +Calling `link_to` and `redirect_to` with user-controlled parameters can also lead to cross-site scripting. + +Do also sanitize and validate URL schemes. + +References: - [XSS Defense in Rails](https://youtu.be/2VFavqfDS6w?t=2442) - [XSS Defense with HAML](https://youtu.be/2VFavqfDS6w?t=2796) - [Validating Untrusted URLs in Ruby](https://youtu.be/2VFavqfDS6w?t=3936) - [RoR Model Validators](https://youtu.be/2VFavqfDS6w?t=7636) +#### XSS mitigation and prevention in JavaScript and Vue + +- When updating the content of an HTML element using JavaScript, mark user-controlled values as `textContent` or `nodeValue` instead of `innerHTML`. +- Avoid using `v-html` with user-controlled data, use [`v-safe-html`](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/directives-safe-html-directive--default) instead. +- Consider using [`gl-sprintf`](../../ee/development/i18n/externalization.md#interpolation) to interpolate translated strings securely. +- Avoid `__()` with translations that contain user-controlled values. +- When working with `postMessage`, ensure the `origin` of the message is allowlisted. +- Consider using the [Safe Link Directive](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/directives-safe-link-directive--default) to generate secure hyperlinks by default. + #### GitLab specific libraries for mitigating XSS ##### Vue - [isSafeURL](https://gitlab.com/gitlab-org/gitlab/-/blob/v12.7.5-ee/app/assets/javascripts/lib/utils/url_utility.js#L190-207) +- [GlSprintf](https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/utilities-sprintf--default) #### Content Security Policy - [Content Security Policy](https://www.youtube.com/watch?v=2VFavqfDS6w&t=12991s) - [Use nonce-based Content Security Policy for inline JavaScript](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/65330) -#### Free form input fields - -##### Sanitization - -- [HTML Sanitization](https://youtu.be/2VFavqfDS6w?t=5075) -- [DOMPurify](https://youtu.be/2VFavqfDS6w?t=5381) - -##### `iframe` sandboxes - -- [iframe sandboxing](https://youtu.be/2VFavqfDS6w?t=7043) +#### Free form input field ### Select examples of past XSS issues affecting GitLab -- [Stored XSS in user status](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/55320) +- [Stored XSS in user status](https://gitlab.com/gitlab-org/gitlab-foss/issues/55320) +- [XSS vulnerability on custom project templates form](https://gitlab.com/gitlab-org/gitlab/issues/197302) +- [Stored XSS in branch names](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/55320) +- [Stored XSS in merge request pages](https://gitlab.com/gitlab-org/gitlab/-/issues/35096) -### Developer Training +### Internal Developer Training - [Introduction to XSS](https://www.youtube.com/watch?v=PXR8PTojHmc&t=7785s) - [Reflected XSS](https://youtu.be/2VFavqfDS6w?t=603s) @@ -347,3 +368,29 @@ Once you've [determined when and where](#setting-expectations) the user submitte - [RoR model validators](https://youtu.be/2VFavqfDS6w?t=7636) - [Allowlist input validation](https://youtu.be/2VFavqfDS6w?t=7816) - [Content Security Policy](https://www.youtube.com/watch?v=2VFavqfDS6w&t=12991s) + +## Path Traversal guidelines + +### Description + +Path Traversal vulnerabilities grant attackers access to arbitrary directories and files on the server that is executing an application, including data, code or credentials. + +### Impact + +Path Traversal attacks can lead to multiple critical and high severity issues, like arbitrary file read, remote code execution or information disclosure. + +### When to consider + +When working with user-controlled filenames/paths and filesystem APIs. + +### Mitigation and prevention + +In order to prevent Path Traversal vulnerabilities, user-controlled filenames or paths should be validated before being processed. + +- Comparing user input against an allowlist of allowed values or verifying that it only contains allowed characters. +- After validating the user supplied input, it should be appended to the base directory and the path should be canonicalized using the filesystem API. + +#### GitLab specific validations + +- [`Gitlab::Utils.check_path_traversal`](https://gitlab.com/gitlab-org/security/gitlab/-/blob/master/lib/gitlab/utils.rb#L12-24) can be used to validate user input against Path Traversal vulnerabilities. Remember to add further validation when setting the `allowed_absolute` option to `true`. +- [`file_path` API validator](https://gitlab.com/gitlab-org/security/gitlab/-/blob/master/lib/api/validations/validators/file_path.rb) to validate user input when working with the Grape gem. diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 7ae3c9e9de2..2793756ff64 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -175,9 +175,9 @@ Jobs can have an `urgency` attribute set, which can be `:high`, | **Urgency** | **Queue Scheduling Target** | **Execution Latency Requirement** | |--------------|-----------------------------|------------------------------------| -| `:high` | 100 milliseconds | p50 of 1 second, p99 of 10 seconds | -| `:low` | 1 minute | Maximum run time of 1 hour | -| `:throttled` | None | Maximum run time of 1 hour | +| `:high` | 10 seconds | p50 of 1 second, p99 of 10 seconds | +| `:low` | 1 minute | Maximum run time of 5 minutes | +| `:throttled` | None | Maximum run time of 5 minutes | To set a job's urgency, use the `urgency` class method: @@ -225,6 +225,47 @@ work between two different workers, one with `urgency :high` code that executes quickly, and the other with `urgency :low`, which has no execution latency requirements (but also has lower scheduling targets). +### Changing a queue's urgency + +On GitLab.com, we run Sidekiq in several +[shards](https://dashboards.gitlab.net/d/sidekiq-shard-detail/sidekiq-shard-detail), +each of which represents a particular type of workload. + +When changing a queue's urgency, or adding a new queue, we need to take +into account the expected workload on the new shard. Note that, if we're +changing an existing queue, there is also an effect on the old shard, +but that will always be a reduction in work. + +To do this, we want to calculate the expected increase in total execution time +and RPS (throughput) for the new shard. We can get these values from: + +- The [Queue Detail + dashboard](https://dashboards.gitlab.net/d/sidekiq-queue-detail/sidekiq-queue-detail) + has values for the queue itself. For a new queue, we can look for + queues that have similar patterns or are scheduled in similar + circumstances. +- The [Shard Detail + dashboard](https://dashboards.gitlab.net/d/sidekiq-shard-detail/sidekiq-shard-detail) + has Total Execution Time and Throughput (RPS). The Shard Utilization + panel will show if there is currently any excess capacity for this + shard. + +We can then calculate the RPS * average runtime (estimated for new jobs) +for the queue we're changing to see what the relative increase in RPS and +execution time we expect for the new shard: + +```ruby +new_queue_consumption = queue_rps * queue_duration_avg +shard_consumption = shard_rps * shard_duration_avg + +(new_queue_consumption / shard_consumption) * 100 +``` + +If we expect an increase of **less than 5%**, then no further action is needed. + +Otherwise, please ping `@gitlab-org/scalability` on the merge request and ask +for a review. + ## Jobs with External Dependencies Most background jobs in the GitLab application communicate with other GitLab @@ -262,7 +303,8 @@ class ExternalDependencyWorker end ``` -NOTE: **Note:** Note that a job cannot be both high urgency and have +NOTE: **Note:** +Note that a job cannot be both high urgency and have external dependencies. ## CPU-bound and Memory-bound Workers @@ -337,55 +379,10 @@ We use the following approach to determine whether a worker is CPU-bound: - Note that these values should not be used over small sample sizes, but rather over fairly large aggregates. -## Feature Categorization - -Each Sidekiq worker, or one of its ancestor classes, must declare a -`feature_category` attribute. This attribute maps each worker to a feature -category. This is done for error budgeting, alert routing, and team attribution -for Sidekiq workers. - -The declaration uses the `feature_category` class method, as shown below. - -```ruby -class SomeScheduledTaskWorker - include ApplicationWorker - - # Declares that this worker is part of the - # `continuous_integration` feature category - feature_category :continuous_integration - - # ... -end -``` - -The list of value values can be found in the file `config/feature_categories.yml`. -This file is, in turn generated from the [`stages.yml` from the GitLab Company Handbook -source](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml). - -### Updating `config/feature_categories.yml` - -Occasionally new features will be added to GitLab stages. When this occurs, you -can automatically update `config/feature_categories.yml` by running -`scripts/update-feature-categories`. This script will fetch and parse -[`stages.yml`](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml) -and generate a new version of the file, which needs to be checked into source control. - -### Excluding Sidekiq workers from feature categorization +## Feature category -A few Sidekiq workers, that are used across all features, cannot be mapped to a -single category. These should be declared as such using the `feature_category_not_owned!` - declaration, as shown below: - -```ruby -class SomeCrossCuttingConcernWorker - include ApplicationWorker - - # Declares that this worker does not map to a feature category - feature_category_not_owned! - - # ... -end -``` +All Sidekiq workers must define a known [feature +category](feature_categorization/index.md#sidekiq-workers). ## Job weights @@ -401,6 +398,8 @@ default weight, which is 1. ## Worker context +> - [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/9) in GitLab 12.8. + To have some more information about workers in the logs, we add [metadata to the jobs in the form of an `ApplicationContext`](logging.md#logging-context-metadata-through-rails-or-grape-requests). @@ -417,27 +416,27 @@ need to do anything. There are however some instances when there would be no context present when the job is scheduled, or the context that is present is -likely to be incorrect. For these instances we've added rubocop-rules +likely to be incorrect. For these instances, we've added Rubocop rules to draw attention and avoid incorrect metadata in our logs. As with most our cops, there are perfectly valid reasons for disabling them. In this case it could be that the context from the request is correct. Or maybe you've specified a context already in a way that -isn't picked up by the cops. In any case, please leave a code-comment +isn't picked up by the cops. In any case, leave a code comment pointing to which context will be used when disabling the cops. -When you do provide objects to the context, please make sure that the -route for namespaces and projects is pre-loaded. This can be done using +When you do provide objects to the context, make sure that the +route for namespaces and projects is pre-loaded. This can be done by using the `.with_route` scope defined on all `Routable`s. -### Cron-Workers +### Cron workers -The context is automatically cleared for workers in the cronjob-queue -(which `include CronjobQueue`), even when scheduling them from +The context is automatically cleared for workers in the Cronjob queue +(`include CronjobQueue`), even when scheduling them from requests. We do this to avoid incorrect metadata when other jobs are -scheduled from the cron-worker. +scheduled from the cron worker. -Cron-Workers themselves run instance wide, so they aren't scoped to +Cron workers themselves run instance wide, so they aren't scoped to users, namespaces, projects, or other resources that should be added to the context. @@ -449,46 +448,46 @@ somewhere within the worker: 1. Wrap the code that schedules jobs in the `with_context` helper: -```ruby - def perform - deletion_cutoff = Gitlab::CurrentSettings - .deletion_adjourned_period.days.ago.to_date - projects = Project.with_route.with_namespace - .aimed_for_deletion(deletion_cutoff) + ```ruby + def perform + deletion_cutoff = Gitlab::CurrentSettings + .deletion_adjourned_period.days.ago.to_date + projects = Project.with_route.with_namespace + .aimed_for_deletion(deletion_cutoff) - projects.find_each(batch_size: 100).with_index do |project, index| - delay = index * INTERVAL + projects.find_each(batch_size: 100).with_index do |project, index| + delay = index * INTERVAL - with_context(project: project) do - AdjournedProjectDeletionWorker.perform_in(delay, project.id) - end - end - end -``` + with_context(project: project) do + AdjournedProjectDeletionWorker.perform_in(delay, project.id) + end + end + end + ``` 1. Use the a batch scheduling method that provides context: -```ruby - def schedule_projects_in_batch(projects) - ProjectImportScheduleWorker.bulk_perform_async_with_contexts( - projects, - arguments_proc: -> (project) { project.id }, - context_proc: -> (project) { { project: project } } - ) - end -``` - -or when scheduling with delays: - -```ruby - diffs.each_batch(of: BATCH_SIZE) do |diffs, index| - DeleteDiffFilesWorker - .bulk_perform_in_with_contexts(index * 5.minutes, - diffs, - arguments_proc: -> (diff) { diff.id }, - context_proc: -> (diff) { { project: diff.merge_request.target_project } }) - end -``` + ```ruby + def schedule_projects_in_batch(projects) + ProjectImportScheduleWorker.bulk_perform_async_with_contexts( + projects, + arguments_proc: -> (project) { project.id }, + context_proc: -> (project) { { project: project } } + ) + end + ``` + + Or, when scheduling with delays: + + ```ruby + diffs.each_batch(of: BATCH_SIZE) do |diffs, index| + DeleteDiffFilesWorker + .bulk_perform_in_with_contexts(index * 5.minutes, + diffs, + arguments_proc: -> (diff) { diff.id }, + context_proc: -> (diff) { { project: diff.merge_request.target_project } }) + end + ``` ### Jobs scheduled in bulk @@ -512,11 +511,11 @@ For example: Each object from the enumerable in the first argument is yielded into 2 blocks: -The `arguments_proc` which needs to return the list of arguments the -job needs to be scheduled with. +- The `arguments_proc` which needs to return the list of arguments the + job needs to be scheduled with. -The `context_proc` which needs to return a hash with the context -information for the job. +- The `context_proc` which needs to return a hash with the context + information for the job. ## Arguments logging @@ -597,7 +596,6 @@ There are two options for safely adding new arguments to Sidekiq workers: 1. Set up a [multi-step deployment](#multi-step-deployment) in which the new argument is first added to the worker 1. Use a [parameter hash](#parameter-hash) for additional arguments. This is perhaps the most flexible option. -1. Use a parameter hash for additional arguments. This is perhaps the most flexible option. ##### Multi-step deployment diff --git a/doc/development/telemetry/index.md b/doc/development/telemetry/index.md index aee16e4049a..0000e7e9e4f 100644 --- a/doc/development/telemetry/index.md +++ b/doc/development/telemetry/index.md @@ -20,6 +20,7 @@ Telemetry Guide: 1. [Our tracking tools](#our-tracking-tools) 1. [What data can be tracked](#what-data-can-be-tracked) 1. [Telemetry systems overview](#telemetry-systems-overview) + 1. [Snowflake data warehouse](#snowflake-data-warehouse) [Usage Ping Guide](usage_ping.md) @@ -44,9 +45,9 @@ Telemetry Guide: More useful links: - [Telemetry Direction](https://about.gitlab.com/direction/telemetry/) -- [Data Analysis Process](https://about.gitlab.com/handbook/business-ops/data-team/#-data-analysis-process) -- [Data for Product Managers](https://about.gitlab.com/handbook/business-ops/data-team/data-for-product-managers/) -- [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/data-infrastructure/) +- [Data Analysis Process](https://about.gitlab.com/handbook/business-ops/data-team/#data-analysis-process/) +- [Data for Product Managers](https://about.gitlab.com/handbook/business-ops/data-team/programs/data-for-product-managers/) +- [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/platform/infrastructure/) ## Our tracking tools @@ -68,7 +69,7 @@ For more details, read the [Usage Ping](usage_ping.md) guide. ### Database import -Database imports are full imports of data into GitLab's data warehouse. For GitLab.com, the PostgreSQL database is loaded into Snowflake data warehouse every 6 hours. For more details, see the [data team handbook](https://about.gitlab.com/handbook/business-ops/data-team/#extract-and-load). +Database imports are full imports of data into GitLab's data warehouse. For GitLab.com, the PostgreSQL database is loaded into Snowflake data warehouse every 6 hours. For more details, see the [data team handbook](https://about.gitlab.com/handbook/business-ops/data-team/platform/#extract-and-load). ### Log system @@ -144,7 +145,7 @@ The systems overview is a simplified diagram showing the interactions between Gi For Telemetry purposes, GitLab Inc has three major components: -1. [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/data-infrastructure/): This contains everything managed by our data team including Sisense Dashboards for visualization, Snowflake for Data Warehousing, incoming data sources such as PostgreSQL Pipeline and S3 Bucket, and lastly our data collectors [GitLab.com's Snowplow Collector](https://about.gitlab.com/handbook/engineering/infrastructure/library/snowplow/) and GitLab's Versions Application. +1. [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/platform/infrastructure/): This contains everything managed by our data team including Sisense Dashboards for visualization, Snowflake for Data Warehousing, incoming data sources such as PostgreSQL Pipeline and S3 Bucket, and lastly our data collectors [GitLab.com's Snowplow Collector](https://about.gitlab.com/handbook/engineering/infrastructure/library/snowplow/) and GitLab's Versions Application. 1. GitLab.com: This is the production GitLab application which is made up of a Client and Server. On the Client or browser side, a Snowplow JS Tracker (Frontend) is used to track client-side events. On the Server or application side, a Snowplow Ruby Tracker (Backend) is used to track server-side events. The server also contains Usage Ping which leverages a PostgreSQL database and a Redis in-memory data store to report on usage data. Lastly, the server also contains System Logs which are generated from running the GitLab application. 1. [Monitoring infrastructure](https://about.gitlab.com/handbook/engineering/monitoring/): This is the infrastructure used to ensure GitLab.com is operating smoothly. System Logs are sent from GitLab.com to our monitoring infrastructure and collected by a FluentD collector. From FluentD, logs are either sent to long term Google Cloud Services cold storage via Stackdriver, or, they are sent to our Elastic Cluster via Cloud Pub/Sub which can be explored in real-time using Kibana. @@ -169,3 +170,19 @@ The differences between GitLab.com and self-managed are summarized below: | Self-Managed | **{dotted-circle}**(1) | **{dotted-circle}**(1) | **{check-circle}** | **{dotted-circle}** | **{dotted-circle}** | Note (1): Snowplow JS and Snowplow Ruby are available on self-managed, however, the Snowplow Collector endpoint is set to a self-managed Snowplow Collector which GitLab Inc does not have access to. + +## Snowflake data warehouse + +The Snowflake data warehouse is where we keep all of GitLab Inc's data. + +### Data sources + +There are several data sources available in Snowflake and Sisense each representing a different view of the data along the transformation pipeline. + +| Source | Description | Access | +| ------ | ------ | ------ | +| raw | These tables are the raw data source | Access via Snowflake | +| analytics_staging | These tables have undergone little to no data transformation, meaning they're basically clones of the raw data source | Access via Snowflake or Sisense | +| analytics | These tables have typically undergone more data transformation. They will typically end in `_xf` to represent the fact that they are transformed | Access via Snowflake or Sisense | + +If you are a Product Manager interested in the raw data, you will likely focus on the `analytics` and `analytics_staging` sources. The raw source is limited to the data and infrastructure teams. For more information, please see [Data For Product Managers: What's the difference between analytics_staging and analytics?](https://about.gitlab.com/handbook/business-ops/data-team/programs/data-for-product-managers/#whats-the-difference-between-analytics_staging-and-analytics) diff --git a/doc/development/telemetry/snowplow.md b/doc/development/telemetry/snowplow.md index b7090ee4d20..f03742afe2d 100644 --- a/doc/development/telemetry/snowplow.md +++ b/doc/development/telemetry/snowplow.md @@ -6,15 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Snowplow Guide -This guide provides a details about how Snowplow works. It includes the following sections: - -1. [What is Snowplow](#what-is-snowplow) -1. [Snowplow schema](#snowplow-schema) -1. [Enabling Snowplow](#enabling-snowplow) -1. [Snowplow request flow](#snowplow-request-flow) -1. [Implementing Snowplow JS (Frontend) tracking](#implementing-snowplow-js-frontend-tracking) -1. [Implementing Snowplow Ruby (Backend) tracking](#implementing-snowplow-ruby-backend-tracking) -1. [Developing and testing Snowplow](#developing-and-testing-snowplow) +This guide provides an overview of how Snowplow works, and implementation details. For more information about Telemetry, see: @@ -24,32 +16,31 @@ For more information about Telemetry, see: More useful links: - [Telemetry Direction](https://about.gitlab.com/direction/telemetry/) -- [Data Analysis Process](https://about.gitlab.com/handbook/business-ops/data-team/#-data-analysis-process) -- [Data for Product Managers](https://about.gitlab.com/handbook/business-ops/data-team/data-for-product-managers/) -- [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/data-infrastructure/) +- [Data Analysis Process](https://about.gitlab.com/handbook/business-ops/data-team/#data-analysis-process/) +- [Data for Product Managers](https://about.gitlab.com/handbook/business-ops/data-team/programs/data-for-product-managers/) +- [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/platform/infrastructure/) ## What is Snowplow Snowplow is an enterprise-grade marketing and product analytics platform which helps track the way users engage with our website and application. -From [Snowplow's documentation](https://github.com/snowplow/snowplow), Snowplow consists of six loosely-coupled sub-systems: +[Snowplow](https://github.com/snowplow/snowplow) consists of the following loosely-coupled sub-systems: -- **Trackers** fire Snowplow events. Currently Snowplow has 12 trackers, covering web, mobile, desktop, server and IoT -- **Collectors** receive Snowplow events from trackers. Currently we have three different event collectors, sinking events either to Amazon S3, Apache Kafka or Amazon Kinesis -- **Enrich** cleans up the raw Snowplow events, enriches them and puts them into storage. Currently we have a Hadoop-based enrichment process, and a Kinesis- or Kafka-based process -- **Storage** is where the Snowplow events live. Currently we store the Snowplow events in a flat file structure on S3, and in the Redshift and PostgreSQL databases -- **Data modeling** is where event-level data is joined with other data sets and aggregated into smaller data sets, and business logic is applied. This produces a clean set of tables which make it easier to perform analysis on the data. We have data models for Redshift and Looker +- **Trackers** fire Snowplow events. Snowplow has 12 trackers, covering web, mobile, desktop, server, and IoT. +- **Collectors** receive Snowplow events from trackers. We have three different event collectors, synchronizing events either to Amazon S3, Apache Kafka, or Amazon Kinesis. +- **Enrich** cleans up the raw Snowplow events, enriches them and puts them into storage. We have an Hadoop-based enrichment process, and a Kinesis-based or Kafka-based process. +- **Storage** is where the Snowplow events live. We store the Snowplow events in a flat file structure on S3, and in the Redshift and PostgreSQL databases. +- **Data modeling** is where event-level data is joined with other data sets and aggregated into smaller data sets, and business logic is applied. This produces a clean set of tables which make it easier to perform analysis on the data. We have data models for Redshift and Looker. - **Analytics** are performed on the Snowplow events or on the aggregate tables.  ->  ## Snowplow schema -We currently have many definitions of Snowplow's schema. We have an active issue to [standardize this schema](https://gitlab.com/gitlab-org/gitlab/-/issues/207930) including the following definitions: +We have many definitions of Snowplow's schema. We have an active issue to [standardize this schema](https://gitlab.com/gitlab-org/gitlab/-/issues/207930) including the following definitions: - Frontend and backend taxonomy as listed below -- [Feature instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy) +- [Feature instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy) - [Self describing events](https://github.com/snowplow/snowplow/wiki/Custom-events#self-describing-events) - [Iglu schema](https://gitlab.com/gitlab-org/iglu/) - [Snowplow authored events](https://github.com/snowplow/snowplow/wiki/Snowplow-authored-events) @@ -58,8 +49,8 @@ We currently have many definitions of Snowplow's schema. We have an active issue Tracking can be enabled at: -- The instance level, which will enable tracking on both the frontend and backend layers. -- User level, though user tracking can be disabled on a per-user basis. GitLab tracking respects the [Do Not Track](https://www.eff.org/issues/do-not-track) standard, so any user who has enabled the Do Not Track option in their browser will also not be tracked from a user level. +- The instance level, which enables tracking on both the frontend and backend layers. +- User level, though user tracking can be disabled on a per-user basis. GitLab tracking respects the [Do Not Track](https://www.eff.org/issues/do-not-track) standard, so any user who has enabled the Do Not Track option in their browser is not tracked at a user level. We utilize Snowplow for the majority of our tracking strategy and it is enabled on GitLab.com. On a self-managed instance, Snowplow can be enabled by navigating to: @@ -69,14 +60,20 @@ We utilize Snowplow for the majority of our tracking strategy and it is enabled The following configuration is required: | Name | Value | -| ------------- | ------------------------- | +|---------------|---------------------------| | Collector | `snowplow.trx.gitlab.net` | | Site ID | `gitlab` | | Cookie domain | `.gitlab.com` | ## Snowplow request flow -The following example shows a basic request/response flow between a Snowplow JS / Ruby Trackers on GitLab.com, [the GitLab.com Snowplow Collector](https://about.gitlab.com/handbook/engineering/infrastructure/library/snowplow/), GitLab's S3 Bucket, GitLab's Snowflake Data Warehouse, and Sisense.: +The following example shows a basic request/response flow between the following components: + +- Snowplow JS / Ruby Trackers on GitLab.com +- [GitLab.com Snowplow Collector](https://gitlab.com/gitlab-com/gl-infra/readiness/-/blob/master/library/snowplow/index.md) +- GitLab's S3 Bucket +- GitLab's Snowflake Data Warehouse +- Sisense: ```mermaid sequenceDiagram @@ -101,17 +98,17 @@ sequenceDiagram ## Implementing Snowplow JS (Frontend) tracking -GitLab provides `Tracking`, an interface that wraps the [Snowplow JavaScript Tracker](https://github.com/snowplow/snowplow/wiki/javascript-tracker) for tracking custom events. There are a few ways to utilize tracking, but each generally requires at minimum, a `category` and an `action`. Additional data can be provided that adheres to our [Feature instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). +GitLab provides `Tracking`, an interface that wraps the [Snowplow JavaScript Tracker](https://github.com/snowplow/snowplow/wiki/javascript-tracker) for tracking custom events. There are a few ways to utilize tracking, but each generally requires at minimum, a `category` and an `action`. Additional data can be provided that adheres to our [Feature instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy). -| field | type | default value | description | -|:-----------|:-------|:---------------------------|:------------| -| `category` | string | document.body.dataset.page | Page or subsection of a page that events are being captured within. | +| field | type | default value | description | +|:-----------|:-------|:---------------------------|:---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `category` | string | document.body.dataset.page | Page or subsection of a page that events are being captured within. | | `action` | string | 'generic' | Action the user is taking. Clicks should be `click` and activations should be `activate`, so for example, focusing a form field would be `activate_form_input`, and clicking a button would be `click_button`. | -| `data` | object | {} | Additional data such as `label`, `property`, `value`, and `context` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | +| `data` | object | {} | Additional data such as `label`, `property`, `value`, and `context` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy). | ### Tracking in HAML (or Vue Templates) -When working within HAML (or Vue templates) we can add `data-track-*` attributes to elements of interest. All elements that have a `data-track-event` attribute will automatically have event tracking bound on clicks. +When working within HAML (or Vue templates) we can add `data-track-*` attributes to elements of interest. All elements that have a `data-track-event` attribute automatically have event tracking bound on clicks. Below is an example of `data-track-*` attributes assigned to a button: @@ -127,17 +124,17 @@ Below is an example of `data-track-*` attributes assigned to a button: /> ``` -Event listeners are bound at the document level to handle click events on or within elements with these data attributes. This allows for them to be properly handled on re-rendering and changes to the DOM, but it's important to know that because of the way these events are bound, click events shouldn't be stopped from propagating up the DOM tree. If for any reason click events are being stopped from propagating, you'll need to implement your own listeners and follow the instructions in [Tracking in raw JavaScript](#tracking-in-raw-javascript). +Event listeners are bound at the document level to handle click events on or within elements with these data attributes. This allows them to be properly handled on re-rendering and changes to the DOM. Note that because of the way these events are bound, click events should not be stopped from propagating up the DOM tree. If for any reason click events are being stopped from propagating, you need to implement your own listeners and follow the instructions in [Tracking in raw JavaScript](#tracking-in-raw-javascript). Below is a list of supported `data-track-*` attributes: | attribute | required | description | |:----------------------|:---------|:------------| | `data-track-event` | true | Action the user is taking. Clicks must be prepended with `click` and activations must be prepended with `activate`. For example, focusing a form field would be `activate_form_input` and clicking a button would be `click_button`. | -| `data-track-label` | false | The `label` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | -| `data-track-property` | false | The `property` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | -| `data-track-value` | false | The `value` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). If omitted, this will be the element's `value` property or an empty string. For checkboxes, the default value will be the element's checked attribute or `false` when unchecked. | -| `data-track-context` | false | The `context` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | +| `data-track-label` | false | The `label` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy). | +| `data-track-property` | false | The `property` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy). | +| `data-track-value` | false | The `value` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy). If omitted, this is the element's `value` property or an empty string. For checkboxes, the default value is the element's checked attribute or `false` when unchecked. | +| `data-track-context` | false | The `context` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/product-processes/#taxonomy). | ### Tracking within Vue components @@ -148,9 +145,9 @@ import Tracking from '~/tracking'; const trackingMixin = Tracking.mixin({ label: 'right_sidebar' }); ``` -You can provide default options that will be passed along whenever an event is tracked from within your component. For instance, if all events within a component should be tracked with a given `label`, you can provide one at this time. Available defaults are `category`, `label`, `property`, and `value`. If no category is specified, `document.body.dataset.page` is used as the default. +You can provide default options that are passed along whenever an event is tracked from within your component. For instance, if all events within a component should be tracked with a given `label`, you can provide one at this time. Available defaults are `category`, `label`, `property`, and `value`. If no category is specified, `document.body.dataset.page` is used as the default. -You can then use the mixin normally in your component with the `mixin`, Vue declaration. The mixin also provides the ability to specify tracking options in `data` or `computed`. These will override any defaults and allows the values to be dynamic from props, or based on state. +You can then use the mixin normally in your component with the `mixin` Vue declaration. The mixin also provides the ability to specify tracking options in `data` or `computed`. These override any defaults and allow the values to be dynamic from props, or based on state. ```javascript export default { @@ -240,7 +237,6 @@ describe('MyTracking', () => { }); }); }); - ``` In obsolete Karma tests it's used as below: @@ -278,13 +274,13 @@ GitLab provides `Gitlab::Tracking`, an interface that wraps the [Snowplow Ruby T Custom event tracking and instrumentation can be added by directly calling the `GitLab::Tracking.event` class method, which accepts the following arguments: -| argument | type | default value | description | -|:-----------|:-------|:---------------------------|:------------| -| `category` | string | 'application' | Area or aspect of the application. This could be `HealthCheckController` or `Lfs::FileTransformer` for instance. | -| `action` | string | 'generic' | The action being taken, which can be anything from a controller action like `create` to something like an Active Record callback. | -| `data` | object | {} | Additional data such as `label`, `property`, `value`, and `context` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). These will be set as empty strings if you don't provide them. | +| argument | type | default value | description | +|:-----------|:-------|:--------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `category` | string | 'application' | Area or aspect of the application. This could be `HealthCheckController` or `Lfs::FileTransformer` for instance. | +| `action` | string | 'generic' | The action being taken, which can be anything from a controller action like `create` to something like an Active Record callback. | +| `data` | object | {} | Additional data such as `label`, `property`, `value`, and `context` as described [in our Feature Instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). These are set as empty strings if you don't provide them. | -Tracking can be viewed as either tracking user behavior, or can be utilized for instrumentation to monitor and visual performance over time in an area or aspect of code. +Tracking can be viewed as either tracking user behavior, or can be utilized for instrumentation to monitor and visualize performance over time in an area or aspect of code. For example: @@ -309,27 +305,27 @@ We use the [AsyncEmitter](https://github.com/snowplow/snowplow/wiki/Ruby-Tracker There are several tools for developing and testing Snowplow Event -| Testing Tool | Frontend Tracking | Backend Tracking | Local Development Environment | Production Environment | -| ------ | ------ | ------ | ------ | ------ | -| Snowplow Analytics Debugger Chrome Extension | ✅ | ❌ | ✅ | ✅ | -| Snowplow Inspector Chrome Extension | ✅ | ❌ | ✅ | ✅ | -| Snowplow Micro | ✅ | ✅ | ✅ | ❌ | -| Snowplow Mini | ✅ | ✅ | ❌ | ✅ | +| Testing Tool | Frontend Tracking | Backend Tracking | Local Development Environment | Production Environment | +|----------------------------------------------|--------------------|---------------------|-------------------------------|------------------------| +| Snowplow Analytics Debugger Chrome Extension | **{check-circle}** | **{dotted-circle}** | **{check-circle}** | **{check-circle}** | +| Snowplow Inspector Chrome Extension | **{check-circle}** | **{dotted-circle}** | **{check-circle}** | **{check-circle}** | +| Snowplow Micro | **{check-circle}** | **{check-circle}** | **{check-circle}** | **{dotted-circle}** | +| Snowplow Mini | **{check-circle}** | **{check-circle}** | **{dotted-circle}** | **{check-circle}** | ### Snowplow Analytics Debugger Chrome Extension Snowplow Analytics Debugger is a browser extension for testing frontend events. This works on production, staging and local development environments. -1. Install [Snowplow Analytics Debugger](https://chrome.google.com/webstore/detail/snowplow-analytics-debugg/jbnlcgeengmijcghameodeaenefieedm) chrome browser extension -1. Open Chrome DevTools to the Snowplow Analytics Debugger tab -1. Learn more at [Igloo Analytics](https://www.iglooanalytics.com/blog/snowplow-analytics-debugger-chrome-extension.html) +1. Install the [Snowplow Analytics Debugger](https://chrome.google.com/webstore/detail/snowplow-analytics-debugg/jbnlcgeengmijcghameodeaenefieedm) Chrome browser extension. +1. Open Chrome DevTools to the Snowplow Analytics Debugger tab. +1. Learn more at [Igloo Analytics](https://www.iglooanalytics.com/blog/snowplow-analytics-debugger-chrome-extension.html). ### Snowplow Inspector Chrome Extension Snowplow Inspector Chrome Extension is a browser extension for testing frontend events. This works on production, staging and local development environments. -1. Install [Snowplow Inspector](https://chrome.google.com/webstore/detail/snowplow-inspector/maplkdomeamdlngconidoefjpogkmljm?hl=en) -1. Open the chrome extension by pressing the Snowplow Inspector icon beside the address bar +1. Install [Snowplow Inspector](https://chrome.google.com/webstore/detail/snowplow-inspector/maplkdomeamdlngconidoefjpogkmljm?hl=en). +1. Open the Chrome extension by pressing the Snowplow Inspector icon beside the address bar. 1. Click around on a webpage with Snowplow and you should see JavaScript events firing in the inspector window. ### Snowplow Micro @@ -342,53 +338,59 @@ Snowplow Micro is a Docker-based solution for testing frontend and backend event - Look at the [Snowplow Micro repository](https://github.com/snowplow-incubator/snowplow-micro) - Watch our [installation guide recording](https://www.youtube.com/watch?v=OX46fo_A0Ag) -1. Install [Snowplow Micro](https://github.com/snowplow-incubator/snowplow-micro) +1. Install [Snowplow Micro](https://github.com/snowplow-incubator/snowplow-micro): -``` bash -docker run --mount type=bind,source=$(pwd)/example,destination=/config -p 9090:9090 snowplow/snowplow-micro:latest --collector-config /config/micro.conf --iglu /config/iglu.json -``` + ```shell + docker run --mount type=bind,source=$(pwd)/example,destination=/config -p 9090:9090 snowplow/snowplow-micro:latest --collector-config /config/micro.conf --iglu /config/iglu.json + ``` -1. Install snowplow micro by cloning the settings in [this project](https://gitlab.com/a_akgun/snowplow-micro). +1. Install snowplow micro by cloning the settings in [this project](https://gitlab.com/a_akgun/snowplow-micro): - ``` bash - git clone git@gitlab.com:a_akgun/snowplow-micro.git - ./snowplow-micro.sh - ``` + ```shell + git clone git@gitlab.com:a_akgun/snowplow-micro.git + ./snowplow-micro.sh + ``` -1. Update port in SQL (needed to set 9090) +1. Update port in SQL to set `9090`: - ``` bash - gdk psql -d gitlabhq_development - update application_settings set snowplow_collector_hostname='localhost:9090', snowplow_enabled=true, snowplow_cookie_domain='.gitlab.com'; - ``` + ```shell + gdk psql -d gitlabhq_development + update application_settings set snowplow_collector_hostname='localhost:9090', snowplow_enabled=true, snowplow_cookie_domain='.gitlab.com'; + ``` 1. Update `app/assets/javascripts/tracking.js` to [remove this line](https://gitlab.com/snippets/1918635): - ``` javascript - forceSecureTracker: true - ``` + ```javascript + forceSecureTracker: true + ``` 1. Update `lib/gitlab/tracking.rb` to [add these lines](https://gitlab.com/snippets/1918635): - ``` ruby - protocol: 'http', - port: 9090, - ``` + ```ruby + protocol: 'http', + port: 9090, + ``` 1. Update `lib/gitlab/tracking.rb` to [change async emitter from https to http](https://gitlab.com/snippets/1918635): - ``` ruby - SnowplowTracker::AsyncEmitter.new(Gitlab::CurrentSettings.snowplow_collector_hostname, protocol: 'http'), - ``` + ```ruby + SnowplowTracker::AsyncEmitter.new(Gitlab::CurrentSettings.snowplow_collector_hostname, protocol: 'http'), + ``` 1. Enable Snowplow in the admin area, Settings::Integrations::Snowplow to point to: - `http://localhost:3000/admin/application_settings/integrations#js-snowplow-settings` -1. `gdk restart` -1. Send a test Snowplow event from the Rails console + `http://localhost:3000/admin/application_settings/integrations#js-snowplow-settings`. + +1. Restart GDK: + + ```shell + `gdk restart` + ``` + +1. Send a test Snowplow event from the Rails console: - ``` ruby - Gitlab::Tracking.self_describing_event('iglu:com.gitlab/pageview_context/jsonschema/1-0-0', { page_type: ‘MY_TYPE' }, context: nil ) - ``` + ```ruby + Gitlab::Tracking.self_describing_event('iglu:com.gitlab/pageview_context/jsonschema/1-0-0', { page_type: ‘MY_TYPE' }, context: nil ) + ``` ### Snowplow Mini @@ -396,4 +398,4 @@ docker run --mount type=bind,source=$(pwd)/example,destination=/config -p 9090:9 Snowplow Mini can be used for testing frontend and backend events on a production, staging and local development environment. -For GitLab.com, we are currently setting up a [QA and Testing environment](https://gitlab.com/gitlab-org/telemetry/-/issues/266) using Snowplow Mini. +For GitLab.com, we're setting up a [QA and Testing environment](https://gitlab.com/gitlab-org/telemetry/-/issues/266) using Snowplow Mini. diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index 0f438e02772..5e78e2c5f25 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -21,9 +21,9 @@ For more information about Telemetry, see: More useful links: - [Telemetry Direction](https://about.gitlab.com/direction/telemetry/) -- [Data Analysis Process](https://about.gitlab.com/handbook/business-ops/data-team/#-data-analysis-process) -- [Data for Product Managers](https://about.gitlab.com/handbook/business-ops/data-team/data-for-product-managers/) -- [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/data-infrastructure/) +- [Data Analysis Process](https://about.gitlab.com/handbook/business-ops/data-team/#data-analysis-process/) +- [Data for Product Managers](https://about.gitlab.com/handbook/business-ops/data-team/programs/data-for-product-managers/) +- [Data Infrastructure](https://about.gitlab.com/handbook/business-ops/data-team/platform/infrastructure/) ## What is Usage Ping? @@ -299,7 +299,7 @@ Paste the SQL query into `#database-lab` to see how the query performs at scale. - `#database-lab` is a Slack channel which uses a production-sized environment to test your queries. - GitLab.com’s production database has a 15 second timeout. -- For each query we require an execution time of under 1 second due to cold caches which can 10x this time. +- Any single query must stay below 1 second execution time with cold caches. - Add a specialized index on columns involved to reduce the execution time. In order to have an understanding of the query's execution we add in the MR description the following information: @@ -326,7 +326,17 @@ When adding, changing, or updating metrics, please update the [Usage Statistics Check if new metrics need to be added to the Versions Application. See `usage_data` [schema](https://gitlab.com/gitlab-services/version-gitlab-com/-/blob/master/db/schema.rb#L147) and usage data [parameters accepted](https://gitlab.com/gitlab-services/version-gitlab-com/-/blob/master/app/services/usage_ping.rb). Any metrics added under the `counts` key are saved in the `counts` column. -### 6. Ask for a Telemetry Review +For further details, see the [Process to add additional instrumentation to the Usage Ping](https://about.gitlab.com/handbook/product/product-processes/#process-to-add-additional-instrumentation-to-the-usage-ping). + +### 6. Add the feature label + +Add the `feature` label to the Merge Request for new Usage Ping metrics. These are user-facing changes and are part of expanding the Usage Ping feature. + +### 7. Add a changelog file + +Ensure you comply with the [Changelog entries guide](../changelog.md). + +### 8. Ask for a Telemetry Review On GitLab.com, we have DangerBot setup to monitor Telemetry related files and DangerBot will recommend a Telemetry review. Mention `@gitlab-org/growth/telemetry/engineers` in your MR for a review. @@ -378,275 +388,355 @@ appear to be associated to any of the services running, since they all appear to ## Usage Statistics definitions -| Statistic | Section | Stage | Tier | Description | -|:--------------------------------------------------------|:-----------------------------------|:------------|:---------------|:--------------------------------------------------| -| `uuid` | | | | | -| `hostname` | | | | | -| `version` | | | | | -| `installation_type` | | | | | -| `active_user_count` | | | | | -| `recorded_at` | | | | | -| `edition` | | | | | -| `license_md5` | | | | | -| `license_id` | | | | | -| `historical_max_users` | | | | | -| `Name` | `licensee` | | | | -| `Email` | `licensee` | | | | -| `Company` | `licensee` | | | | -| `license_user_count` | | | | | -| `license_starts_at` | | | | | -| `license_expires_at` | | | | | -| `license_plan` | | | | | -| `license_trial` | | | | | -| `assignee_lists` | `counts` | | | | -| `boards` | `counts` | | | | -| `ci_builds` | `counts` | `verify` | | Unique builds in project | -| `ci_internal_pipelines` | `counts` | `verify` | | Total pipelines in GitLab repositories | -| `ci_external_pipelines` | `counts` | `verify` | | Total pipelines in external repositories | -| `ci_pipeline_config_auto_devops` | `counts` | `verify` | | Total pipelines from an Auto DevOps template | -| `ci_pipeline_config_repository` | `counts` | `verify` | | Total Pipelines from templates in repository | -| `ci_runners` | `counts` | `verify` | | Total configured Runners in project | -| `ci_triggers` | `counts` | `verify` | | Total configured Triggers in project | -| `ci_pipeline_schedules` | `counts` | `verify` | | Pipeline schedules in GitLab | -| `auto_devops_enabled` | `counts` |`configure` | | Projects with Auto DevOps template enabled | -| `auto_devops_disabled` | `counts` |`configure` | | Projects with Auto DevOps template disabled | -| `deploy_keys` | `counts` | | | | -| `deployments` | `counts` |`release` | | Total deployments | -| `dast_jobs` | `counts` | | | | -| `successful_deployments` | `counts` |`release` | | Total successful deployments | -| `failed_deployments` | `counts` |`release` | | Total failed deployments | -| `environments` | `counts` |`release` | | Total available and stopped environments | -| `clusters` | `counts` |`configure` | | Total GitLab Managed clusters both enabled and disabled | -| `clusters_enabled` | `counts` |`configure` | | Total GitLab Managed clusters currently enabled | -| `project_clusters_enabled` | `counts` |`configure` | | Total GitLab Managed clusters attached to projects| -| `group_clusters_enabled` | `counts` |`configure` | | Total GitLab Managed clusters attached to groups | -| `instance_clusters_enabled` | `counts` |`configure` | | Total GitLab Managed clusters attached to the instance | -| `clusters_disabled` | `counts` |`configure` | | Total GitLab Managed disabled clusters | -| `project_clusters_disabled` | `counts` |`configure` | | Total GitLab Managed disabled clusters previously attached to projects | -| `group_clusters_disabled` | `counts` |`configure` | | Total GitLab Managed disabled clusters previously attached to groups | -| `instance_clusters_disabled` | `counts` |`configure` | | Total GitLab Managed disabled clusters previously attached to the instance | -| `clusters_platforms_eks` | `counts` |`configure` | | Total GitLab Managed clusters provisioned with GitLab on AWS EKS | -| `clusters_platforms_gke` | `counts` |`configure` | | Total GitLab Managed clusters provisioned with GitLab on GCE GKE | -| `clusters_platforms_user` | `counts` |`configure` | | Total GitLab Managed clusters that are user provisioned | -| `clusters_applications_helm` | `counts` |`configure` | | Total GitLab Managed clusters with Helm enabled | -| `clusters_applications_ingress` | `counts` |`configure` | | Total GitLab Managed clusters with Ingress enabled | -| `clusters_applications_cert_managers` | `counts` |`configure` | | Total GitLab Managed clusters with Cert Manager enabled | -| `clusters_applications_crossplane` | `counts` |`configure` | | Total GitLab Managed clusters with Crossplane enabled | -| `clusters_applications_prometheus` | `counts` |`configure` | | Total GitLab Managed clusters with Prometheus enabled | -| `clusters_applications_runner` | `counts` |`configure` | | Total GitLab Managed clusters with Runner enabled | -| `clusters_applications_knative` | `counts` |`configure` | | Total GitLab Managed clusters with Knative enabled | -| `clusters_applications_elastic_stack` | `counts` |`configure` | | Total GitLab Managed clusters with Elastic Stack enabled | -| `clusters_management_project` | `counts` |`configure` | | Total GitLab Managed clusters with defined cluster management project | -| `in_review_folder` | `counts` | | | | -| `grafana_integrated_projects` | `counts` | | | | -| `groups` | `counts` | | | | -| `issues` | `counts` | | | | -| `issues_created_from_gitlab_error_tracking_ui` | `counts` | `monitor` | | | -| `issues_with_associated_zoom_link` | `counts` | `monitor` | | | -| `issues_using_zoom_quick_actions` | `counts` | `monitor` | | | -| `issues_with_embedded_grafana_charts_approx` | `counts` | `monitor` | | | -| `issues_with_health_status` | `counts` | | | | -| `keys` | `counts` | | | | -| `label_lists` | `counts` | | | | -| `lfs_objects` | `counts` | | | | -| `milestone_lists` | `counts` | | | | -| `milestones` | `counts` | | | | -| `pages_domains` | `counts` |`release` | | Total GitLab Pages domains | -| `pool_repositories` | `counts` | | | | -| `projects` | `counts` | | | | -| `projects_imported_from_github` | `counts` | | | | -| `projects_with_repositories_enabled` | `counts` | | | | -| `projects_with_error_tracking_enabled` | `counts` | `monitor` | | | -| `protected_branches` | `counts` | | | | -| `releases` | `counts` |`release` | | Unique release tags | -| `remote_mirrors` | `counts` | | | | -| `requirements_created` | `counts` | | | | -| `snippets` | `counts` | | | | -| `suggestions` | `counts` | | | | -| `todos` | `counts` | | | | -| `uploads` | `counts` | | | | -| `web_hooks` | `counts` | | | | -| `projects_alerts_active` | `counts` | | | | -| `projects_asana_active` | `counts` | | | | -| `projects_assembla_active` | `counts` | | | | -| `projects_bamboo_active` | `counts` | | | | -| `projects_bugzilla_active` | `counts` | | | | -| `projects_buildkite_active` | `counts` | | | | -| `projects_campfire_active` | `counts` | | | | -| `projects_custom_issue_tracker_active` | `counts` | | | | -| `projects_discord_active` | `counts` | | | | -| `projects_drone_ci_active` | `counts` | | | | -| `projects_emails_on_push_active` | `counts` | | | | -| `projects_external_wiki_active` | `counts` | | | -| `projects_flowdock_active` | `counts` | | | | -| `projects_github_active` | `counts` | | | | -| `projects_hangouts_chat_active` | `counts` | | | | -| `projects_hipchat_active` | `counts` | | | | -| `projects_irker_active` | `counts` | | | | -| `projects_jenkins_active` | `counts` | | | | -| `projects_jira_active` | `counts` | | | | -| `projects_mattermost_active` | `counts` | | | | -| `projects_mattermost_slash_commands_active` | `counts` | | | | -| `projects_microsoft_teams_active` | `counts` | | | | -| `projects_packagist_active` | `counts` | | | | -| `projects_pipelines_email_active` | `counts` | | | | -| `projects_pivotaltracker_active` | `counts` | | | | -| `projects_prometheus_active` | `counts` | | | | -| `projects_pushover_active` | `counts` | | | | -| `projects_redmine_active` | `counts` | | | | -| `projects_slack_active` | `counts` | | | | -| `projects_slack_slash_commands_active` | `counts` | | | | -| `projects_teamcity_active` | `counts` | | | | -| `projects_unify_circuit_active` | `counts` | | | | -| `projects_webex_teams_active` | `counts` | | | | -| `projects_youtrack_active` | `counts` | | | | -| `projects_slack_notifications_active` | `counts` | | | | -| `projects_slack_slash_active` | `counts` | | | | -| `projects_jira_server_active` | `counts` | | | | -| `projects_jira_cloud_active` | `counts` | | | | -| `projects_jira_dvcs_cloud_active` | `counts` | | | | -| `projects_jira_dvcs_server_active` | `counts` | | | | -| `labels` | `counts` | | | | -| `merge_requests` | `counts` | | | | -| `merge_requests_users` | `counts` | | | | -| `notes` | `counts` | | | | -| `wiki_pages_create` | `counts` | | | | -| `wiki_pages_update` | `counts` | | | | -| `wiki_pages_delete` | `counts` | | | | -| `web_ide_commits` | `counts` | | | | -| `web_ide_views` | `counts` | | | | -| `web_ide_merge_requests` | `counts` | | | | -| `web_ide_previews` | `counts` | | | | -| `snippet_comment` | `counts` | | | | -| `commit_comment` | `counts` | | | | -| `merge_request_comment` | `counts` | | | | -| `snippet_create` | `counts` | | | | -| `snippet_update` | `counts` | | | | -| `navbar_searches` | `counts` | | | | -| `cycle_analytics_views` | `counts` | | | | -| `productivity_analytics_views` | `counts` | | | | -| `source_code_pushes` | `counts` | | | | -| `merge_request_create` | `counts` | | | | -| `design_management_designs_create` | `counts` | | | | -| `design_management_designs_update` | `counts` | | | | -| `design_management_designs_delete` | `counts` | | | | -| `licenses_list_views` | `counts` | | | | -| `user_preferences_group_overview_details` | `counts` | | | | -| `user_preferences_group_overview_security_dashboard` | `counts` | | | | -| `ingress_modsecurity_logging` | `counts` | | | | -| `ingress_modsecurity_blocking` | `counts` | | | | -| `ingress_modsecurity_disabled` | `counts` | | | | -| `ingress_modsecurity_not_installed` | `counts` | | | | -| `dependency_list_usages_total` | `counts` | | | | -| `epics` | `counts` | | | | -| `feature_flags` | `counts` | | | | -| `geo_nodes` | `counts` | `geo` | | Number of sites in a Geo deployment | -| `geo_event_log_max_id` | `counts` | `geo` | | Number of replication events on a Geo primary | -| `incident_issues` | `counts` | `monitor` | | Issues created by the alert bot | -| `alert_bot_incident_issues` | `counts` | `monitor` | | Issues created by the alert bot | -| `incident_labeled_issues` | `counts` | `monitor` | | Issues with the incident label | -| `issues_created_gitlab_alerts` | `counts` | `monitor` | | Issues created from alerts by non-alert bot users | -| `issues_created_manually_from_alerts` | `counts` | `monitor` | | Issues created from alerts by non-alert bot users | -| `issues_created_from_alerts` | `counts` | `monitor` | | Issues created from Prometheus and alert management alerts | -| `ldap_group_links` | `counts` | | | | -| `ldap_keys` | `counts` | | | | -| `ldap_users` | `counts` | | | | -| `pod_logs_usages_total` | `counts` | | | | -| `projects_enforcing_code_owner_approval` | `counts` | | | | -| `projects_mirrored_with_pipelines_enabled` | `counts` |`release` | | Projects with repository mirroring enabled | -| `projects_reporting_ci_cd_back_to_github` | `counts` |`verify` | | Projects with a GitHub service pipeline enabled | -| `projects_with_packages` | `counts` |`package` | | Projects with package registry configured | -| `projects_with_prometheus_alerts` | `counts` |`monitor` | | Projects with Prometheus alerting enabled | -| `projects_with_tracing_enabled` | `counts` |`monitor` | | Projects with tracing enabled | -| `projects_with_alerts_service_enabled` | `counts` |`monitor` | | Projects with alerting service enabled | -| `template_repositories` | `counts` | | | | -| `container_scanning_jobs` | `counts` | | | | -| `dependency_scanning_jobs` | `counts` | | | | -| `license_management_jobs` | `counts` | | | | -| `sast_jobs` | `counts` | | | | -| `status_page_projects` | `counts` | `monitor` | | Projects with status page enabled | -| `status_page_issues` | `counts` | `monitor` | | Issues published to a Status Page | -| `status_page_incident_publishes` | `counts` | `monitor` | | Cumulative count of usages of publish operation | -| `status_page_incident_unpublishes` | `counts` | `monitor` | | Cumulative count of usages of unpublish operation | -| `epics_deepest_relationship_level` | `counts` | | | | -| `operations_dashboard_default_dashboard` | `counts` | `monitor` | | Active users with enabled operations dashboard | -| `operations_dashboard_users_with_projects_added` | `counts` | `monitor` | | Active users with projects on operations dashboard| -| `container_registry_enabled` | | | | | -| `dependency_proxy_enabled` | | | | | -| `gitlab_shared_runners_enabled` | | | | | -| `gravatar_enabled` | | | | | -| `ldap_enabled` | | | | | -| `mattermost_enabled` | | | | | -| `omniauth_enabled` | | | | | -| `prometheus_metrics_enabled` | | | | | -| `reply_by_email_enabled` | | | | | -| `average` | `avg_cycle_analytics - code` | | | | -| `sd` | `avg_cycle_analytics - code` | | | | -| `missing` | `avg_cycle_analytics - code` | | | | -| `average` | `avg_cycle_analytics - test` | | | | -| `sd` | `avg_cycle_analytics - test` | | | | -| `missing` | `avg_cycle_analytics - test` | | | | -| `average` | `avg_cycle_analytics - review` | | | | -| `sd` | `avg_cycle_analytics - review` | | | | -| `missing` | `avg_cycle_analytics - review` | | | | -| `average` | `avg_cycle_analytics - staging` | | | | -| `sd` | `avg_cycle_analytics - staging` | | | | -| `missing` | `avg_cycle_analytics - staging` | | | | -| `average` | `avg_cycle_analytics - production` | | | | -| `sd` | `avg_cycle_analytics - production` | | | | -| `missing` | `avg_cycle_analytics - production` | | | | -| `total` | `avg_cycle_analytics` | | | | -| `clusters_applications_cert_managers` | `usage_activity_by_stage` | `configure` | | Unique clusters with certificate managers enabled | -| `clusters_applications_helm` | `usage_activity_by_stage` | `configure` | | Unique clusters with Helm enabled | -| `clusters_applications_ingress` | `usage_activity_by_stage` | `configure` | | Unique clusters with Ingress enabled | -| `clusters_applications_knative` | `usage_activity_by_stage` | `configure` | | Unique clusters with Knative enabled | -| `clusters_management_project` | `usage_activity_by_stage` | `configure` | | Unique clusters with project management enabled | -| `clusters_disabled` | `usage_activity_by_stage` | `configure` | | Total non-"GitLab Managed clusters" | -| `clusters_enabled` | `usage_activity_by_stage` | `configure` | | Total GitLab Managed clusters | -| `clusters_platforms_gke` | `usage_activity_by_stage` | `configure` | | Unique clusters with Google Cloud installed | -| `clusters_platforms_eks` | `usage_activity_by_stage` | `configure` | | Unique clusters with AWS installed | -| `clusters_platforms_user` | `usage_activity_by_stage` | `configure` | | Unique clusters that are user provided | -| `instance_clusters_disabled` | `usage_activity_by_stage` | `configure` | | Unique clusters disabled on instance | -| `instance_clusters_enabled` | `usage_activity_by_stage` | `configure` | | Unique clusters enabled on instance | -| `group_clusters_disabled` | `usage_activity_by_stage` | `configure` | | Unique clusters disabled on group | -| `group_clusters_enabled` | `usage_activity_by_stage` | `configure` | | Unique clusters enabled on group | -| `project_clusters_disabled` | `usage_activity_by_stage` | `configure` | | Unique clusters disabled on project | -| `project_clusters_enabled` | `usage_activity_by_stage` | `configure` | | Unique clusters enabled on project | -| `projects_slack_notifications_active` | `usage_activity_by_stage` | `configure` | | Unique projects with Slack service enabled | -| `projects_slack_slash_active` | `usage_activity_by_stage` | `configure` | | Unique projects with Slack '/' commands enabled | -| `projects_with_prometheus_alerts: 0` | `usage_activity_by_stage` | `monitor` | | Projects with Prometheus enabled and no alerts | -| `deploy_keys` | `usage_activity_by_stage` | `create` | | | -| `keys` | `usage_activity_by_stage` | `create` | | | -| `projects_jira_dvcs_server_active` | `usage_activity_by_stage` | `plan` | | | -| `service_desk_enabled_projects` | `usage_activity_by_stage` | `plan` | | | -| `service_desk_issues` | `usage_activity_by_stage` | `plan` | | | -| `todos: 0` | `usage_activity_by_stage` | `plan` | | | -| `deployments` | `usage_activity_by_stage` | `release` | | Total deployments | -| `failed_deployments` | `usage_activity_by_stage` | `release` | | Total failed deployments | -| `projects_mirrored_with_pipelines_enabled` | `usage_activity_by_stage` | `release` | | Projects with repository mirroring enabled | -| `releases` | `usage_activity_by_stage` | `release` | | Unique release tags in project | -| `successful_deployments: 0` | `usage_activity_by_stage` | `release` | | Total successful deployments | -| `user_preferences_group_overview_security_dashboard: 0` | `usage_activity_by_stage` | `secure` | | | -| `ci_builds` | `usage_activity_by_stage` | `verify` | | Unique builds in project | -| `ci_external_pipelines` | `usage_activity_by_stage` | `verify` | | Total pipelines in external repositories | -| `ci_internal_pipelines` | `usage_activity_by_stage` | `verify` | | Total pipelines in GitLab repositories | -| `ci_pipeline_config_auto_devops` | `usage_activity_by_stage` | `verify` | | Total pipelines from an Auto DevOps template | -| `ci_pipeline_config_repository` | `usage_activity_by_stage` | `verify` | | Pipelines from templates in repository | -| `ci_pipeline_schedules` | `usage_activity_by_stage` | `verify` | | Pipeline schedules in GitLab | -| `ci_pipelines` | `usage_activity_by_stage` | `verify` | | Total pipelines | -| `ci_triggers` | `usage_activity_by_stage` | `verify` | | Triggers enabled | -| `clusters_applications_runner` | `usage_activity_by_stage` | `verify` | | Unique clusters with Runner enabled | -| `projects_reporting_ci_cd_back_to_github: 0` | `usage_activity_by_stage` | `verify` | | Unique projects with a GitHub pipeline enabled | -| `nodes` | `topology` | `enablement`| | The list of server nodes on which GitLab components are running | -| `duration_s` | `topology` | `enablement`| | Time it took to collect topology data | -| `node_memory_total_bytes` | `topology > nodes` | `enablement`| | The total available memory of this node | -| `node_cpus` | `topology > nodes` | `enablement`| | The number of CPU cores of this node | -| `node_services` | `topology > nodes` | `enablement`| | The list of GitLab services running on this node | -| `name` | `topology > nodes > node_services` | `enablement`| | The name of the GitLab service running on this node | -| `process_count` | `topology > nodes > node_services` | `enablement`| | The number of processes running for this service | -| `process_memory_rss` | `topology > nodes > node_services` | `enablement`| | The average Resident Set Size of a service process | -| `process_memory_uss` | `topology > nodes > node_services` | `enablement`| | The average Unique Set Size of a service process | -| `process_memory_pss` | `topology > nodes > node_services` | `enablement`| | The average Proportional Set Size of a service process | +| Statistic | Section | Stage | Tier | Edition | Description | +| --------------------------------------------------------- | ------------------------------------ | ------------- | ---------------- | ------- | -------------------------------------------------------------------------- | +| `uuid` | | | | | | +| `hostname` | | | | | | +| `version` | | | | | | +| `installation_type` | | | | | | +| `active_user_count` | | | | | | +| `recorded_at` | | | | | | +| `recording_ce_finished_at` | | | | CE+EE | When the core features were computed | +| `recording_ee_finished_at` | | | | EE | When the EE-specific features were computed | +| `edition` | | | | | | +| `license_md5` | | | | | | +| `license_id` | | | | | | +| `historical_max_users` | | | | | | +| `Name` | `licensee` | | | | | +| `Email` | `licensee` | | | | | +| `Company` | `licensee` | | | | | +| `license_user_count` | | | | | | +| `license_starts_at` | | | | | | +| `license_expires_at` | | | | | | +| `license_plan` | | | | | | +| `license_trial` | | | | | | +| `assignee_lists` | `counts` | | | | | +| `boards` | `counts` | | | | | +| `ci_builds` | `counts` | `verify` | | | Unique builds in project | +| `ci_internal_pipelines` | `counts` | `verify` | | | Total pipelines in GitLab repositories | +| `ci_external_pipelines` | `counts` | `verify` | | | Total pipelines in external repositories | +| `ci_pipeline_config_auto_devops` | `counts` | `verify` | | | Total pipelines from an Auto DevOps template | +| `ci_pipeline_config_repository` | `counts` | `verify` | | | Total Pipelines from templates in repository | +| `ci_runners` | `counts` | `verify` | | | Total configured Runners in project | +| `ci_triggers` | `counts` | `verify` | | | Total configured Triggers in project | +| `ci_pipeline_schedules` | `counts` | `verify` | | | Pipeline schedules in GitLab | +| `auto_devops_enabled` | `counts` | `configure` | | | Projects with Auto DevOps template enabled | +| `auto_devops_disabled` | `counts` | `configure` | | | Projects with Auto DevOps template disabled | +| `deploy_keys` | `counts` | | | | | +| `deployments` | `counts` | `release` | | | Total deployments | +| `deployments` | `counts_monthly` | `release` | | | Total deployments last 28 days | +| `dast_jobs` | `counts` | | | | | +| `successful_deployments` | `counts` | `release` | | | Total successful deployments | +| `successful_deployments` | `counts_monthly` | `release` | | | Total successful deployments last 28 days | +| `failed_deployments` | `counts` | `release` | | | Total failed deployments | +| `failed_deployments` | `counts_monthly` | `release` | | | Total failed deployments last 28 days | +| `environments` | `counts` | `release` | | | Total available and stopped environments | +| `clusters` | `counts` | `configure` | | | Total GitLab Managed clusters both enabled and disabled | +| `clusters_enabled` | `counts` | `configure` | | | Total GitLab Managed clusters currently enabled | +| `project_clusters_enabled` | `counts` | `configure` | | | Total GitLab Managed clusters attached to projects | +| `group_clusters_enabled` | `counts` | `configure` | | | Total GitLab Managed clusters attached to groups | +| `instance_clusters_enabled` | `counts` | `configure` | | | Total GitLab Managed clusters attached to the instance | +| `clusters_disabled` | `counts` | `configure` | | | Total GitLab Managed disabled clusters | +| `project_clusters_disabled` | `counts` | `configure` | | | Total GitLab Managed disabled clusters previously attached to projects | +| `group_clusters_disabled` | `counts` | `configure` | | | Total GitLab Managed disabled clusters previously attached to groups | +| `instance_clusters_disabled` | `counts` | `configure` | | | Total GitLab Managed disabled clusters previously attached to the instance | +| `clusters_platforms_eks` | `counts` | `configure` | | | Total GitLab Managed clusters provisioned with GitLab on AWS EKS | +| `clusters_platforms_gke` | `counts` | `configure` | | | Total GitLab Managed clusters provisioned with GitLab on GCE GKE | +| `clusters_platforms_user` | `counts` | `configure` | | | Total GitLab Managed clusters that are user provisioned | +| `clusters_applications_helm` | `counts` | `configure` | | | Total GitLab Managed clusters with Helm enabled | +| `clusters_applications_ingress` | `counts` | `configure` | | | Total GitLab Managed clusters with Ingress enabled | +| `clusters_applications_cert_managers` | `counts` | `configure` | | | Total GitLab Managed clusters with Cert Manager enabled | +| `clusters_applications_crossplane` | `counts` | `configure` | | | Total GitLab Managed clusters with Crossplane enabled | +| `clusters_applications_prometheus` | `counts` | `configure` | | | Total GitLab Managed clusters with Prometheus enabled | +| `clusters_applications_runner` | `counts` | `configure` | | | Total GitLab Managed clusters with Runner enabled | +| `clusters_applications_knative` | `counts` | `configure` | | | Total GitLab Managed clusters with Knative enabled | +| `clusters_applications_elastic_stack` | `counts` | `configure` | | | Total GitLab Managed clusters with Elastic Stack enabled | +| `clusters_applications_cilium` | `counts` | `configure` | | | Total GitLab Managed clusters with Cilium enabled | +| `clusters_management_project` | `counts` | `configure` | | | Total GitLab Managed clusters with defined cluster management project | +| `in_review_folder` | `counts` | | | | | +| `grafana_integrated_projects` | `counts` | | | | | +| `groups` | `counts` | | | | | +| `issues` | `counts` | | | | | +| `issues_created_from_gitlab_error_tracking_ui` | `counts` | `monitor` | | | | +| `issues_with_associated_zoom_link` | `counts` | `monitor` | | | | +| `issues_using_zoom_quick_actions` | `counts` | `monitor` | | | | +| `issues_with_embedded_grafana_charts_approx` | `counts` | `monitor` | | | | +| `issues_with_health_status` | `counts` | | | | | +| `keys` | `counts` | | | | | +| `label_lists` | `counts` | | | | | +| `lfs_objects` | `counts` | | | | | +| `milestone_lists` | `counts` | | | | | +| `milestones` | `counts` | | | | | +| `pages_domains` | `counts` | `release` | | | Total GitLab Pages domains | +| `pool_repositories` | `counts` | | | | | +| `projects` | `counts` | | | | | +| `projects_imported_from_github` | `counts` | | | | | +| `projects_with_repositories_enabled` | `counts` | | | | | +| `projects_with_error_tracking_enabled` | `counts` | `monitor` | | | | +| `protected_branches` | `counts` | | | | | +| `releases` | `counts` | `release` | | | Unique release tags | +| `remote_mirrors` | `counts` | | | | | +| `requirements_created` | `counts` | | | | | +| `snippets` | `counts` | 'create' | | CE+EE | | +| `snippets` | `counts_monthly` | 'create' | | CE+EE | | +| `personal_snippets` | `counts` | 'create' | | CE+EE | | +| `personal_snippets` | `counts_monthly` | 'create' | | CE+EE | | +| `project_snippets` | `counts` | 'create' | | CE+EE | | +| `project_snippets` | `counts_monthly` | 'create' | | CE+EE | | +| `suggestions` | `counts` | | | | | +| `todos` | `counts` | | | | | +| `uploads` | `counts` | | | | | +| `web_hooks` | `counts` | | | | | +| `projects_alerts_active` | `counts` | | | | | +| `projects_asana_active` | `counts` | | | | | +| `projects_assembla_active` | `counts` | | | | | +| `projects_bamboo_active` | `counts` | | | | | +| `projects_bugzilla_active` | `counts` | | | | | +| `projects_buildkite_active` | `counts` | | | | | +| `projects_campfire_active` | `counts` | | | | | +| `projects_custom_issue_tracker_active` | `counts` | | | | | +| `projects_discord_active` | `counts` | | | | | +| `projects_drone_ci_active` | `counts` | | | | | +| `projects_emails_on_push_active` | `counts` | | | | | +| `projects_external_wiki_active` | `counts` | | | | | +| `projects_flowdock_active` | `counts` | | | | | +| `projects_github_active` | `counts` | | | | | +| `projects_hangouts_chat_active` | `counts` | | | | | +| `projects_hipchat_active` | `counts` | | | | | +| `projects_irker_active` | `counts` | | | | | +| `projects_jenkins_active` | `counts` | | | | | +| `projects_jira_active` | `counts` | | | | | +| `projects_mattermost_active` | `counts` | | | | | +| `projects_mattermost_slash_commands_active` | `counts` | | | | | +| `projects_microsoft_teams_active` | `counts` | | | | | +| `projects_packagist_active` | `counts` | | | | | +| `projects_pipelines_email_active` | `counts` | | | | | +| `projects_pivotaltracker_active` | `counts` | | | | | +| `projects_prometheus_active` | `counts` | | | | | +| `projects_pushover_active` | `counts` | | | | | +| `projects_redmine_active` | `counts` | | | | | +| `projects_slack_active` | `counts` | | | | | +| `projects_slack_slash_commands_active` | `counts` | | | | | +| `projects_teamcity_active` | `counts` | | | | | +| `projects_unify_circuit_active` | `counts` | | | | | +| `projects_webex_teams_active` | `counts` | | | | | +| `projects_youtrack_active` | `counts` | | | | | +| `projects_jira_server_active` | `counts` | | | | | +| `projects_jira_cloud_active` | `counts` | | | | | +| `projects_jira_dvcs_cloud_active` | `counts` | | | | | +| `projects_jira_dvcs_server_active` | `counts` | | | | | +| `projects_jira_issuelist_active` | `counts` | `create` | | EE | Total Jira Issue feature enabled | +| `labels` | `counts` | | | | | +| `merge_requests` | `counts` | | | | | +| `merge_requests_users` | `counts` | | | | | +| `notes` | `counts` | | | | | +| `wiki_pages_create` | `counts` | | | | | +| `wiki_pages_update` | `counts` | | | | | +| `wiki_pages_delete` | `counts` | | | | | +| `web_ide_commits` | `counts` | | | | | +| `web_ide_views` | `counts` | | | | | +| `web_ide_merge_requests` | `counts` | | | | | +| `web_ide_previews` | `counts` | | | | | +| `snippet_comment` | `counts` | | | | | +| `commit_comment` | `counts` | | | | | +| `merge_request_comment` | `counts` | | | | | +| `snippet_create` | `counts` | | | | | +| `snippet_update` | `counts` | | | | | +| `navbar_searches` | `counts` | | | | | +| `cycle_analytics_views` | `counts` | | | | | +| `productivity_analytics_views` | `counts` | | | | | +| `source_code_pushes` | `counts` | | | | | +| `merge_request_create` | `counts` | | | | | +| `design_management_designs_create` | `counts` | | | | | +| `design_management_designs_update` | `counts` | | | | | +| `design_management_designs_delete` | `counts` | | | | | +| `licenses_list_views` | `counts` | | | | | +| `user_preferences_group_overview_details` | `counts` | | | | | +| `user_preferences_group_overview_security_dashboard` | `counts` | | | | | +| `ingress_modsecurity_logging` | `counts` | | | | | +| `ingress_modsecurity_blocking` | `counts` | | | | | +| `ingress_modsecurity_disabled` | `counts` | | | | | +| `ingress_modsecurity_not_installed` | `counts` | | | | | +| `dependency_list_usages_total` | `counts` | | | | | +| `epics` | `counts` | | | | | +| `feature_flags` | `counts` | | | | | +| `geo_nodes` | `counts` | `geo` | | | Number of sites in a Geo deployment | +| `geo_event_log_max_id` | `counts` | `geo` | | | Number of replication events on a Geo primary | +| `incident_issues` | `counts` | `monitor` | | | Issues created by the alert bot | +| `alert_bot_incident_issues` | `counts` | `monitor` | | | Issues created by the alert bot | +| `incident_labeled_issues` | `counts` | `monitor` | | | Issues with the incident label | +| `issues_created_gitlab_alerts` | `counts` | `monitor` | | | Issues created from alerts by non-alert bot users | +| `issues_created_manually_from_alerts` | `counts` | `monitor` | | | Issues created from alerts by non-alert bot users | +| `issues_created_from_alerts` | `counts` | `monitor` | | | Issues created from Prometheus and alert management alerts | +| `ldap_group_links` | `counts` | | | | | +| `ldap_keys` | `counts` | | | | | +| `ldap_users` | `counts` | | | | | +| `pod_logs_usages_total` | `counts` | | | | | +| `projects_enforcing_code_owner_approval` | `counts` | | | | | +| `projects_mirrored_with_pipelines_enabled` | `counts` | `release` | | | Projects with repository mirroring enabled | +| `projects_reporting_ci_cd_back_to_github` | `counts` | `verify` | | | Projects with a GitHub service pipeline enabled | +| `projects_with_packages` | `counts` | `package` | | | Projects with package registry configured | +| `projects_with_prometheus_alerts` | `counts` | `monitor` | | | Projects with Prometheus alerting enabled | +| `projects_with_tracing_enabled` | `counts` | `monitor` | | | Projects with tracing enabled | +| `projects_with_alerts_service_enabled` | `counts` | `monitor` | | | Projects with alerting service enabled | +| `template_repositories` | `counts` | | | | | +| `container_scanning_jobs` | `counts` | | | | | +| `dependency_scanning_jobs` | `counts` | | | | | +| `license_management_jobs` | `counts` | | | | | +| `sast_jobs` | `counts` | | | | | +| `status_page_projects` | `counts` | `monitor` | | | Projects with status page enabled | +| `status_page_issues` | `counts` | `monitor` | | | Issues published to a Status Page | +| `status_page_incident_publishes` | `counts` | `monitor` | | | Cumulative count of usages of publish operation | +| `status_page_incident_unpublishes` | `counts` | `monitor` | | | Cumulative count of usages of unpublish operation | +| `epics_deepest_relationship_level` | `counts` | | | | | +| `operations_dashboard_default_dashboard` | `counts` | `monitor` | | | Active users with enabled operations dashboard | +| `operations_dashboard_users_with_projects_added` | `counts` | `monitor` | | | Active users with projects on operations dashboard | +| `container_registry_enabled` | | | | | | +| `dependency_proxy_enabled` | | | | | | +| `gitlab_shared_runners_enabled` | | | | | | +| `gravatar_enabled` | | | | | | +| `ldap_enabled` | | | | | | +| `mattermost_enabled` | | | | | | +| `omniauth_enabled` | | | | | | +| `prometheus_enabled` | | | | | Whether the bundled Prometheus is enabled | +| `prometheus_metrics_enabled` | | | | | | +| `reply_by_email_enabled` | | | | | | +| `average` | `avg_cycle_analytics - code` | | | | | +| `sd` | `avg_cycle_analytics - code` | | | | | +| `missing` | `avg_cycle_analytics - code` | | | | | +| `average` | `avg_cycle_analytics - test` | | | | | +| `sd` | `avg_cycle_analytics - test` | | | | | +| `missing` | `avg_cycle_analytics - test` | | | | | +| `average` | `avg_cycle_analytics - review` | | | | | +| `sd` | `avg_cycle_analytics - review` | | | | | +| `missing` | `avg_cycle_analytics - review` | | | | | +| `average` | `avg_cycle_analytics - staging` | | | | | +| `sd` | `avg_cycle_analytics - staging` | | | | | +| `missing` | `avg_cycle_analytics - staging` | | | | | +| `average` | `avg_cycle_analytics - production` | | | | | +| `sd` | `avg_cycle_analytics - production` | | | | | +| `missing` | `avg_cycle_analytics - production` | | | | | +| `total` | `avg_cycle_analytics` | | | | | +| `g_analytics_contribution` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/contribution_analytics | +| `g_analytics_insights` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/insights | +| `g_analytics_issues` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/issues_analytics | +| `g_analytics_productivity` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/analytics/productivity_analytics | +| `g_analytics_valuestream` | `analytics_unique_visits` | `manage` | | | Visits to /groups/:group/-/analytics/value_stream_analytics | +| `p_analytics_pipelines` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/pipelines/charts | +| `p_analytics_code_reviews` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/analytics/code_reviews | +| `p_analytics_valuestream` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/value_stream_analytics | +| `p_analytics_insights` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/insights | +| `p_analytics_issues` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/analytics/issues_analytics | +| `p_analytics_repo` | `analytics_unique_visits` | `manage` | | | Visits to /:group/:project/-/graphs/master/charts | +| `u_analytics_todos` | `analytics_unique_visits` | `manage` | | | Visits to /dashboard/todos | +| `i_analytics_cohorts` | `analytics_unique_visits` | `manage` | | | Visits to /-/instance_statistics/cohorts | +| `i_analytics_dev_ops_score` | `analytics_unique_visits` | `manage` | | | Visits to /-/instance_statistics/dev_ops_score | +| `analytics_unique_visits_for_any_target` | `analytics_unique_visits` | `manage` | | | Visits to any of the pages listed above | +| `clusters_applications_cert_managers` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with certificate managers enabled | +| `clusters_applications_helm` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with Helm enabled | +| `clusters_applications_ingress` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with Ingress enabled | +| `clusters_applications_knative` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with Knative enabled | +| `clusters_management_project` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with project management enabled | +| `clusters_disabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Total non-"GitLab Managed clusters" | +| `clusters_enabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Total GitLab Managed clusters | +| `clusters_platforms_gke` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with Google Cloud installed | +| `clusters_platforms_eks` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters with AWS installed | +| `clusters_platforms_user` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters that are user provided | +| `instance_clusters_disabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters disabled on instance | +| `instance_clusters_enabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters enabled on instance | +| `group_clusters_disabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters disabled on group | +| `group_clusters_enabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters enabled on group | +| `project_clusters_disabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters disabled on project | +| `project_clusters_enabled` | `usage_activity_by_stage` | `configure` | | CE+EE | Unique clusters enabled on project | +| `projects_slack_notifications_active` | `usage_activity_by_stage` | `configure` | | EE | Unique projects with Slack service enabled | +| `projects_slack_slash_active` | `usage_activity_by_stage` | `configure` | | EE | Unique projects with Slack '/' commands enabled | +| `projects_with_prometheus_alerts` | `usage_activity_by_stage` | `configure` | | EE | Projects with Prometheus enabled and no alerts | +| `deploy_keys` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `keys` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `merge_requests` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `projects_with_disable_overriding_approvers_per_merge_request` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `projects_without_disable_overriding_approvers_per_merge_request` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `remote_mirrors` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `snippets` | `usage_activity_by_stage` | `create` | | CE+EE | | +| `merge_requests_users` | `usage_activity_by_stage_monthly` | `create` | | CE+EE | Unique count of users who used a merge request | +| `action_monthly_active_users_project_repo` | `usage_activity_by_stage_monthly` | `create` | | CE+EE | Unique count of users who pushed to a project repo | +| `action_monthly_active_users_design_management` | `usage_activity_by_stage_monthly` | `create` | | CE+EE | Unique count of users who interacted with the design system management | +| `action_monthly_active_users_wiki_repo` | `usage_activity_by_stage_monthly` | `create` | | CE+EE | Unique count of users who created or updated a wiki repo | +| `projects_enforcing_code_owner_approval` | `usage_activity_by_stage` | `create` | | EE | | +| `merge_requests_with_optional_codeowners` | `usage_activity_by_stage` | `create` | | EE | | +| `merge_requests_with_required_codeowners` | `usage_activity_by_stage` | `create` | | EE | | +| `projects_imported_from_github` | `usage_activity_by_stage` | `create` | | EE | | +| `projects_with_repositories_enabled` | `usage_activity_by_stage` | `create` | | EE | | +| `protected_branches` | `usage_activity_by_stage` | `create` | | EE | | +| `suggestions` | `usage_activity_by_stage` | `create` | | EE | | +| `approval_project_rules` | `usage_activity_by_stage` | `create` | | EE | Number of project approval rules | +| `approval_project_rules_with_target_branch` | `usage_activity_by_stage` | `create` | | EE | Number of project approval rules with not default target branch | +| `merge_requests_with_added_rules` | `usage_activity_by_stage` | `create` | | EE | Merge Requests with added rules | +| `clusters` | `usage_activity_by_stage` | `monitor` | | CE+EE | | +| `clusters_applications_prometheus` | `usage_activity_by_stage` | `monitor` | | CE+EE | | +| `operations_dashboard_default_dashboard` | `usage_activity_by_stage` | `monitor` | | CE+EE | | +| `operations_dashboard_users_with_projects_added` | `usage_activity_by_stage` | `monitor` | | EE | | +| `projects_prometheus_active` | `usage_activity_by_stage` | `monitor` | | EE | | +| `projects_with_error_tracking_enabled` | `usage_activity_by_stage` | `monitor` | | EE | | +| `projects_with_tracing_enabled` | `usage_activity_by_stage` | `monitor` | | EE | | +| `events` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `groups` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `users_created_at` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `omniauth_providers` | `usage_activity_by_stage` | `manage` | | CE+EE | | +| `ldap_keys` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_users` | `usage_activity_by_stage` | `manage` | | EE | | +| `value_stream_management_customized_group_stages` | `usage_activity_by_stage` | `manage` | | EE | | +| `projects_with_compliance_framework` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_servers` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_group_sync_enabled` | `usage_activity_by_stage` | `manage` | | EE | | +| `ldap_admin_sync_enabled` | `usage_activity_by_stage` | `manage` | | EE | | +| `group_saml_enabled` | `usage_activity_by_stage` | `manage` | | EE | | +| `issues` | `usage_activity_by_stage` | `plan` | | CE+EE | | +| `notes` | `usage_activity_by_stage` | `plan` | | CE+EE | | +| `projects` | `usage_activity_by_stage` | `plan` | | CE+EE | | +| `todos` | `usage_activity_by_stage` | `plan` | | CE+EE | | +| `assignee_lists` | `usage_activity_by_stage` | `plan` | | EE | | +| `epics` | `usage_activity_by_stage` | `plan` | | EE | | +| `label_lists` | `usage_activity_by_stage` | `plan` | | EE | | +| `milestone_lists` | `usage_activity_by_stage` | `plan` | | EE | | +| `projects_jira_active` | `usage_activity_by_stage` | `plan` | | EE | | +| `projects_jira_dvcs_server_active` | `usage_activity_by_stage` | `plan` | | EE | | +| `projects_jira_dvcs_server_active` | `usage_activity_by_stage` | `plan` | | EE | | +| `service_desk_enabled_projects` | `usage_activity_by_stage` | `plan` | | CE+EE | | +| `service_desk_issues` | `usage_activity_by_stage` | `plan` | | CE+EE | | +| `deployments` | `usage_activity_by_stage` | `release` | | CE+EE | Total deployments | +| `failed_deployments` | `usage_activity_by_stage` | `release` | | CE+EE | Total failed deployments | +| `projects_mirrored_with_pipelines_enabled` | `usage_activity_by_stage` | `release` | | EE | Projects with repository mirroring enabled | +| `releases` | `usage_activity_by_stage` | `release` | | CE+EE | Unique release tags in project | +| `successful_deployments` | `usage_activity_by_stage` | `release` | | CE+EE | Total successful deployments | +| `user_preferences_group_overview_security_dashboard` | `usage_activity_by_stage` | `secure` | | | | +| `ci_builds` | `usage_activity_by_stage` | `verify` | | CE+EE | Unique builds in project | +| `ci_external_pipelines` | `usage_activity_by_stage` | `verify` | | CE+EE | Total pipelines in external repositories | +| `ci_internal_pipelines` | `usage_activity_by_stage` | `verify` | | CE+EE | Total pipelines in GitLab repositories | +| `ci_pipeline_config_auto_devops` | `usage_activity_by_stage` | `verify` | | CE+EE | Total pipelines from an Auto DevOps template | +| `ci_pipeline_config_repository` | `usage_activity_by_stage` | `verify` | | CE+EE | Pipelines from templates in repository | +| `ci_pipeline_schedules` | `usage_activity_by_stage` | `verify` | | CE+EE | Pipeline schedules in GitLab | +| `ci_pipelines` | `usage_activity_by_stage` | `verify` | | CE+EE | Total pipelines | +| `ci_triggers` | `usage_activity_by_stage` | `verify` | | CE+EE | Triggers enabled | +| `clusters_applications_runner` | `usage_activity_by_stage` | `verify` | | CE+EE | Unique clusters with Runner enabled | +| `projects_reporting_ci_cd_back_to_github` | `usage_activity_by_stage` | `verify` | | EE | Unique projects with a GitHub pipeline enabled | +| `merge_requests_users` | `usage_activity_by_stage_monthly` | `create` | | | Unique count of users who used a merge request | +| `duration_s` | `topology` | `enablement` | | | Time it took to collect topology data | +| `application_requests_per_hour` | `topology` | `enablement` | | | Number of requests to the web application per hour | +| `failures` | `topology` | `enablement` | | | Contains information about failed queries | +| `nodes` | `topology` | `enablement` | | | The list of server nodes on which GitLab components are running | +| `node_memory_total_bytes` | `topology > nodes` | `enablement` | | | The total available memory of this node | +| `node_cpus` | `topology > nodes` | `enablement` | | | The number of CPU cores of this node | +| `node_uname_info` | `topology > nodes` | `enablement` | | | The basic hardware architecture and OS release information on this node | +| `node_services` | `topology > nodes` | `enablement` | | | The list of GitLab services running on this node | +| `name` | `topology > nodes > node_services` | `enablement` | | | The name of the GitLab service running on this node | +| `process_count` | `topology > nodes > node_services` | `enablement` | | | The number of processes running for this service | +| `process_memory_rss` | `topology > nodes > node_services` | `enablement` | | | The average Resident Set Size of a service process | +| `process_memory_uss` | `topology > nodes > node_services` | `enablement` | | | The average Unique Set Size of a service process | +| `process_memory_pss` | `topology > nodes > node_services` | `enablement` | | | The average Proportional Set Size of a service process | +| `server` | `topology > nodes > node_services` | `enablement` | | | The type of web server used (Unicorn or Puma) | +| `network_policy_forwards` | `counts` | `defend` | | EE | Cumulative count of forwarded packets by Container Network | +| `network_policy_drops` | `counts` | `defend` | | EE | Cumulative count of dropped packets by Container Network | ## Example Usage Ping payload @@ -690,6 +780,7 @@ The following is example content of the Usage Ping payload. "ldap_enabled": false, "mattermost_enabled": false, "omniauth_enabled": true, + "prometheus_enabled": false, "prometheus_metrics_enabled": false, "reply_by_email_enabled": "incoming+%{key}@incoming.gitlab.com", "signup_enabled": true, @@ -765,6 +856,10 @@ The following is example content of the Usage Ping payload. }, "total": 999 }, + "analytics_unique_visits": { + "g_analytics_contribution": 999, + ... + }, "usage_activity_by_stage": { "configure": { "project_clusters_enabled": 999, @@ -840,17 +935,26 @@ The following is example content of the Usage Ping payload. } }, "topology": { + "duration_s": 0.013836685999194742, + "application_requests_per_hour": 4224, + "failures": [], "nodes": [ { "node_memory_total_bytes": 33269903360, "node_cpus": 16, + "node_uname_info": { + "machine": "x86_64", + "sysname": "Linux", + "release": "4.19.76-linuxkit" + }, "node_services": [ { "name": "web", "process_count": 16, "process_memory_pss": 233349888, "process_memory_rss": 788220927, - "process_memory_uss": 195295487 + "process_memory_uss": 195295487, + "server": "puma" }, { "name": "sidekiq", @@ -864,8 +968,7 @@ The following is example content of the Usage Ping payload. ... }, ... - ], - "duration_s": 0.013836685999194742 + ] } } ``` diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 7bb8473117f..4e46e691405 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -110,7 +110,8 @@ Use the coverage reports to ensure your tests cover 100% of your code. ### System / Feature tests -NOTE: **Note:** Before writing a new system test, [please consider **not** +NOTE: **Note:** +Before writing a new system test, [please consider **not** writing one](testing_levels.md#consider-not-writing-a-system-test)! - Feature specs should be named `ROLE_ACTION_spec.rb`, such as @@ -741,7 +742,7 @@ GitLab uses [factory_bot](https://github.com/thoughtbot/factory_bot) as a test f - There should be only one top-level factory definition per file. - FactoryBot methods are mixed in to all RSpec groups. This means you can (and should) call `create(...)` instead of `FactoryBot.create(...)`. -- Make use of [traits](https://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#Traits) to clean up definitions and usages. +- Make use of [traits](https://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#traits) to clean up definitions and usages. - When defining a factory, don't define attributes that are not required for the resulting record to pass validation. - When instantiating from a factory, don't supply attributes that aren't @@ -813,6 +814,40 @@ file which is used by the `spec/fast_spec_helper.rb` file. See [Fast unit tests](#fast-unit-tests) for more details about the `spec/fast_spec_helper.rb` file. +### Test environment logging + +Services for the test environment are automatically configured and started when +tests are run, including Gitaly, Workhorse, Elasticsearch, and Capybara. When run in CI, or +if the service needs to be installed, the test environment will log information +about set-up time, producing log messages like the following: + +```plaintext +==> Setting up Gitaly... + Gitaly set up in 31.459649 seconds... + +==> Setting up GitLab Workhorse... + GitLab Workhorse set up in 29.695619 seconds... +fatal: update refs/heads/diff-files-symlink-to-image: invalid <newvalue>: 8cfca84 +From https://gitlab.com/gitlab-org/gitlab-test + * [new branch] diff-files-image-to-symlink -> origin/diff-files-image-to-symlink + * [new branch] diff-files-symlink-to-image -> origin/diff-files-symlink-to-image + * [new branch] diff-files-symlink-to-text -> origin/diff-files-symlink-to-text + * [new branch] diff-files-text-to-symlink -> origin/diff-files-text-to-symlink + b80faa8..40232f7 snippet/multiple-files -> origin/snippet/multiple-files + * [new branch] testing/branch-with-#-hash -> origin/testing/branch-with-#-hash + +==> Setting up GitLab Elasticsearch Indexer... + GitLab Elasticsearch Indexer set up in 26.514623 seconds... +``` + +This information is omitted when running locally and when no action needs +to be performed. If you would always like to see these messages, set the +following environment variable: + +```shell +GITLAB_TESTING_LOG_LEVEL=debug +``` + --- [Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/end_to_end/beginners_guide.md b/doc/development/testing_guide/end_to_end/beginners_guide.md index 73960a2f74d..15a9b4406ab 100644 --- a/doc/development/testing_guide/end_to_end/beginners_guide.md +++ b/doc/development/testing_guide/end_to_end/beginners_guide.md @@ -62,7 +62,7 @@ end-to-end flows, and is easiest to understand. The GitLab QA end-to-end tests are organized by the different [stages in the DevOps lifecycle](https://gitlab.com/gitlab-org/gitlab-foss/tree/master/qa/qa/specs/features/browser_ui). Determine where the test should be placed by -[stage](https://about.gitlab.com/handbook/product/categories/#devops-stages), +[stage](https://about.gitlab.com/handbook/product/product-categories/#devops-stages), determine which feature the test will belong to, and then place it in a subdirectory under the stage. @@ -80,13 +80,21 @@ file `basic_login_spec.rb`. ### The outer `context` block -Specs have an outer `context` indicating the DevOps stage. +See the [`RSpec.describe` outer block](#the-outer-rspecdescribe-block) + +CAUTION: **Deprecation notice:** +The outer `context` [was deprecated](https://gitlab.com/gitlab-org/quality/team-tasks/-/issues/550) in `13.2` +in adherance to RSpec 4.0 specifications. Use `RSpec.describe` instead. + +### The outer `RSpec.describe` block + +Specs have an outer `RSpec.describe` indicating the DevOps stage. ```ruby # frozen_string_literal: true module QA - context 'Manage' do + RSpec.describe 'Manage' do end end @@ -94,13 +102,13 @@ end ### The `describe` block -Inside of our outer `context`, describe the feature to test. In this case, `Login`. +Inside of our outer `RSpec.describe`, describe the feature to test. In this case, `Login`. ```ruby # frozen_string_literal: true module QA - context 'Manage' do + RSpec.describe 'Manage' do describe 'Login' do end @@ -115,7 +123,7 @@ writing end-to-end tests is to write test case descriptions as `it` blocks: ```ruby module QA - context 'Manage' do + RSpec.describe 'Manage' do describe 'Login' do it 'can login' do @@ -139,7 +147,7 @@ Begin by logging in. # frozen_string_literal: true module QA - context 'Manage' do + RSpec.describe 'Manage' do describe 'Login' do it 'can login' do Flow::Login.sign_in @@ -162,7 +170,7 @@ should answer the question "What do we test?" # frozen_string_literal: true module QA - context 'Manage' do + RSpec.describe 'Manage' do describe 'Login' do it 'can login' do Flow::Login.sign_in @@ -210,7 +218,7 @@ a call to `sign_in`. # frozen_string_literal: true module QA - context 'Manage' do + RSpec.describe 'Manage' do describe 'Login' do before do Flow::Login.sign_in @@ -247,7 +255,7 @@ stage, so [create a file](#identify-the-devops-stage) in # frozen_string_literal: true module QA - context 'Plan' do + RSpec.describe 'Plan' do describe 'Issues' do let(:issue) do Resource::Issue.fabricate_via_api! do |issue| diff --git a/doc/development/testing_guide/end_to_end/environment_selection.md b/doc/development/testing_guide/end_to_end/environment_selection.md new file mode 100644 index 00000000000..9eb7db72a51 --- /dev/null +++ b/doc/development/testing_guide/end_to_end/environment_selection.md @@ -0,0 +1,54 @@ +# Environment selection + +Some tests are designed to be run against specific environments. We can specify +what environments to run tests against using the `only` metadata. + +## Available switches + +| Switch | Function | Type | +| -------| ------- | ----- | +| `tld` | Set the top-level domain matcher | `String` | +| `subdomain` | Set the subdomain matcher | `Array` or `String` | +| `domain` | Set the domain matcher | `String` | +| `production` | Match against production | `Static` | + +CAUTION: **Caution:** +You cannot specify `:production` and `{ <switch>: 'value' }` simultaneously. +These options are mutually exclusive. If you want to specify production, you +can control the `tld` and `domain` independently. + +## Examples + +| Environment | Key | Matches (regex) | +| ---------------- | --- | --------------- | +| `any` | `` | `.+.com` | +| `gitlab.com` | `only: :production` | `gitlab.com` | +| `staging.gitlab.com` | `only: { subdomain: :staging }` | `(staging).+.com` | +| `gitlab.com and staging.gitlab.com` | `only: { subdomain: /(staging.)?/, domain: 'gitlab' }` | `(staging.)?gitlab.com` | +| `dev.gitlab.org` | `only: { tld: '.org', domain: 'gitlab', subdomain: 'dev' }` | `(dev).gitlab.org` | +| `staging.gitlab.com & domain.gitlab.com` | `only: { subdomain: %i[staging domain] }` | `(staging|domain).+.com` | + +```ruby +RSpec.describe 'Area' do + it 'runs in any environment' do; end + + it 'runs only in production', only: :production do; end + + it 'runs only in staging', only: { subdomain: :staging } do; end + + it 'runs in dev', only: { tld: '.org', domain: 'gitlab', subdomain: 'dev' } do; end + + it 'runs in prod and staging', only: { subdomain: /(staging.)?/, domain: 'gitlab' } {} +end +``` + +NOTE: **Note:** +If the test has a `before` or `after`, you must add the `only` metadata +to the outer `RSpec.describe`. + +## Quarantining a test for a specific environment + +Similarly to specifying that a test should only run against a specific environment, it's also possible to quarantine a +test only when it runs against a specific environment. The syntax is exactly the same, except that the `only: { ... }` +hash is nested in the [`quarantine: { ... }`](https://about.gitlab.com/handbook/engineering/quality/guidelines/debugging-qa-test-failures/#quarantining-tests) hash. +For instance, `quarantine: { only: { subdomain: :staging } }` will only quarantine the test when run against staging. diff --git a/doc/development/testing_guide/end_to_end/feature_flags.md b/doc/development/testing_guide/end_to_end/feature_flags.md index ada20cc9dad..87a9738b313 100644 --- a/doc/development/testing_guide/end_to_end/feature_flags.md +++ b/doc/development/testing_guide/end_to_end/feature_flags.md @@ -7,7 +7,7 @@ Note that administrator authorization is required to change feature flags. `QA:: Please be sure to include the tag `:requires_admin` so that the test can be skipped in environments where admin access is not available. ```ruby -context "with feature flag enabled", :requires_admin do +RSpec.describe "with feature flag enabled", :requires_admin do before do Runtime::Feature.enable('feature_flag_name') end diff --git a/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md b/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md index b1a8a14163c..4059c1960e2 100644 --- a/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md +++ b/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md @@ -10,10 +10,11 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec | `:elasticsearch` | The test requires an Elasticsearch service. It is used by the [instance-level scenario](https://gitlab.com/gitlab-org/gitlab-qa#definitions) [`Test::Integration::Elasticsearch`](https://gitlab.com/gitlab-org/gitlab/-/blob/72b62b51bdf513e2936301cb6c7c91ec27c35b4d/qa/qa/ee/scenario/test/integration/elasticsearch.rb) to include only tests that require Elasticsearch. | | `:kubernetes` | The test includes a GitLab instance that is configured to be run behind an SSH tunnel, allowing a TLS-accessible GitLab. This test will also include provisioning of at least one Kubernetes cluster to test against. *This tag is often be paired with `:orchestrated`.* | | `:orchestrated` | The GitLab instance under test may be [configured by `gitlab-qa`](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/master/docs/what_tests_can_be_run.md#orchestrated-tests) to be different to the default GitLab configuration, or `gitlab-qa` may launch additional services in separate Docker containers, or both. Tests tagged with `:orchestrated` are excluded when testing environments where we can't dynamically modify GitLab's configuration (for example, Staging). | -| `:quarantine` | The test has been [quarantined](https://about.gitlab.com/handbook/engineering/quality/guidelines/debugging-qa-test-failures/#quarantining-tests), will run in a separate job that only includes quarantined tests, and is allowed to fail. The test will be skipped in its regular job so that if it fails it will not hold up the pipeline. | +| `:quarantine` | The test has been [quarantined](https://about.gitlab.com/handbook/engineering/quality/guidelines/debugging-qa-test-failures/#quarantining-tests), will run in a separate job that only includes quarantined tests, and is allowed to fail. The test will be skipped in its regular job so that if it fails it will not hold up the pipeline. Note that you can also [quarantine a test only when it runs against specific environment](environment_selection.md#quarantining-a-test-for-a-specific-environment). | | `:reliable` | The test has been [promoted to a reliable test](https://about.gitlab.com/handbook/engineering/quality/guidelines/reliable-tests/#promoting-an-existing-test-to-reliable) meaning it passes consistently in all pipelines, including merge requests. | | `:requires_admin` | The test requires an admin account. Tests with the tag are excluded when run against Canary and Production environments. | | `:runner` | The test depends on and will set up a GitLab Runner instance, typically to run a pipeline. | | `:gitaly_ha` | The test will run against a GitLab instance where repositories are stored on redundant Gitaly nodes behind a Praefect node. All nodes are [separate containers](../../../administration/gitaly/praefect.md#requirements-for-configuring-a-gitaly-cluster). Tests that use this tag have a longer setup time since there are three additional containers that need to be started. | | `:skip_live_env` | The test will be excluded when run against live deployed environments such as Staging, Canary, and Production. | | `:jira` | The test requires a Jira Server. [GitLab-QA](https://gitlab.com/gitlab-org/gitlab-qa) will provision the Jira Server in a Docker container when the `Test::Integration::Jira` test scenario is run. +| `:only` | The test is only to be run against specific environments. See [Environment selection](environment_selection.md) for more information. | diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 37e1066e7aa..ef9fd748dbb 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -545,6 +545,99 @@ In order to ensure that a clean wrapper object and DOM are being used in each te See also the [Vue Test Utils documentation on `destroy`](https://vue-test-utils.vuejs.org/api/wrapper/#destroy). +### Jest best practices + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34209) in GitLab 13.2. + +#### Prefer `toBe` over `toEqual` when comparing primitive values + +Jest has [`toBe`](https://jestjs.io/docs/en/expect#tobevalue) and +[`toEqual`](https://jestjs.io/docs/en/expect#toequalvalue) matchers. +As [`toBe`](https://jestjs.io/docs/en/expect#tobevalue) uses +[`Object.is`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is) +to compare values, it's faster (by default) than using `toEqual`. +While the latter will eventually fallback to leverage [`Object.is`](https://github.com/facebook/jest/blob/master/packages/expect/src/jasmineUtils.ts#L91), +for primitive values, it should only be used when complex objects need a comparison. + +Examples: + +```javascript +const foo = 1; + +// good +expect(foo).toBe(1); + +// bad +expect(foo).toEqual(1); +``` + +#### Prefer more befitting matchers + +Jest provides useful matchers like `toHaveLength` or `toBeUndefined` to make your tests more +readable and to produce more understandable error messages. Check their docs for the +[full list of matchers](https://jestjs.io/docs/en/expect#methods). + +Examples: + +```javascript +const arr = [1, 2]; + +// prints: +// Expected length: 1 +// Received length: 2 +expect(arr).toHaveLength(1); + +// prints: +// Expected: 1 +// Received: 2 +expect(arr.length).toBe(1); + +// prints: +// expect(received).toBe(expected) // Object.is equality +// Expected: undefined +// Received: "bar" +const foo = 'bar'; +expect(foo).toBe(undefined); + +// prints: +// expect(received).toBeUndefined() +// Received: "bar" +const foo = 'bar'; +expect(foo).toBeUndefined(); +``` + +#### Avoid using `toBeTruthy` or `toBeFalsy` + +Jest also provides following matchers: `toBeTruthy` and `toBeFalsy`. We should not use them because +they make tests weaker and produce false-positive results. + +For example, `expect(someBoolean).toBeFalsy()` passes when `someBoolean === null`, and when +`someBoolean === false`. + +#### Tricky `toBeDefined` matcher + +Jest has the tricky `toBeDefined` matcher that can produce false positive test. Because it +[validates](https://github.com/facebook/jest/blob/master/packages/expect/src/matchers.ts#L204) +the given value for `undefined` only. + +```javascript +// good +expect(wrapper.find('foo').exists()).toBe(true); + +// bad +// if finder returns null, the test will pass +expect(wrapper.find('foo')).toBeDefined(); +``` + +#### Avoid using `setImmediate` + +Try to avoid using `setImmediate`. `setImmediate` is an ad-hoc solution to run your callback after +the I/O completes. And it's not part of the Web API, hence, we target NodeJS environments in our +unit tests. + +Instead of `setImmediate`, use `jest.runAllTimers` or `jest.runOnlyPendingTimers` to run pending timers. +The latter is useful when you have `setInterval` in the code. **Remember:** our Jest configuration uses fake timers. + ## Factories TBU @@ -856,7 +949,8 @@ Some regressions only affect a specific browser version. We can install and test [BrowserStack](https://www.browserstack.com/) allows you to test more than 1200 mobile devices and browsers. You can use it directly through the [live app](https://www.browserstack.com/live) or you can install the [chrome extension](https://chrome.google.com/webstore/detail/browserstack/nkihdmlheodkdfojglpcjjmioefjahjb) for easy access. -You can find the credentials on 1Password, under `frontendteam@gitlab.com`. +Sign in to BrowserStack with the credentials saved in the **Engineering** vault of GitLab's +[shared 1Password account](https://about.gitlab.com/handbook/security/#1password-guide). ### Firefox diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 3d7aea89e73..54f8ca0d98b 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -9,7 +9,7 @@ pipeline](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6665). ```mermaid graph TD - A["build-qa-image, gitlab:assets:compile pull-cache<br/>(canonical default refs only)"]; + A["build-qa-image, compile-production-assets<br/>(canonical default refs only)"]; B[review-build-cng]; C[review-deploy]; D[CNG-mirror]; @@ -30,7 +30,7 @@ subgraph "2. gitlab `review-prepare` stage" end subgraph "3. gitlab `review` stage" - C["review-deploy<br><br>Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br><br>Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`<br>Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] + C["review-deploy<br><br>Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br><br>Cloud Native images are deployed to the `review-apps`<br>Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] end subgraph "4. gitlab `qa` stage" @@ -44,25 +44,27 @@ subgraph "CNG-mirror pipeline" ### Detailed explanation -1. On every [pipeline](https://gitlab.com/gitlab-org/gitlab/pipelines/125315730) during the `test` stage, the - [`gitlab:assets:compile`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724487) job is automatically started. - - Once it's done, it starts the [`review-build-cng`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724808) - manual job since the [`CNG-mirror`](https://gitlab.com/gitlab-org/build/CNG-mirror) pipeline triggered in the +1. On every [pipeline](https://gitlab.com/gitlab-org/gitlab/pipelines/125315730) during the `prepare` stage, the + [`compile-production-assets`](https://gitlab.com/gitlab-org/gitlab/-/jobs/641770154) job is automatically started. + - Once it's done, the [`review-build-cng`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724808) + job starts since the [`CNG-mirror`](https://gitlab.com/gitlab-org/build/CNG-mirror) pipeline triggered in the following step depends on it. -1. The [`review-build-cng`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724808) job [triggers a pipeline](https://gitlab.com/gitlab-org/build/CNG-mirror/pipelines/44364657) +1. Once `compile-production-assets` is done, the [`review-build-cng`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724808) + job [triggers a pipeline](https://gitlab.com/gitlab-org/build/CNG-mirror/pipelines/44364657) in the [`CNG-mirror`](https://gitlab.com/gitlab-org/build/CNG-mirror) project. + - The `review-build-cng` job automatically starts only if your MR includes + [CI or frontend changes](../pipelines.md#changes-patterns). In other cases, the job is manual. - The [`CNG-mirror`](https://gitlab.com/gitlab-org/build/CNG-mirror/pipelines/44364657) pipeline creates the Docker images of each component (e.g. `gitlab-rails-ee`, `gitlab-shell`, `gitaly` etc.) based on the commit from the [GitLab pipeline](https://gitlab.com/gitlab-org/gitlab/pipelines/125315730) and stores them in its [registry](https://gitlab.com/gitlab-org/build/CNG-mirror/container_registry). - We use the [`CNG-mirror`](https://gitlab.com/gitlab-org/build/CNG-mirror) project so that the `CNG`, (Cloud - Native GitLab), project's registry is not overloaded with a - lot of transient Docker images. + Native GitLab), project's registry is not overloaded with a lot of transient Docker images. - Note that the official CNG images are built by the `cloud-native-image` job, which runs only for tags, and triggers itself a [`CNG`](https://gitlab.com/gitlab-org/build/CNG) pipeline. -1. Once the `test` stage is done, the [`review-deploy`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724810) job +1. Once `review-build-cng` is done, the [`review-deploy`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724810) job deploys the Review App using [the official GitLab Helm chart](https://gitlab.com/gitlab-org/charts/gitlab/) to - the [`review-apps-ce`](https://console.cloud.google.com/kubernetes/clusters/details/us-central1-a/review-apps-ce?project=gitlab-review-apps) / [`review-apps-ee`](https://console.cloud.google.com/kubernetes/clusters/details/us-central1-b/review-apps-ee?project=gitlab-review-apps) + the [`review-apps`](https://console.cloud.google.com/kubernetes/clusters/details/us-central1-b/review-apps?project=gitlab-review-apps) Kubernetes cluster on GCP. - The actual scripts used to deploy the Review App can be found at [`scripts/review_apps/review-apps.sh`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/scripts/review_apps/review-apps.sh). @@ -94,10 +96,9 @@ subgraph "CNG-mirror pipeline" - The manual `review-stop` can be used to stop a Review App manually, and is also started by GitLab once a merge request's branch is deleted after being merged. -- The Kubernetes cluster is connected to the `gitlab-{ce,ee}` projects using +- The Kubernetes cluster is connected to the `gitlab` projects using [GitLab's Kubernetes integration](../../user/project/clusters/index.md). This basically - allows to have a link to the Review App directly from the merge request - widget. + allows to have a link to the Review App directly from the merge request widget. ### Auto-stopping of Review Apps @@ -136,11 +137,10 @@ browser performance testing using a ### Node pools -The `review-apps-ee` and `review-apps-ce` clusters are currently set up with +The `review-apps` cluster is currently set up with the following node pools: -- `review-apps-ee` of pre-emptible `e2-highcpu-16` (16 vCPU, 16 GB memory) nodes with autoscaling -- `review-apps-ce` of pre-emptible `n1-standard-8` (8 vCPU, 16 GB memory) nodes with autoscaling +- `e2-highcpu-16` (16 vCPU, 16 GB memory) pre-emptible nodes with autoscaling ### Helm @@ -189,9 +189,7 @@ secure note named `gitlab-{ce,ee} Review App's root password`. 1. Click on the `KUBECTL` dropdown, then `Exec` -> `task-runner`. 1. Replace `-c task-runner -- ls` with `-it -- gitlab-rails console` from the default command or - - Run `kubectl exec --namespace review-apps-ce review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz -it -- gitlab-rails console` and - - Replace `review-apps-ce` with `review-apps-ee` if the Review App - is running EE, and + - Run `kubectl exec --namespace review-apps review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz -it -- gitlab-rails console` and - Replace `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz` with your Pod's name. diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index 2210ec94696..88d7cf9ca08 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -482,7 +482,7 @@ Every new feature should come with a [test plan](https://gitlab.com/gitlab-org/g | Tests path | Testing engine | Notes | | ---------- | -------------- | ----- | -| `qa/qa/specs/features/` | [Capybara](https://github.com/teamcapybara/capybara) + [RSpec](https://github.com/rspec/rspec-rails#feature-specs) + Custom QA framework | Tests should be placed under their corresponding [Product category](https://about.gitlab.com/handbook/product/categories/) | +| `qa/qa/specs/features/` | [Capybara](https://github.com/teamcapybara/capybara) + [RSpec](https://github.com/rspec/rspec-rails#feature-specs) + Custom QA framework | Tests should be placed under their corresponding [Product category](https://about.gitlab.com/handbook/product/product-categories/) | > See [end-to-end tests](end_to_end/index.md) for more information. diff --git a/doc/development/testing_guide/testing_migrations_guide.md b/doc/development/testing_guide/testing_migrations_guide.md index a03b940fe40..8ee758177c3 100644 --- a/doc/development/testing_guide/testing_migrations_guide.md +++ b/doc/development/testing_guide/testing_migrations_guide.md @@ -112,7 +112,7 @@ migration. You can find the complete spec in require 'spec_helper' require Rails.root.join('db', 'post_migrate', '20170526185842_migrate_pipeline_stages.rb') -describe MigratePipelineStages do +RSpec.describe MigratePipelineStages do # Create test data - pipeline and CI/CD jobs. let(:jobs) { table(:ci_builds) } let(:stages) { table(:ci_stages) } diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 407899b23d5..d0b8aa18f5c 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -130,7 +130,8 @@ class CleanupUsersUpdatedAtRename < ActiveRecord::Migration[4.2] end ``` -NOTE: **Note:** If you're renaming a [large table](https://gitlab.com/gitlab-org/gitlab/-/blob/master/rubocop/rubocop-migrations.yml#L3), please carefully consider the state when the first migration has run but the second cleanup migration hasn't been run yet. +NOTE: **Note:** +If you're renaming a [large table](https://gitlab.com/gitlab-org/gitlab/-/blob/master/rubocop/rubocop-migrations.yml#L3), please carefully consider the state when the first migration has run but the second cleanup migration hasn't been run yet. With [Canary](https://about.gitlab.com/handbook/engineering/infrastructure/library/canary/) it is possible that the system runs in this state for a significant amount of time. ## Changing Column Constraints @@ -202,6 +203,21 @@ end And that's it, we're done! +### Casting data to a new type + +Some type changes require casting data to a new type. For example when changing from `text` to `jsonb`. +In this case, use the `type_cast_function` option. +Make sure there is no bad data and the cast will always succeed. You can also provide a custom function that handles +casting errors. + +Example migration: + +```ruby + def up + change_column_type_concurrently :users, :settings, :jsonb, type_cast_function: 'jsonb' + end +``` + ## Changing The Schema For Large Tables While `change_column_type_concurrently` and `rename_column_concurrently` can be @@ -317,30 +333,11 @@ migrations](background_migrations.md#cleaning-up). ## Adding Indexes -Adding indexes is an expensive process that blocks INSERT and UPDATE queries for -the duration. You can work around this by using the `CONCURRENTLY` option: - -```sql -CREATE INDEX CONCURRENTLY index_name ON projects (column_name); -``` - -Migrations can take advantage of this by using the method -`add_concurrent_index`. For example: - -```ruby -class MyMigration < ActiveRecord::Migration[4.2] - def up - add_concurrent_index :projects, :column_name - end - - def down - remove_index(:projects, :column_name) if index_exists?(:projects, :column_name) - end -end -``` +Adding indexes does not require downtime when `add_concurrent_index` +is used. -Note that `add_concurrent_index` can not be reversed automatically, thus you -need to manually define `up` and `down`. +See also [Migration Style Guide](migration_style_guide.md#adding-indexes) +for more information. ## Dropping Indexes |
