diff options
Diffstat (limited to 'doc/development')
72 files changed, 744 insertions, 304 deletions
diff --git a/doc/development/agent/index.md b/doc/development/agent/index.md new file mode 100644 index 00000000000..4361863daeb --- /dev/null +++ b/doc/development/agent/index.md @@ -0,0 +1,83 @@ +--- +stage: Configure +group: Configure +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 +--- + +# Kubernetes Agent development **(PREMIUM ONLY)** + +This page contains developer-specific information about the GitLab Kubernetes Agent. +[End-user documentation about the GitLab Kubernetes Agent](../../user/clusters/agent/index.md) +is also available. + +The agent can help you perform tasks like these: + +- Integrate a cluster, located behind a firewall or NAT, with GitLab. To + learn more, read [issue #212810, Invert the model GitLab.com uses for Kubernetes integration by leveraging long lived reverse tunnels](https://gitlab.com/gitlab-org/gitlab/-/issues/212810). +- Access API endpoints in a cluster in real time. For an example use case, read + [issue #218220, Allow Prometheus in K8s cluster to be installed manually](https://gitlab.com/gitlab-org/gitlab/-/issues/218220#note_348729266). +- Enable real-time features by pushing information about events happening in a cluster. + For example, you could build a cluster view dashboard to visualize changes in progress + in a cluster. For more information about these efforts, read about the + [Real-Time Working Group](https://about.gitlab.com/company/team/structure/working-groups/real-time/). +- Enable a [cache of Kubernetes objects through informers](https://github.com/kubernetes/client-go/blob/ccd5becdffb7fd8006e31341baaaacd14db2dcb7/tools/cache/shared_informer.go#L34-L183), + kept up-to-date with very low latency. This cache helps you: + + - Reduce or eliminate information propagation latency by avoiding Kubernetes API calls + and polling, and only fetching data from an up-to-date cache. + - Lower the load placed on the Kubernetes API by removing polling. + - Eliminate any rate-limiting errors by removing polling. + - Simplify backend code by replacing polling code with cache access. While it's another + API call, no polling is needed. This example describes [fetching cached data synchronously from the front end](https://gitlab.com/gitlab-org/gitlab/-/issues/217792#note_348582537) instead of fetching data from the Kubernetes API. + +## Architecture of the Kubernetes Agent + +The GitLab Kubernetes Agent and the GitLab Kubernetes Agent Server use +[bidirectional streaming](https://grpc.io/docs/guides/concepts/#bidirectional-streaming-rpc) +to allow the connection acceptor (the gRPC server, GitLab Kubernetes Agent Server) to +act as a client. The connection acceptor sends requests as gRPC replies. The client-server +relationship is inverted because the connection must be initiated from inside the +Kubernetes cluster to bypass any firewall or NAT the cluster may be located behind. +To learn more about this inversion, read +[issue #212810](https://gitlab.com/gitlab-org/gitlab/-/issues/212810). + +This diagram describes how GitLab (`GitLab RoR`), the GitLab Kubernetes Agent (`agentk`), and the GitLab Kubernetes Agent Server (`kas`) work together. + +```mermaid +graph TB + agentk -- gRPC bidirectional streaming --> kas + + subgraph "GitLab" + kas[kas] + GitLabRoR[GitLab RoR] -- gRPC --> kas + kas -- gRPC --> Gitaly[Gitaly] + kas -- REST API --> GitLabRoR + end + + subgraph "Kubernetes cluster" + agentk[agentk] + end +``` + +- `GitLab RoR` is the main GitLab application. It uses gRPC to talk to `kas`. +- `agentk` is the GitLab Kubernetes Agent. It keeps a connection established to a + `kas` instance, waiting for requests to process. It may also actively send information + about things happening in the cluster. +- `kas` is the GitLab Kubernetes Agent Server, and is responsible for: + - Accepting requests from `agentk`. + - [Authentication of requests](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/doc/identity_and_auth.md) from `agentk` by querying `GitLab RoR`. + - Fetching agent's configuration from a corresponding Git repository by querying Gitaly. + - Matching incoming requests from `GitLab RoR` with existing connections from + the right `agentk`, forwarding requests to it and forwarding responses back. + - (Optional) Sending notifications through ActionCable for events received from `agentk`. + - Polling manifest repositories for [GitOps support](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/doc/gitops.md) by communicating with Gitaly. + +## Guiding principles + +GitLab prefers to add logic into `kas` rather than `agentk`. `agentk` should be kept +streamlined and small to minimize the need for upgrades. On GitLab.com, `kas` is +managed by GitLab, so upgrades and features can be added without requiring you +to upgrade `agentk` in your clusters. + +`agentk` can't be viewed as a dumb reverse proxy because features are planned to be built +[on top of the cache with informers](https://github.com/kubernetes/client-go/blob/ccd5becdffb7fd8006e31341baaaacd14db2dcb7/tools/cache/shared_informer.go#L34-L183). diff --git a/doc/development/cached_queries.md b/doc/development/cached_queries.md index 812e5f88754..16ee831f7bf 100644 --- a/doc/development/cached_queries.md +++ b/doc/development/cached_queries.md @@ -1,80 +1,101 @@ -# Cached queries guidelines +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- -Rails provides an [SQL query cache](https://guides.rubyonrails.org/caching_with_rails.html#sql-caching), -used to cache the results of database queries for the duration of the request. +# Cached queries guidelines -If Rails encounters the same query again for that request, -it will use the cached result set as opposed to running the query against the database again. +Rails provides an [SQL query cache](https://guides.rubyonrails.org/caching_with_rails.html#sql-caching) +which is used to cache the results of database queries for the duration of a request. +When Rails encounters the same query again within the same request, it uses the cached +result set instead of running the query against the database again. -The query results are only cached for the duration of that single request, it does not persist across multiple requests. +The query results are only cached for the duration of that single request, and +don't persist across multiple requests. ## Why cached queries are considered bad -The cached queries help with reducing DB load, but they still: +Cached queries help by reducing the load on the database, but they still: - Consume memory. -- Require as to re-instantiate each `ActiveRecord` object. -- Require as to re-instantiate each relation of the object. -- Make us spend additional CPU-cycles to look into a list of cached queries. - -The Cached SQL queries are cheaper, but they are not cheap at all from `memory` perspective. -They could mask [N+1 query problem](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations), -so we should threat them the same way we threat regular N+1 queries. +- Require Rails to re-instantiate each `ActiveRecord` object. +- Require Rails to re-instantiate each relation of the object. +- Make us spend additional CPU cycles to look into a list of cached queries. + +Although cached queries are cheaper from a database perspective, they are potentially +more expensive from a memory perspective. They could mask +[N+1 query problems](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations), +so you should treat them the same way you treat regular N+1 queries. -In case of N+1 queries, masked with cached queries, we are executing the same query N times. -It will not hit the database N times, it will return the cached results instead. -This is still expensive since we need to re-initialize objects each time, and this is CPU/Memory expensive. -Instead, we should use the same in-memory objects, if possible. +In cases of N+1 queries masked by cached queries, the same query is executed N times. +It will not hit the database N times but instead returns the cached results N times. +This is still expensive because you need to re-initialize objects each time at a +greater expense to the CPU and memory resources. Instead, you should use the same +in-memory objects whenever possible. -When we introduce a new feature, we should avoid N+1 problems, -minimize the [query count](merge_request_performance_guidelines.md#query-counts), and pay special attention that [cached -queries](merge_request_performance_guidelines.md#cached-queries) are not masking N+1 problems. +When you introduce a new feature, you should: -## How to detect +- Avoid N+1 queries. +- Minimize the [query count](merge_request_performance_guidelines.md#query-counts). +- Pay special attention to ensure + [cached queries](merge_request_performance_guidelines.md#cached-queries) are not + masking N+1 problems. + +## How to detect cached queries ### Detect potential offenders by using Kibana -On GitLab.com, we are logging entries with the number of executed cached queries in the -`pubsub-redis-inf-gprd*` index with the [`db_cached_count`](https://log.gprd.gitlab.net/goto/77d18d80ad84c5df1bf1da5c2cd35b82). -We can filter endpoints that have a large number of executed cached queries. For example, if we encounter an endpoint -that has 100+ `db_cached_count`, this could indicate that there is an N+1 problem masked with cached queries. -We should probably investigate this endpoint further, to check if we are executing duplicated cached queries. +GitLab.com, logs entries with the number of executed cached queries in the +`pubsub-redis-inf-gprd*` index as +[`db_cached_count`](https://log.gprd.gitlab.net/goto/77d18d80ad84c5df1bf1da5c2cd35b82). +You can filter by endpoints that have a large number of executed cached queries. For +example, an endpoint with a `db_cached_count` greater than 100 can indicate an N+1 problem which +is masked by cached queries. You should investigate this endpoint further to determine +if it is indeed executing duplicated cached queries. + +For more Kibana visualizations related to cached queries, read +[issue #259007, 'Provide metrics that would help us to detect the potential N+1 CACHED SQL calls'](https://gitlab.com/gitlab-org/gitlab/-/issues/259007). -For more cached queries Kibana visualizations see [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/259007). +### Inspect suspicious endpoints using the Performance Bar -### Inspect suspicious endpoint using Performance Bar +When building features, use the +[performance bar](../administration/monitoring/performance/performance_bar.md) +to view the list of database queries, including cached queries. The +performance bar shows a warning when the number of total executed and cached queries is +greater than 100. -When building features, you could use the [performance bar](../administration/monitoring/performance/performance_bar.md) -to list database queries, which will include cached queries as well. The performance bar will show a warning -when threshold of total executed queries (including cached ones) has exceeded 100 queries. +To learn more about the statistics available to you, read the +[Performance Bar documentation](../administration/monitoring/performance/performance_bar.md). ## What to look for -Using [Kibana](cached_queries.md#detect-potential-offenders-by-using-kibana), you can look for a large number -of executed cached queries. End-points with large number of `db_cached_count` could indicate that there -are probably a lot of duplicated cached queries, which often indicates a masked N+1 problem. +Using [Kibana](#detect-potential-offenders-by-using-kibana), you can look for a large number +of executed cached queries. Endpoints with a large `db_cached_count` could suggest a large number +of duplicated cached queries, which often indicates a masked N+1 problem. -When you investigate specific endpoint, you could use -the [performance bar](cached_queries.md#inspect-suspicious-endpoint-using-performance-bar). -If you see a lot of similar queries, this often indicates an N+1 query issue (or a similar kind of query batching problem). -If you see same cached query executed multiple times, this often indicates a masked N+1 query problem. +When you investigate a specific endpoint, use +the [performance bar](#inspect-suspicious-endpoints-using-the-performance-bar) +to identify similar and cached queries, which may also indicate an N+1 query issue +(or a similar kind of query batching problem). -For example, let's say you wanted to debug `GroupMembers` page. +### An example -In the left corner of the performance bar you could see **Database queries** showing the total number of database queries +For example, let's debug the "Group Members" page. In the left corner of the +performance bar, **Database queries** shows the total number of database queries and the number of executed cached queries:  -We can see that there are 55 cached queries. By clicking on the number, a modal window with more details is shown. -Cached queries are marked with the `cached` label, so they are easy to spot. We can see that there are multiple duplicated -cached queries: +The page included 55 cached queries. Clicking the number displays a modal window +with more details about queries. Cached queries are marked with the `cached` label +below the query. You can see multiple duplicate cached queries in this modal window:  -If we click on `...` for one of them, it will expand the actual stack trace: +Click **...** to expand the actual stack trace: -```shell +```ruby [ "app/models/group.rb:305:in `has_owner?'", "ee/app/views/shared/members/ee/_license_badge.html.haml:1", @@ -99,24 +120,30 @@ If we click on `...` for one of them, it will expand the actual stack trace: ] ``` -The stack trace, shows us that we obviously have an N+1 problem, since we are repeatably executing for each group member: +The stack trace shows an N+1 problem, because the code repeatedly executes +`group.has_owner?(current_user)` for each group member. To solve this issue, +move the repeated line of code outside of the loop, passing the result to each rendered member instead: -```ruby -group.has_owner?(current_user) -``` +```erb +- current_user_is_group_owner = @group && @group.has_owner?(current_user) -This is easily solvable by extracting this check, above the loop. += render partial: 'shared/members/member', + collection: @members, as: :member, + locals: { membership_source: @group, + group: @group, + current_user_is_group_owner: current_user_is_group_owner } +``` -After [the fix](https://gitlab.com/gitlab-org/gitlab/-/issues/231468), we now have: +After [fixing the cached query](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44626/diffs#27c2761d66e496495be07d0925697f7e62b5bd14), the performance bar now shows only +6 cached queries:  ## How to measure the impact of the change -We can use the [memory profiler](performance.md#using-memory-profiler) to profile our code. -For the previous example, we could wrap the profiler around the `Groups::GroupMembersController#index` action. - -We had: +Use the [memory profiler](performance.md#using-memory-profiler) to profile your code. +For [this example](#an-example), wrap the profiler around the `Groups::GroupMembersController#index` action. Before the fix, the application had +the following statistics: - Total allocated: 7133601 bytes (84858 objects) - Total retained: 757595 bytes (6070 objects) @@ -124,7 +151,8 @@ We had: - `db_cached_count`: 55 - `db_duration`: 303ms -After the fix, we can see that we have reduced the allocated memory as well as the number of cached queries and improved execution time: +The fix reduced the allocated memory, and the number of cached queries. These +factors help improve the overall execution time: - Total allocated: 5313899 bytes (65290 objects), 1810KB (25%) less - Total retained: 685593 bytes (5278 objects), 72KB (9%) less @@ -132,7 +160,7 @@ After the fix, we can see that we have reduced the allocated memory as well as t - `db_cached_count`: 6 (89% less) - `db_duration`: 162ms (87% faster) -## See also +## For more information - [Metrics that would help us detect the potential N+1 Cached SQL calls](https://gitlab.com/gitlab-org/gitlab/-/issues/259007) - [Merge Request performance guidelines for cached queries](merge_request_performance_guidelines.md#cached-queries) diff --git a/doc/development/cicd/index.md b/doc/development/cicd/index.md index 30ccc52ec5e..d1f22a559c6 100644 --- a/doc/development/cicd/index.md +++ b/doc/development/cicd/index.md @@ -32,8 +32,8 @@ On the left side we have the events that can trigger a pipeline based on various - When GitHub integration is used with [external pull requests](../../ci/ci_cd_for_external_repos/index.md#pipelines-for-external-pull-requests). - When an upstream pipeline contains a [bridge job](../../ci/yaml/README.md#trigger) which triggers a downstream pipeline. -Triggering any of these events will invoke the [`CreatePipelineService`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/ci/create_pipeline_service.rb) -which takes as input event data and the user triggering it, then will attempt to create a pipeline. +Triggering any of these events invokes the [`CreatePipelineService`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/ci/create_pipeline_service.rb) +which takes as input event data and the user triggering it, then attempts to create a pipeline. The `CreatePipelineService` relies heavily on the [`YAML Processor`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/yaml_processor.rb) component, which is responsible for taking in a YAML blob as input and returns the abstract data structure of a @@ -65,20 +65,20 @@ the `Runner API Gateway`. We can register, delete, and verify runners, which also causes read/write queries to the database. After a runner is connected, it keeps asking for the next job to execute. This invokes the [`RegisterJobService`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/services/ci/register_job_service.rb) -which will pick the next job and assign it to the runner. At this point the job will transition to a +which picks the next job and assigns it to the runner. At this point the job transitions to a `running` state, which again triggers `ProcessPipelineService` due to the status change. For more details read [Job scheduling](#job-scheduling)). While a job is being executed, the runner sends logs back to the server as well any possible artifacts that need to be stored. Also, a job may depend on artifacts from previous jobs in order to run. In this -case the runner will download them using a dedicated API endpoint. +case the runner downloads them using a dedicated API endpoint. Artifacts are stored in object storage, while metadata is kept in the database. An important example of artifacts are reports (JUnit, SAST, DAST, etc.) which are parsed and rendered in the merge request. Job status transitions are not all automated. A user may run [manual jobs](../../ci/yaml/README.md#whenmanual), cancel a pipeline, retry specific failed jobs or the entire pipeline. Anything that -causes a job to change status will trigger `ProcessPipelineService`, as it's responsible for +causes a job to change status triggers `ProcessPipelineService`, as it's responsible for tracking the status of the entire pipeline. A special type of job is the [bridge job](../../ci/yaml/README.md#trigger) which is executed server-side @@ -90,7 +90,7 @@ from the `CreatePipelineService` every time a downstream pipeline is triggered. When a Pipeline is created all its jobs are created at once for all stages, with an initial state of `created`. This makes it possible to visualize the full content of a pipeline. -A job with the `created` state won't be seen by the runner yet. To make it possible to assign a job to a runner, the job must transition first into the `pending` state, which can happen if: +A job with the `created` state isn't seen by the runner yet. To make it possible to assign a job to a runner, the job must transition first into the `pending` state, which can happen if: 1. The job is created in the very first stage of the pipeline. 1. The job required a manual start and it has been triggered. @@ -135,7 +135,7 @@ 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. +If a job contains tags, the runner doesn't 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/code_comments.md b/doc/development/code_comments.md index d9ab719d18a..00e077eda8b 100644 --- a/doc/development/code_comments.md +++ b/doc/development/code_comments.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w Whenever you add comment to the code that is expected to be addressed at any time in future, please create a technical debt issue for it. Then put a link to it -to the code comment you've created. This will allow other developers to quickly +to the code comment you've created. This allows other developers to quickly check if a comment is still relevant and what needs to be done to address it. Examples: diff --git a/doc/development/cycle_analytics.md b/doc/development/cycle_analytics.md index 3947a012bd5..1619f3dcb10 100644 --- a/doc/development/cycle_analytics.md +++ b/doc/development/cycle_analytics.md @@ -3,3 +3,6 @@ redirect_to: 'value_stream_analytics.md' --- This document was moved to [another location](value_stream_analytics.md) + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/database/index.md b/doc/development/database/index.md index 19159c6c0ff..3ab865170ae 100644 --- a/doc/development/database/index.md +++ b/doc/development/database/index.md @@ -59,6 +59,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w - [Client-side connection-pool](client_side_connection_pool.md) - [Updating multiple values](setting_multiple_values.md) - [Constraints naming conventions](constraint_naming_convention.md) +- [Query performance guidelines](../query_performance.md) ## Case studies diff --git a/doc/development/database_review.md b/doc/development/database_review.md index d1ec32af464..5ae0c25c3b5 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -158,8 +158,8 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac - Maintainer: After the merge request is merged, notify Release Managers about it on `#f_upcoming_release` Slack channel. - Check consistency with `db/structure.sql` and that migrations are [reversible](migration_style_guide.md#reversibility) - Check that the relevant version files under `db/schema_migrations` were added or removed. - - Check queries timing (If any): Queries executed in a migration - need to fit comfortably within `15s` - preferably much less than that - on GitLab.com. + - Check queries timing (If any): In a single transaction, cumulative query time executed in a migration + needs to fit comfortably within `15s` - preferably much less than that - on GitLab.com. - For column removals, make sure the column has been [ignored in a previous release](what_requires_downtime.md#dropping-columns) - Check [background migrations](background_migrations.md): - Establish a time estimate for execution on GitLab.com. For historical purposes, @@ -190,7 +190,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac - For given queries, review parameters regarding data distribution - [Check query plans](understanding_explain_plans.md) and suggest improvements to queries (changing the query, schema or adding indexes and similar) - - General guideline is for queries to come in below 100ms execution time + - General guideline is for queries to come in below [100ms execution time](query_performance.md#timing-guidelines-for-queries) - Avoid N+1 problems and minimalize the [query count](merge_request_performance_guidelines.md#query-counts). ### Timing guidelines for migrations @@ -206,4 +206,4 @@ Keep in mind that all runtimes should be measured against GitLab.com. |----|----|---| | Regular migrations on `db/migrate` | `3 minutes` | A valid exception are index creation as this can take a long time. | | Post migrations on `db/post_migrate` | `10 minutes` | | -| Background migrations | --- | Since these are suitable for larger tables, it's not possible to set a precise timing guideline, however, any single query must stay below `1 second` execution time with cold caches. | +| Background migrations | --- | Since these are suitable for larger tables, it's not possible to set a precise timing guideline, however, any single query must stay below [`1 second` execution time](query_performance.md#timing-guidelines-for-queries) with cold caches. | diff --git a/doc/development/distributed_tracing.md b/doc/development/distributed_tracing.md index 6d277f9ae99..53c81901718 100644 --- a/doc/development/distributed_tracing.md +++ b/doc/development/distributed_tracing.md @@ -59,7 +59,7 @@ on non-Go GitLab subsystems. GitLab uses the `GITLAB_TRACING` environment variable to configure distributed tracing. The same configuration is used for all components (e.g., Workhorse, Rails, etc). -When `GITLAB_TRACING` is not set, the application will not be instrumented, meaning that there is +When `GITLAB_TRACING` is not set, the application isn't instrumented, meaning that there is no overhead at all. To enable `GITLAB_TRACING`, a valid _"configuration-string"_ value should be set, with a URL-like @@ -94,8 +94,8 @@ by typing `p` `b` in the browser window. Once the performance bar is enabled, click on the **Trace** link in the performance bar to go to the Jaeger UI. -The Jaeger search UI will return a query for the `Correlation-ID` of the current request. Normally, -this search should return a single trace result. Clicking this result will show the detail of the +The Jaeger search UI returns a query for the `Correlation-ID` of the current request. Normally, +this search should return a single trace result. Clicking this result shows the detail of the trace in a hierarchical time-line.  @@ -154,7 +154,7 @@ This should start the process with the default listening ports. ### 2. Configure the `GITLAB_TRACING` environment variable -Once you have Jaeger running, you'll need to configure the `GITLAB_TRACING` variable with the +Once you have Jaeger running, configure the `GITLAB_TRACING` variable with the appropriate configuration string. **TL;DR:** If you are running everything on the same host, use the following value: @@ -178,7 +178,7 @@ This configuration string uses the Jaeger driver `opentracing://jaeger` with the | `udp_endpoint` | `localhost:6831` | This is the default. Configures Jaeger to send trace information to the UDP listener on port `6831` using compact thrift protocol. Note that we've experienced some issues with the [Jaeger Client for Ruby](https://github.com/salemove/jaeger-client-ruby) when using this protocol. | | `sampler` | `probabalistic` | Configures Jaeger to use a probabilistic random sampler. The rate of samples is configured by the `sampler_param` value. | | `sampler_param` | `0.01` | Use a ratio of `0.01` to configure the `probabalistic` sampler to randomly sample _1%_ of traces. | -| `service_name` | `api` | Override the service name used by the Jaeger backend. This parameter will take precedence over the application-supplied value. | +| `service_name` | `api` | Override the service name used by the Jaeger backend. This parameter takes precedence over the application-supplied value. | NOTE: **Note:** The same `GITLAB_TRACING` value should to be configured in the environment @@ -189,7 +189,7 @@ variables for all GitLab processes, including Workhorse, Gitaly, Rails, and Side After the `GITLAB_TRACING` environment variable is exported to all GitLab services, start the application. -When `GITLAB_TRACING` is configured properly, the application will log this on startup: +When `GITLAB_TRACING` is configured properly, the application logs this on startup: ```shell 13:41:53 gitlab-workhorse.1 | 2019/02/12 13:41:53 Tracing enabled @@ -198,7 +198,7 @@ When `GITLAB_TRACING` is configured properly, the application will log this on s ... ``` -If `GITLAB_TRACING` is not configured correctly, this will also be logged: +If `GITLAB_TRACING` is not configured correctly, this issue is logged: ```shell 13:43:45 gitaly.1 | 2019/02/12 13:43:45 skipping tracing configuration step: tracer: unable to load driver mytracer @@ -216,5 +216,5 @@ not set. By default, the Jaeger search UI is available at <http://localhost:16686/search>. TIP: **Tip:** -Don't forget that you will need to generate traces by using the application before +Don't forget that you must generate traces by using the application before they appear in the Jaeger UI. diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index 4a9f2a626f7..1595a841ade 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -3,3 +3,6 @@ redirect_to: 'documentation/styleguide.md' --- This document was moved to [another location](documentation/styleguide.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/documentation/feature-change-workflow.md b/doc/development/documentation/feature-change-workflow.md index 004d8833e63..78e5510ffca 100644 --- a/doc/development/documentation/feature-change-workflow.md +++ b/doc/development/documentation/feature-change-workflow.md @@ -3,3 +3,6 @@ redirect_to: 'workflow.md' --- This document was moved to [another location](workflow.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/documentation/improvement-workflow.md b/doc/development/documentation/improvement-workflow.md index 004d8833e63..78e5510ffca 100644 --- a/doc/development/documentation/improvement-workflow.md +++ b/doc/development/documentation/improvement-workflow.md @@ -3,3 +3,6 @@ redirect_to: 'workflow.md' --- This document was moved to [another location](workflow.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 69a8ff10878..65472ce026e 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -38,8 +38,8 @@ Documentation issues and merge requests are part of their respective repositorie The [CI pipeline for the main GitLab project](../pipelines.md) is configured to automatically run only the jobs that match the type of contribution. If your contribution contains -**only** documentation changes, then only documentation-related jobs will be run, and -the pipeline will complete much faster than a code contribution. +**only** documentation changes, then only documentation-related jobs run, and +the pipeline completes much faster than a code contribution. If you are submitting documentation-only changes to Runner, Omnibus, or Charts, the fast pipeline is not determined automatically. Instead, create branches for @@ -152,7 +152,7 @@ comments: false 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 displayed at the top of the page if defined: +Nanoc layout), which is displayed at the top of the page if defined: - `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 @@ -225,9 +225,9 @@ Things to note: the document might also be referenced in the views of GitLab (`app/`) which will render when visiting `/help`, and sometimes in the testing suite (`spec/`). You must search these paths for references to the doc and update them as well. -- The above `git grep` command will search recursively in the directory you run +- The above `git grep` command searches recursively in the directory you run it in for `workflow/lfs/lfs_administration` and `lfs/lfs_administration` - and will print the file and the line where this file is mentioned. + and prints the file and the line where this file is mentioned. You may ask why the two greps. Since [we use relative paths to link to documentation](styleguide/index.md#links), sometimes it might be useful to search a path deeper. - The `*.md` extension is not used when a document is linked to GitLab's @@ -267,7 +267,7 @@ Before getting started, make sure you read the introductory section - Label the MR `Documentation` (can only be done by people with `developer` access, for example, GitLab team members) - Assign the correct milestone per note below (can only be done by people with `developer` access, for example, GitLab team members) -Documentation will be merged if it is an improvement on existing content, +Documentation is merged if it is an improvement on existing content, represents a good-faith effort to follow the template and style standards, and is believed to be accurate. @@ -285,16 +285,16 @@ Every GitLab instance includes the documentation, which is available at `/help` (`https://gitlab.example.com/help`). For example, <https://gitlab.com/help>. 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 +after a merge request gets merged, it is available online on the same day. +However, it's shipped (and available on `/help`) within the milestone assigned to the MR. For example, let's say your merge request has a milestone set to 11.3, which -will be released on 2018-09-22. If it gets merged on 2018-09-15, it will be +a release date of 2018-09-22. If it gets merged on 2018-09-15, it is available online on 2018-09-15, but, as the feature freeze date has passed, if the MR does not have a `~"Pick into 11.3"` label, the milestone has to be changed -to 11.4 and it will be shipped with all GitLab packages only on 2018-10-22, -with GitLab 11.4. Meaning, it will only be available under `/help` from GitLab +to 11.4 and it ships with all GitLab packages only on 2018-10-22, +with GitLab 11.4. Meaning, it's available only with `/help` from GitLab 11.4 onward, but available on <https://docs.gitlab.com/> on the same day it was merged. ### Linking to `/help` @@ -365,7 +365,7 @@ You can combine one or more of the following: ### GitLab `/help` tests Several [RSpec tests](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/features/help_pages_spec.rb) -are run to ensure GitLab documentation renders and works correctly. In particular, that [main docs landing page](../../README.md) will work correctly from `/help`. +are run to ensure GitLab documentation renders and works correctly. In particular, that [main docs landing page](../../README.md) works correctly from `/help`. For example, [GitLab.com's `/help`](https://gitlab.com/help). ## Docs site architecture @@ -392,20 +392,20 @@ The live preview is currently enabled for the following projects: If your merge request has docs changes, you can use the manual `review-docs-deploy` job to deploy the docs review app for your merge request. -You will need at least Maintainer permissions to be able to run it. +You need at least Maintainer permissions to be able to run it.  You must push a branch to those repositories, as it doesn't work for forks. -The `review-docs-deploy*` job will: +The `review-docs-deploy*` job: -1. Create a new branch in the [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) +1. Creates a new branch in the [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) project named after the scheme: `docs-preview-$DOCS_GITLAB_REPO_SUFFIX-$CI_MERGE_REQUEST_IID`, where `DOCS_GITLAB_REPO_SUFFIX` is the suffix for each product, e.g, `ee` for EE, `omnibus` for Omnibus GitLab, etc, and `CI_MERGE_REQUEST_IID` is the ID of the respective merge request. -1. Trigger a cross project pipeline and build the docs site with your changes. +1. Triggers a cross project pipeline and build the docs site with your changes. In case the review app URL returns 404, this means that either the site is not yet deployed, or something went wrong with the remote pipeline. Give it a few @@ -414,8 +414,8 @@ remote pipeline from the link in the merge request's job output. If the pipeline failed or got stuck, drop a line in the `#docs` chat channel. Make sure that you always delete the branch of the merge request you were -working on. If you don't, the remote docs branch won't be removed either, -and the server where the Review Apps are hosted will eventually be out of +working on. If you don't, the remote docs branch isn't removed either, +and the server where the Review Apps are hosted can eventually run out of disk space. TIP: **Tip:** @@ -449,7 +449,7 @@ If you want to know the in-depth details, here's what's really happening: - The number of the merge request is added so that you can know by the `gitlab-docs` branch name the merge request it originated from. 1. The remote branch is then created if it doesn't exist (meaning you can - re-run the manual job as many times as you want and this step will be skipped). + re-run the manual job as many times as you want and this step is skipped). 1. A new cross-project pipeline is triggered in the docs project. 1. The preview URL is shown both at the job output and in the merge request widget. You also get the link to the remote pipeline. @@ -537,8 +537,12 @@ To have the screenshot focuses few more steps are needed: - **wait for the content**: `expect(screenshot_area).to have_content 'Expiration interval'` - **set the crop area**: `set_crop_data(screenshot_area, 20)` -In particular `set_crop_data` accepts as arguments: a `DOM` element and a padding, the padding will be added around the element enlarging the screenshot area. +In particular, `set_crop_data` accepts as arguments: a `DOM` element and a +padding. The padding is added around the element, enlarging the screenshot area. #### Live example Please use `spec/docs_screenshots/container_registry_docs.rb` as a guide and as an example to create your own scripts. + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 41e38266a58..c437cfd17a3 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -10,7 +10,7 @@ description: 'Writing styles, markup, formatting, and other standards for GitLab This document defines the standards for GitLab's documentation content and files. -For broader information about the documentation, see the [Documentation guidelines](index.md). +For broader information about the documentation, see the [Documentation guidelines](../index.md). For guidelines specific to text in the GitLab interface, see the Pajamas [Content](https://design.gitlab.com/content/error-messages/) section. diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 26a1e9ec3aa..c2c704f75c2 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -122,10 +122,10 @@ This is also not just applied to models. Here's a list of other examples: To test an `EE` namespaced module that extends a CE class with EE features, create the spec file as you normally would in the `ee/spec` directory, including the second `ee/` subdirectory. -For example, an extension `ee/app/models/ee/user.rb` would have its tests in `ee/app/models/ee/user_spec.rb`. +For example, an extension `ee/app/models/ee/user.rb` would have its tests in `ee/spec/models/ee/user_spec.rb`. In the `RSpec.describe` call, use the CE class name where the EE module would be used. -For example, in `ee/app/models/ee/user_spec.rb`, the test would start with: +For example, in `ee/spec/models/ee/user_spec.rb`, the test would start with: ```ruby RSpec.describe User do diff --git a/doc/development/event_tracking/backend.md b/doc/development/event_tracking/backend.md index 79ea80a52ea..24e83ffc524 100644 --- a/doc/development/event_tracking/backend.md +++ b/doc/development/event_tracking/backend.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/index.md' --- This document was moved to [another location](../product_analytics/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/event_tracking/frontend.md b/doc/development/event_tracking/frontend.md index 79ea80a52ea..24e83ffc524 100644 --- a/doc/development/event_tracking/frontend.md +++ b/doc/development/event_tracking/frontend.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/index.md' --- This document was moved to [another location](../product_analytics/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/event_tracking/index.md b/doc/development/event_tracking/index.md index 79ea80a52ea..24e83ffc524 100644 --- a/doc/development/event_tracking/index.md +++ b/doc/development/event_tracking/index.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/index.md' --- This document was moved to [another location](../product_analytics/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md index 3b2f1d21463..feb76c3262f 100644 --- a/doc/development/experiment_guide/index.md +++ b/doc/development/experiment_guide/index.md @@ -6,17 +6,17 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Experiment Guide -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 can be conducted by any GitLab team, most often the teams from the [Growth Sub-department](https://about.gitlab.com/handbook/engineering/development/growth/). Experiments are not tied to releases because they primarily target GitLab.com. -Experiments 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. +Experiments are run as an A/B test and are behind a feature flag to turn the test on or off. Based on the data the experiment generates, the team decides if the experiment had a positive impact and should be made the new default or rolled back. ## Experiment tracking issue 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. -- It was not successful and all code related to the experiment will be removed. +- It was successful and the experiment becomes the new default. +- It was not successful and all code related to the experiment is removed. In either case, an outcome of the experiment should be posted to the issue with the reasoning for the decision. @@ -49,7 +49,6 @@ addressed. }, # Add your experiment here: signup_flow: { - environment: ::Gitlab.dev_env_or_com?, # Target environment, defaults to enabled for development and GitLab.com tracking_category: 'Growth::Activation::Experiment::SignUpFlow' # Used for providing the category when setting up tracking data } }.freeze @@ -80,7 +79,7 @@ addressed. end ``` - The above will check whether the experiment is enabled and push the result to the frontend. + The above checks whether the experiment is enabled and push the result to the frontend. You can check the state of the feature flag in JavaScript: diff --git a/doc/development/fe_guide/event_tracking.md b/doc/development/fe_guide/event_tracking.md index 79ea80a52ea..24e83ffc524 100644 --- a/doc/development/fe_guide/event_tracking.md +++ b/doc/development/fe_guide/event_tracking.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/index.md' --- This document was moved to [another location](../product_analytics/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index cae2435e4ff..762281756fe 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -833,7 +833,7 @@ If your application contains `@client` queries, most probably you will have an A ```javascript import createMockApollo from 'jest/helpers/mock_apollo_helper'; ... -fakeApollo = createMockApollo(requestHandlers, {}); +mockApollo = createMockApollo(requestHandlers, resolvers); ``` Sometimes we want to test a `result` hook of the local query. In order to have it triggered, we need to populate a cache with correct data to be fetched with this query: @@ -849,14 +849,14 @@ query fetchLocalUser { ```javascript import fetchLocalUserQuery from '~/design_management/graphql/queries/fetch_local_user.query.graphql'; -function createComponentWithApollo() { +function createMockApolloProvider() { const requestHandlers = [ [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], ]; - fakeApollo = createMockApollo(requestHandlers, {}); - fakeApollo.clients.defaultClient.cache.writeQuery({ + mockApollo = createMockApollo(requestHandlers, {}); + mockApollo.clients.defaultClient.cache.writeQuery({ query: fetchLocalUserQuery, data: { fetchLocalUser: { @@ -864,15 +864,107 @@ function createComponentWithApollo() { name: 'Test', }, }, - }) + }); - wrapper = shallowMount(Index, { + return mockApollo; +} + +function createComponent(options = {}) { + const { mockApollo } = options; + + return shallowMount(Index, { localVue, - apolloProvider: fakeApollo, + apolloProvider: mockApollo, }); } ``` +Sometimes it is necessary to control what the local resolver returns and inspect how it is called by the component. This can be done by mocking your local resolver: + +```javascript +import fetchLocalUserQuery from '~/design_management/graphql/queries/fetch_local_user.query.graphql'; + +function createMockApolloProvider(options = {}) { + const { fetchLocalUserSpy } = options; + + mockApollo = createMockApollo([], { + Query: { + fetchLocalUser: fetchLocalUserSpy, + }, + }); + + // Necessary for local resolvers to be activated + mockApollo.clients.defaultClient.cache.writeQuery({ + query: fetchLocalUserQuery, + data: {}, + }); + + return mockApollo; +} +``` + +In the test you can then control what the spy is supposed to do and inspect the component after the request have returned: + +```javascript +describe('My Index test with `createMockApollo`', () => { + let wrapper; + let fetchLocalUserSpy; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + fetchLocalUserSpy = null; + }); + + describe('when loading', () => { + beforeEach(() => { + const mockApollo = createMockApolloProvider(); + wrapper = createComponent({ mockApollo }); + }); + + it('displays the loader', () => { + // Assess that the loader is present + }); + }); + + describe('with data', () => { + beforeEach(async () => { + fetchLocalUserSpy = jest.fn().mockResolvedValue(localUserQueryResponse); + const mockApollo = createMockApolloProvider(fetchLocalUserSpy); + wrapper = createComponent({ mockApollo }); + await waitForPromises(); + }); + + it('should fetch data once', () => { + expect(fetchLocalUserSpy).toHaveBeenCalledTimes(1); + }); + + it('displays data', () => { + // Assess that data is present + }); + }); + + describe('with error', () => { + const error = 'Error!'; + + beforeEach(async () => { + fetchLocalUserSpy = jest.fn().mockRejectedValueOnce(error); + const mockApollo = createMockApolloProvider(fetchLocalUserSpy); + wrapper = createComponent({ mockApollo }); + await waitForPromises(); + }); + + it('should fetch data once', () => { + expect(fetchLocalUserSpy).toHaveBeenCalledTimes(1); + }); + + it('displays the error', () => { + // Assess that the error is displayed + }); + }); +}); +``` + ## Handling errors GitLab's GraphQL mutations currently have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index f3fa80325ef..73a5bea189d 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -3,3 +3,6 @@ redirect_to: 'style/javascript.md' --- This document was moved to [another location](style/javascript.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/style_guide_scss.md b/doc/development/fe_guide/style_guide_scss.md index 2b4e6427a18..eaa6d33fb43 100644 --- a/doc/development/fe_guide/style_guide_scss.md +++ b/doc/development/fe_guide/style_guide_scss.md @@ -3,3 +3,6 @@ redirect_to: 'style/scss.md' --- This document was moved to [another location](style/scss.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index b23e37d1eef..457d15235ae 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -3,3 +3,6 @@ redirect_to: '../testing_guide/frontend_testing.md' --- This document was moved to [another location](../testing_guide/frontend_testing.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index cff88388ba0..7456e8df8d9 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -3,3 +3,6 @@ redirect_to: 'feature_flags/index.md' --- This document was moved to [another location](feature_flags/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md index 2855662e1db..a84536eac61 100644 --- a/doc/development/feature_flags/development.md +++ b/doc/development/feature_flags/development.md @@ -23,6 +23,12 @@ All newly-introduced feature flags should be [disabled by default](process.md#fe NOTE: **Note:** This document is the subject of continued work as part of an epic to [improve internal usage of Feature Flags](https://gitlab.com/groups/gitlab-org/-/epics/3551). Raise any suggestions as new issues and attach them to the epic. +## Risk of a broken master (main) branch + +Feature flags **must** be used in the MR that introduces them. Not doing so causes a +[broken master](https://about.gitlab.com/handbook/engineering/workflow/#broken-master) scenario due +to the `rspec:feature-flags` job that only runs on the `master` branch. + ## Types of feature flags Choose a feature flag type that matches the expected usage. diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md index aa91e105513..69b6777d192 100644 --- a/doc/development/file_storage.md +++ b/doc/development/file_storage.md @@ -48,6 +48,7 @@ they are still not 100% standardized. You can see them below: | CI Artifacts (CE) | yes | `shared/artifacts/:disk_hash[0..1]/:disk_hash[2..3]/:disk_hash/:year_:month_:date/:job_id/:job_artifact_id` (`:disk_hash` is SHA256 digest of `project_id`) | `JobArtifactUploader` | Ci::JobArtifact | | LFS Objects (CE) | yes | `shared/lfs-objects/:hex/:hex/:object_hash` | `LfsObjectUploader` | LfsObject | | External merge request diffs | yes | `shared/external-diffs/merge_request_diffs/mr-:parent_id/diff-:id` | `ExternalDiffUploader` | MergeRequestDiff | +| Issuable metric images | yes | `uploads/-/system/issuable_metric_image/file/:id/:filename` | `IssuableMetricImageUploader` | IssuableMetricImage | CI Artifacts and LFS Objects behave differently in CE and EE. In CE they inherit the `GitlabUploader` while in EE they inherit the `ObjectStorage` and store files in and S3 API compatible object store. diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 8bb5cf7af62..040004a6917 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -3,3 +3,6 @@ redirect_to: 'fe_guide/index.md' --- This document was moved to [another location](fe_guide/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/geo/framework.md b/doc/development/geo/framework.md index e440e324c4a..2e3f61278e0 100644 --- a/doc/development/geo/framework.md +++ b/doc/development/geo/framework.md @@ -393,6 +393,8 @@ can track verification state. def change change_table(:widgets) do |t| + t.integer :verification_state, default: 0, limit: 2, null: false + t.column :verification_started_at, :datetime_with_timezone t.integer :verification_retry_count, limit: 2 t.column :verification_retry_at, :datetime_with_timezone t.column :verified_at, :datetime_with_timezone @@ -431,13 +433,12 @@ can track verification state. end ``` -1. Add a partial index on `verification_failure` and `verification_checksum` to ensure - re-verification can be performed efficiently: +1. Add an index on `verification_state` to ensure verification can be performed efficiently: ```ruby # frozen_string_literal: true - class AddVerificationFailureIndexToWidgets < ActiveRecord::Migration[6.0] + class AddVerificationStateIndexToWidgets < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers DOWNTIME = false @@ -445,22 +446,28 @@ can track verification state. disable_ddl_transaction! def up - add_concurrent_index :widgets, :verification_failure, where: "(verification_failure IS NOT NULL)", name: "widgets_verification_failure_partial" - add_concurrent_index :widgets, :verification_checksum, where: "(verification_checksum IS NOT NULL)", name: "widgets_verification_checksum_partial" + add_concurrent_index :widgets, :verification_state, name: "index_widgets_on_verification_state" end def down - remove_concurrent_index :widgets, :verification_failure - remove_concurrent_index :widgets, :verification_checksum + remove_concurrent_index :widgets, :verification_state end end ``` +1. Add the `Gitlab::Geo::VerificationState` concern to the `widget` model if it is not already included in `Gitlab::Geo::ReplicableModel`: + + ```ruby + class Widget < ApplicationRecord + ... + include ::Gitlab::Geo::VerificationState + ... + end + ``` + ##### Option 2: Create a separate `widget_states` table with verification state fields -1. Create a `widget_states` table and add a partial index on `verification_failure` and - `verification_checksum` to ensure re-verification can be performed efficiently. Order - the columns according to [the guidelines](../ordering_table_columns.md): +1. Create a `widget_states` table and add an index on `verification_state` to ensure verification can be performed efficiently. Order the columns according to [the guidelines](../ordering_table_columns.md): ```ruby # frozen_string_literal: true @@ -477,14 +484,15 @@ can track verification state. with_lock_retries do create_table :widget_states, id: false do |t| t.references :widget, primary_key: true, null: false, foreign_key: { on_delete: :cascade } + t.integer :verification_state, default: 0, limit: 2, null: false + t.column :verification_started_at, :datetime_with_timezone t.datetime_with_timezone :verification_retry_at t.datetime_with_timezone :verified_at t.integer :verification_retry_count, limit: 2 t.binary :verification_checksum, using: 'verification_checksum::bytea' t.text :verification_failure - t.index :verification_failure, where: "(verification_failure IS NOT NULL)", name: "widgets_verification_failure_partial" - t.index :verification_checksum, where: "(verification_checksum IS NOT NULL)", name: "widgets_verification_checksum_partial" + t.index :verification_state, name: "index_widget_states_on_verification_state" end end end @@ -498,6 +506,20 @@ can track verification state. end ``` +1. Add the following lines to the `widget_state.rb` model: + + ```ruby + class WidgetState < ApplicationRecord + ... + self.primary_key = :widget_id + + include ::Gitlab::Geo::VerificationState + + belongs_to :widget, inverse_of: :widget_state + ... + end + ``` + 1. Add the following lines to the `widget` model: ```ruby @@ -547,14 +569,16 @@ Metrics are gathered by `Geo::MetricsUpdateWorker`, persisted in 1. Add the following to `spec/factories/widgets.rb`: ```ruby - trait(:checksummed) do + trait(:verification_succeeded) do with_file verification_checksum { 'abc' } + verification_state { Widget.verification_state_value(:verification_succeeded) } end - trait(:checksum_failure) do + trait(:verification_failed) do with_file verification_failure { 'Could not calculate the checksum' } + verification_state { Widget.verification_state_value(:verification_failed) } end ``` diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index 8b4e5090abb..9b2081b2821 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -84,7 +84,7 @@ If your test-suite is failing with Gitaly issues, as a first step, try running: rm -rf tmp/tests/gitaly ``` -During RSpec tests, the Gitaly instance will write logs to `gitlab/log/gitaly-test.log`. +During RSpec tests, the Gitaly instance writes logs to `gitlab/log/gitaly-test.log`. ## Legacy Rugged code @@ -124,23 +124,23 @@ 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 Rugged unless explicitly discussed with the [Gitaly -Team](https://gitlab.com/groups/gl-gitaly/group_members). This code will +Team](https://gitlab.com/groups/gl-gitaly/group_members). This code does NOT work on GitLab.com or other GitLab instances that do not use NFS. ## `TooManyInvocationsError` errors During development and testing, you may experience `Gitlab::GitalyClient::TooManyInvocationsError` failures. -The `GitalyClient` will attempt to block against potential n+1 issues by raising this error +The `GitalyClient` attempts to block against potential n+1 issues by raising this error when Gitaly is called more than 30 times in a single Rails request or Sidekiq execution. -As a temporary measure, export `GITALY_DISABLE_REQUEST_LIMITS=1` to suppress the error. This will disable the n+1 detection +As a temporary measure, export `GITALY_DISABLE_REQUEST_LIMITS=1` to suppress the error. This disables the n+1 detection in your development environment. Please raise an issue in the GitLab CE or EE repositories to report the issue. Include the labels ~Gitaly ~performance ~"technical debt". Please ensure that the issue contains the full stack trace and error message of the `TooManyInvocationsError`. Also include any known failing tests if possible. -Isolate the source of the n+1 problem. This will normally be a loop that results in Gitaly being called for each +Isolate the source of the n+1 problem. This is normally a loop that results in Gitaly being called for each element in an array. If you are unable to isolate the problem, please contact a member of the [Gitaly Team](https://gitlab.com/groups/gl-gitaly/group_members) for assistance. @@ -154,7 +154,7 @@ Gitlab::GitalyClient.allow_n_plus_1_calls do end ``` -Once the code is wrapped in this block, this code-path will be excluded from n+1 detection. +Once the code is wrapped in this block, this code path is excluded from n+1 detection. ## Request counts @@ -184,12 +184,12 @@ branches and SHA to use a custom commit in <https://gitlab.com/gitlab-org/gitaly NOTE: **Note:** With the introduction of auto-deploy for Gitaly, the format of `GITALY_SERVER_VERSION` was aligned with Omnibus syntax. -It no longer supports `=revision`, it will evaluate the file content as a Git -reference (branch or SHA), only if it matches a semver it will prepend a `v`. +It no longer supports `=revision`, it evaluates the file content as a Git +reference (branch or SHA). Only if it matches a semver does it prepend a `v`. If you want to run tests locally against a modified version of Gitaly you can replace `tmp/tests/gitaly` with a symlink. This is much faster -because if will avoid a Gitaly re-install each time you run `rspec`. +because it avoids a Gitaly re-install each time you run `rspec`. ```shell rm -rf tmp/tests/gitaly @@ -197,12 +197,12 @@ ln -s /path/to/gitaly tmp/tests/gitaly ``` Make sure you run `make` in your local Gitaly directory before running -tests. Otherwise, Gitaly will fail to boot. +tests. Otherwise, Gitaly fails to boot. If you make changes to your local Gitaly in between test runs you need to manually run `make` again. -Note that CI tests will not use your locally modified version of +Note that CI tests do not use your locally modified version of Gitaly. To use a custom Gitaly version in CI you need to update GITALY_SERVER_VERSION as described at the beginning of this paragraph. diff --git a/doc/development/go_guide/dependencies.md b/doc/development/go_guide/dependencies.md index 461ee394533..522f9d80cfc 100644 --- a/doc/development/go_guide/dependencies.md +++ b/doc/development/go_guide/dependencies.md @@ -89,28 +89,28 @@ Go 1.12 introduced checksum databases and module proxies. ### Checksums -In addition to `go.mod`, a module will have a `go.sum` file. This file records a +In addition to `go.mod`, a module has a `go.sum` file. This file records a SHA-256 checksum of the code and the `go.mod` file of every version of every dependency that is referenced by the module or one of the module's dependencies. Go continually updates `go.sum` as new dependencies are referenced. When Go fetches the dependencies of a module, if those dependencies already have -an entry in `go.sum`, Go will verify the checksum of these dependencies. If the -checksum does not match what is in `go.sum`, the build will fail. This ensures +an entry in `go.sum`, Go verifies the checksum of these dependencies. If the +checksum does not match what is in `go.sum`, the build fails. This ensures that a given version of a module cannot be changed by its developers or by a malicious party without causing build failures. Go 1.12+ can be configured to use a checksum database. If configured to do so, when Go fetches a dependency and there is no corresponding entry in `go.sum`, Go -will query the configured checksum database(s) for the checksum of the +queries the configured checksum database(s) for the checksum of the dependency instead of calculating it from the downloaded dependency. If the -dependency cannot be found in the checksum database, the build will fail. If the +dependency cannot be found in the checksum database, the build fails. If the downloaded dependency's checksum does not match the result from the checksum -database, the build will fail. The following environment variables control this: +database, the build fails. The following environment variables control this: - `GOSUMDB` identifies the name, and optionally the public key and server URL, of the checksum database to query. - - A value of `off` will entirely disable checksum database queries. + - A value of `off` entirely disables checksum database queries. - Go 1.13+ uses `sum.golang.org` if `GOSUMDB` is not defined. - `GONOSUMDB` is a comma-separated list of module suffixes that checksum database queries should be disabled for. Wildcards are supported. @@ -125,8 +125,8 @@ attempts to fetch the dependency from the configured proxies, in order. The following environment variables control this: - `GOPROXY` is a comma-separated list of module proxies to query. - - A value of `direct` will entirely disable module proxy queries. - - If the last entry in the list is `direct`, Go will fall back to the process + - A value of `direct` entirely disables module proxy queries. + - If the last entry in the list is `direct`, Go falls back to the process described [above](#fetching-packages) if none of the proxies can provide the dependency. - Go 1.13+ uses `proxy.golang.org,direct` if `GOPROXY` is not defined. @@ -159,7 +159,7 @@ From Go 1.12 onward, the process for fetching a module or package is as follows: The downloaded source must contain a `go.mod` file. The `go.mod` file must contain a `module` directive that specifies the name of the module. If the module name as specified by `go.mod` does not match the name that was used to -fetch the module, the module will fail to compile. +fetch the module, the module fails to compile. If the module is being fetched directly and no version was specified, or if the module is being added as a dependency and no version was specified, Go uses the @@ -172,9 +172,9 @@ latest that is also a valid semantic version. In versions prior to Go 1.13, support for authenticating requests made by Go was somewhat inconsistent. Go 1.13 improved support for `.netrc` authentication. If -a request is made over HTTPS and a matching `.netrc` entry can be found, Go will -add HTTP Basic authentication credentials to the request. Go will not -authenticate requests made over HTTP. Go will reject HTTP-only entries in +a request is made over HTTPS and a matching `.netrc` entry can be found, Go +adds HTTP Basic authentication credentials to the request. Go does not +authenticate requests made over HTTP. Go rejects HTTP-only entries in `GOPROXY` that have embedded credentials. In a future version, Go may add support for arbitrary authentication headers. diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index 4077cf2a2c2..7aace206aee 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -20,7 +20,7 @@ the two is best for the job. This page aims to define and organize our Go guidelines, based on our various experiences. Several projects were started with different standards and they -can still have specifics. They will be described in their respective +can still have specifics. They are described in their respective `README.md` or `PROCESS.md` files. ## Dependency Management @@ -89,7 +89,7 @@ projects: ## Code style and format -- Avoid global variables, even in packages. By doing so you will introduce side +- Avoid global variables, even in packages. By doing so you introduce side effects if the package is included multiple times. - Use `goimports` before committing. [goimports](https://godoc.org/golang.org/x/tools/cmd/goimports) @@ -97,7 +97,7 @@ projects: [Gofmt](https://golang.org/cmd/gofmt/), in addition to formatting import lines, adding missing ones and removing unreferenced ones. - Most editors/IDEs will allow you to run commands before/after saving a file, you can set it + Most editors/IDEs allow you to run commands before/after saving a file, you can set it up to run `goimports` so that it's applied to every file when saving. - Place private methods below the first caller method in the source file. @@ -128,7 +128,7 @@ configuration of `golangci-lint`. All options for `golangci-lint` are listed in this [example](https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml). Once [recursive includes](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/56836) -become available, you will be able to share job templates like this +become available, you can share job templates like this [analyzer](https://gitlab.com/gitlab-org/security-products/ci-templates/raw/master/includes-dev/analyzer.yml). ## Dependencies @@ -150,7 +150,7 @@ define and lock dependencies for reproducible builds. It should be used whenever possible. When Go Modules are in use, there should not be a `vendor/` directory. Instead, -Go will automatically download dependencies when they are needed to build the +Go automatically downloads dependencies when they are needed to build the project. This is in line with how dependencies are handled with Bundler in Ruby projects, and makes merge requests easier to review. @@ -177,7 +177,7 @@ databases. In the rare event of managing a hosted database, it's necessary to use a migration system like ActiveRecord is providing. A simple library like [Journey](https://github.com/db-journey/journey), designed to be used in -`postgres` containers, can be deployed as long-running pods. New versions will +`postgres` containers, can be deployed as long-running pods. New versions deploy a new pod, migrating the data automatically. ## Testing @@ -255,7 +255,7 @@ to make the test output easily readable. to use for naming subtests. In the Go standard library, this is commonly the `name string` field. - Use `want`/`expect`/`actual` when you are specifying something in the - test case that will be used for assertion. + test case that is used for assertion. #### Variable names @@ -414,7 +414,7 @@ builds](https://docs.docker.com/develop/develop-images/multistage-build/): Generated Docker images should have the program at their `Entrypoint` to create portable commands. That way, anyone can run the image, and without parameters -it will display its help message (if `cli` has been used). +it displays its help message (if `cli` has been used). ## Distributing Go binaries @@ -476,7 +476,7 @@ 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. +We do not consider the active milestone, `12.10`, because a backport for `12.7` is required in case of a critical security release. 1. If both [Omnibus and CNG](#updating-go-version) were using Go `1.12` in GitLab `12.7` and later, 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. @@ -492,11 +492,11 @@ Use `goimports -local gitlab.com/gitlab-org` before committing. is a tool that automatically formats Go source code using [Gofmt](https://golang.org/cmd/gofmt/), in addition to formatting import lines, adding missing ones and removing unreferenced ones. -By using the `-local gitlab.com/gitlab-org` option, `goimports` will group locally referenced +By using the `-local gitlab.com/gitlab-org` option, `goimports` groups locally referenced packages separately from external ones. See [the imports section](https://github.com/golang/go/wiki/CodeReviewComments#imports) of the Code Review Comments page on the Go wiki for more details. -Most editors/IDEs will allow you to run commands before/after saving a file, you can set it +Most editors/IDEs allow you to run commands before/after saving a file, you can set it up to run `goimports -local gitlab.com/gitlab-org` so that it's applied to every file when saving. --- diff --git a/doc/development/i18n_guide.md b/doc/development/i18n_guide.md index e588b47e203..ddc91f9308e 100644 --- a/doc/development/i18n_guide.md +++ b/doc/development/i18n_guide.md @@ -3,3 +3,6 @@ redirect_to: 'i18n/index.md' --- This document was moved to [another location](i18n/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md index bdbcd52eb61..07e39c66a6c 100644 --- a/doc/development/instrumentation.md +++ b/doc/development/instrumentation.md @@ -19,13 +19,14 @@ Instrumenting methods is done by using the `Gitlab::Metrics::Instrumentation` module. This module offers a few different methods that can be used to instrument code: -- `instrument_method`: instruments a single class method. -- `instrument_instance_method`: instruments a single instance method. -- `instrument_class_hierarchy`: given a Class this method will recursively - instrument all sub-classes (both class and instance methods). -- `instrument_methods`: instruments all public and private class methods of a Module. -- `instrument_instance_methods`: instruments all public and private instance methods of a +- `instrument_method`: Instruments a single class method. +- `instrument_instance_method`: Instruments a single instance method. +- `instrument_class_hierarchy`: Given a Class, this method recursively + instruments all sub-classes (both class and instance methods). +- `instrument_methods`: Instruments all public and private class methods of a Module. +- `instrument_instance_methods`: Instruments all public and private instance + methods of a Module. To remove the need for typing the full `Gitlab::Metrics::Instrumentation` namespace you can use the `configure` class method. This method simply yields @@ -91,7 +92,7 @@ Ruby code. In case of the above snippet you'd run the following: - `$ Banzai::Renderer.render` -This will print out something along the lines of: +This prints a result similar to: ```plaintext From: /path/to/your/gitlab/lib/gitlab/metrics/instrumentation.rb @ line 148: @@ -131,7 +132,7 @@ Three values are measured for a block: Both the real and CPU timings are measured in milliseconds. -Multiple calls to the same block will result in the final values being the sum +Multiple calls to the same block results in the final values being the sum of all individual values. Take this code for example: ```ruby @@ -142,7 +143,7 @@ of all individual values. Take this code for example: end ``` -Here the final value of `sleep_real_time` will be `3`, _not_ `1`. +Here, the final value of `sleep_real_time` is `3`, and not `1`. ## Tracking Custom Events diff --git a/doc/development/integrations/secure_partner_integration.md b/doc/development/integrations/secure_partner_integration.md index 52d10f0bd3c..0d57164ef29 100644 --- a/doc/development/integrations/secure_partner_integration.md +++ b/doc/development/integrations/secure_partner_integration.md @@ -7,7 +7,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Secure Partner Integration - Onboarding Process If you want to integrate your product with the [Secure Stage](https://about.gitlab.com/direction/secure/), -this page will help you understand the developer workflow GitLab intends for +this page describes the developer workflow GitLab intends for our users to follow with regards to security results. These should be used as guidelines so you can build an integration that fits with the workflow GitLab users are already familiar with. @@ -29,7 +29,7 @@ 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 -function. Understanding how users use GitLab today will help you choose the +function. Understanding how users use GitLab today helps you choose the best place to integrate your own product and its results into GitLab. - Developers want to write code without using a new tool to consume results @@ -101,7 +101,7 @@ and complete an integration with the Secure stage. - Users can interact with the findings from your artifact within their workflow. They can dismiss the findings or accept them and create a backlog issue. - To automatically create issues without user interaction, use the [issue API](../../api/issues.md). This will be replaced by [Standalone Vulnerabilities](https://gitlab.com/groups/gitlab-org/-/epics/634) in the future. 1. Optional: Provide auto-remediation steps: - - If you specified `remediations` in your artifact, it is proposed through our [auto-remediation](../../user/application_security/index.md#automatic-remediation-for-vulnerabilities) + - If you specified `remediations` in your artifact, it is proposed through our [automatic remediation](../../user/application_security/index.md#automatic-remediation-for-vulnerabilities) interface. 1. Demo the integration to GitLab: - After you have tested and are ready to demo your integration please @@ -112,7 +112,7 @@ and complete an integration with the Secure stage. to support your go-to-market as appropriate. - Examples of supported marketing could include being listed on our [Security Partner page](https://about.gitlab.com/partners/#security), doing an [Unfiltered blog post](https://about.gitlab.com/handbook/marketing/blog/unfiltered/), - doing a co-branded webinar, or producing a co-branded whitepaper. + doing a co-branded webinar, or producing a co-branded white paper. We have a [video playlist](https://www.youtube.com/playlist?list=PL05JrBw4t0KpMqYxJiOLz-uBIr5w-yP4A) that may be helpful as part of this process. This covers various topics related to integrating your diff --git a/doc/development/licensed_feature_availability.md b/doc/development/licensed_feature_availability.md index f83a57fe1ca..2bdb91daa40 100644 --- a/doc/development/licensed_feature_availability.md +++ b/doc/development/licensed_feature_availability.md @@ -30,7 +30,7 @@ project.feature_available?(:feature_symbol) However, for features such as [Geo](../administration/geo/index.md) and [Load balancing](../administration/database_load_balancing.md), which cannot be restricted -to only a subset of projects or namespaces, the check will be made directly in +to only a subset of projects or namespaces, the check is made directly in the instance license. 1. Add the feature symbol on `EES_FEATURES`, `EEP_FEATURES` or `EEU_FEATURES` constants in diff --git a/doc/development/logging.md b/doc/development/logging.md index 14812978f2d..f81dd3e6438 100644 --- a/doc/development/logging.md +++ b/doc/development/logging.md @@ -35,14 +35,14 @@ Completed 200 OK in 166ms (Views: 117.4ms | ActiveRecord: 27.2ms) These logs suffer from a number of problems: -1. They often lack timestamps or other contextual information (e.g. project ID, user) +1. They often lack timestamps or other contextual information (for example, project ID or user) 1. They may span multiple lines, which make them hard to find via Elasticsearch. 1. They lack a common structure, which make them hard to parse by log forwarders, such as Logstash or Fluentd. This also makes them hard to search. -Note that currently on GitLab.com, any messages in `production.log` will -NOT get indexed by Elasticsearch due to the sheer volume and noise. They +Note that currently on GitLab.com, any messages in `production.log` aren't +indexed by Elasticsearch due to the sheer volume and noise. They do end up in Google Stackdriver, but it is still harder to search for logs there. See the [GitLab.com logging documentation](https://gitlab.com/gitlab-com/runbooks/blob/master/logging/doc/README.md) @@ -73,7 +73,7 @@ importer progresses. Here's what to do: make it easy for people to search pertinent logs in one place. For example, `geo.log` contains all logs pertaining to GitLab Geo. To create a new file: - 1. Choose a filename (e.g. `importer_json.log`). + 1. Choose a filename (for example, `importer_json.log`). 1. Create a new subclass of `Gitlab::JsonLogger`: ```ruby @@ -99,7 +99,7 @@ importer progresses. Here's what to do: ``` Note that it's useful to memoize this because creating a new logger - each time you log will open a file, adding unnecessary overhead. + each time you log opens a file, adding unnecessary overhead. 1. Now insert log messages into your code. When adding logs, make sure to include all the context as key-value pairs: @@ -129,16 +129,16 @@ an Elasticsearch-specific way, the concepts should translate to many systems you might use to index structured logs. GitLab.com uses Elasticsearch to index log data. -Unless a field type is explicitly mapped, Elasticsearch will infer the type from +Unless a field type is explicitly mapped, Elasticsearch infers the type from the first instance of that field value it sees. Subsequent instances of that -field value with different types will either fail to be indexed, or in some -cases (scalar/object conflict), the whole log line will be dropped. +field value with different types either fail to be indexed, or in some +cases (scalar/object conflict), the whole log line is dropped. GitLab.com's logging Elasticsearch sets [`ignore_malformed`](https://www.elastic.co/guide/en/elasticsearch/reference/current/ignore-malformed.html), which allows documents to be indexed even when there are simpler sorts of mapping conflict (for example, number / string), although indexing on the affected fields -will break. +breaks. Examples: @@ -177,17 +177,24 @@ challenged to choose between seconds, milliseconds or any other unit, lean towar (with microseconds precision, i.e. `Gitlab::InstrumentationHelper::DURATION_PRECISION`). In order to make it easier to track timings in the logs, make sure the log key has `_s` as -suffix and `duration` within its name (e.g., `view_duration_s`). +suffix and `duration` within its name (for example, `view_duration_s`). ## Multi-destination Logging -GitLab is transitioning from unstructured/plaintext logs to structured/JSON logs. During this transition period some logs will be recorded in multiple formats through multi-destination logging. +GitLab is transitioning from unstructured/plaintext logs to structured/JSON logs. During this transition period some logs are recorded in multiple formats through multi-destination logging. ### How to use multi-destination logging -Create a new logger class, inheriting from `MultiDestinationLogger` and add an array of loggers to a `LOGGERS` constant. The loggers should be classes that descend from `Gitlab::Logger`. e.g. the user defined loggers in the following examples, could be inheriting from `Gitlab::Logger` and `Gitlab::JsonLogger`, respectively. +Create a new logger class, inheriting from `MultiDestinationLogger` and add an +array of loggers to a `LOGGERS` constant. The loggers should be classes that +descend from `Gitlab::Logger`. For example, the user-defined loggers in the +following examples could be inheriting from `Gitlab::Logger` and +`Gitlab::JsonLogger`, respectively. -You must specify one of the loggers as the `primary_logger`. The `primary_logger` will be used when information about this multi-destination logger is displayed in the app, e.g. using the `Gitlab::Logger.read_latest` method. +You must specify one of the loggers as the `primary_logger`. The +`primary_logger` is used when information about this multi-destination logger is +displayed in the application (for example, using the `Gitlab::Logger.read_latest` +method). The following example sets one of the defined `LOGGERS` as a `primary_logger`. @@ -207,19 +214,19 @@ module Gitlab end ``` -You can now call the usual logging methods on this multi-logger, e.g. +You can now call the usual logging methods on this multi-logger. For example: ```ruby FancyMultiLogger.info(message: "Information") ``` -This message will be logged by each logger registered in `FancyMultiLogger.loggers`. +This message is logged by each logger registered in `FancyMultiLogger.loggers`. ### Passing a string or hash for logging When passing a string or hash to a `MultiDestinationLogger`, the log lines could be formatted differently, depending on the kinds of `LOGGERS` set. -e.g. let's partially define the loggers from the previous example: +For example, let's partially define the loggers from the previous example: ```ruby module Gitlab @@ -304,7 +311,7 @@ It should be noted that manual logging of exceptions is not allowed, as: 1. Very often manually logged exception needs to be tracked to Sentry as well, 1. Manually logged exceptions does not use `correlation_id`, which makes hard to pin them to request, user and context in which this exception was raised, -1. It is very likely that manually logged exceptions will end-up across +1. Manually logged exceptions often end up across multiple files, which increases burden scraping all logging files. To avoid duplicating and having consistent behavior the `Gitlab::ErrorTracking` @@ -356,7 +363,7 @@ end ## Additional steps with new log files -1. Consider log retention settings. By default, Omnibus will rotate any +1. Consider log retention settings. By default, Omnibus rotates any logs in `/var/log/gitlab/gitlab-rails/*.log` every hour and [keep at most 30 compressed files](https://docs.gitlab.com/omnibus/settings/logs.html#logrotate). On GitLab.com, that setting is only 6 compressed files. These settings should suffice diff --git a/doc/development/merge_request_performance_guidelines.md b/doc/development/merge_request_performance_guidelines.md index a5d9a653472..b4a9cb2fd13 100644 --- a/doc/development/merge_request_performance_guidelines.md +++ b/doc/development/merge_request_performance_guidelines.md @@ -166,7 +166,7 @@ Rails provides an [SQL Query Cache](cached_queries.md#cached-queries-guidelines) used to cache the results of database queries for the duration of the request. See [why cached queries are considered bad](cached_queries.md#why-cached-queries-are-considered-bad) and -[how to detect them](cached_queries.md#how-to-detect). +[how to detect them](cached_queries.md#how-to-detect-cached-queries). The code introduced by a merge request, should not execute multiple duplicated cached queries. diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 84679a78545..78acd729b9b 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -153,8 +153,9 @@ and therefore it does not have any records yet. When using a single-transaction migration, a transaction will hold on a database connection for the duration of the migration, so you must make sure the actions in the migration -do not take too much time: In general, queries executed in a migration need to fit comfortably -within `15s` on GitLab.com. +do not take too much time: GitLab.com’s production database has a `15s` timeout, so +in general, the cumulative execution time in a migration should aim to fit comfortably +in that limit. Singular query timings should fit within the [standard limit](query_performance.md#timing-guidelines-for-queries) In case you need to insert, update, or delete a significant amount of data, you: diff --git a/doc/development/new_fe_guide/development/performance.md b/doc/development/new_fe_guide/development/performance.md index 44f5bccde95..7a5da941a68 100644 --- a/doc/development/new_fe_guide/development/performance.md +++ b/doc/development/new_fe_guide/development/performance.md @@ -11,7 +11,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w We have a performance dashboard available in one of our [Grafana instances](https://dashboards.gitlab.net/d/1EBTz3Dmz/sitespeed-page-summary?orgId=1). This dashboard automatically aggregates metric data from [sitespeed.io](https://www.sitespeed.io/) every 6 hours. These changes are displayed after a set number of pages are aggregated. These pages can be found inside a text file in the [`gitlab-build-images` repository](https://gitlab.com/gitlab-org/gitlab-build-images) called [`gitlab.txt`](https://gitlab.com/gitlab-org/gitlab-build-images/blob/master/scripts/gitlab.txt) -Any frontend engineer can contribute to this dashboard. They can contribute by adding or removing URLs of pages from this text file. Please have a [frontend monitoring expert](https://about.gitlab.com/company/team/) review your changes before assigning to a maintainer of the `gitlab-build-images` project. The changes will go live on the next scheduled run after the changes are merged into `master`. +Any frontend engineer can contribute to this dashboard. They can contribute by adding or removing URLs of pages from this text file. Please have a [frontend monitoring expert](https://about.gitlab.com/company/team/) review your changes before assigning to a maintainer of the `gitlab-build-images` project. The changes are pushed live on the next scheduled run after the changes are merged into `master`. There are 3 recommended high impact metrics to review on each page: diff --git a/doc/development/new_fe_guide/development/testing.md b/doc/development/new_fe_guide/development/testing.md index b990425ca3c..034324989ef 100644 --- a/doc/development/new_fe_guide/development/testing.md +++ b/doc/development/new_fe_guide/development/testing.md @@ -3,3 +3,6 @@ redirect_to: '../../testing_guide/frontend_testing.md' --- This document was moved to [another location](../../testing_guide/frontend_testing.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/new_fe_guide/style/html.md b/doc/development/new_fe_guide/style/html.md index 0b4fce13d90..afbfbdcf834 100644 --- a/doc/development/new_fe_guide/style/html.md +++ b/doc/development/new_fe_guide/style/html.md @@ -3,3 +3,6 @@ redirect_to: '../../fe_guide/style/html.md' --- This document was moved to [another location](../../fe_guide/style/html.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/new_fe_guide/style/index.md b/doc/development/new_fe_guide/style/index.md index 284862a2be9..77700441aa3 100644 --- a/doc/development/new_fe_guide/style/index.md +++ b/doc/development/new_fe_guide/style/index.md @@ -3,3 +3,6 @@ redirect_to: '../../fe_guide/style/index.md' --- This document was moved to [another location](../../fe_guide/style/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/new_fe_guide/style/javascript.md b/doc/development/new_fe_guide/style/javascript.md index 003880c2592..17722d2c41b 100644 --- a/doc/development/new_fe_guide/style/javascript.md +++ b/doc/development/new_fe_guide/style/javascript.md @@ -3,3 +3,6 @@ redirect_to: '../../fe_guide/style/javascript.md' --- This document was moved to [another location](../../fe_guide/style/javascript.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/new_fe_guide/style/prettier.md b/doc/development/new_fe_guide/style/prettier.md index 9a95aa96dff..6c0f3b77505 100644 --- a/doc/development/new_fe_guide/style/prettier.md +++ b/doc/development/new_fe_guide/style/prettier.md @@ -3,3 +3,6 @@ redirect_to: '../../fe_guide/tooling.md#formatting-with-prettier' --- This document was moved to [another location](../../fe_guide/tooling.md#formatting-with-prettier). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/packages.md b/doc/development/packages.md index de6cac2ce73..30582a0b9f5 100644 --- a/doc/development/packages.md +++ b/doc/development/packages.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Packages -This document will guide you through adding another [package management system](../administration/packages/index.md) support to GitLab. +This document guides you through adding another [package management system](../administration/packages/index.md) support to GitLab. See already supported package types in [Packages documentation](../administration/packages/index.md) @@ -56,12 +56,12 @@ Group-level and instance-level endpoints are good to have but are optional. Packages are scoped within various levels of access, which is generally configured by setting your remote. A remote endpoint may be set at the project level, meaning when installing packages, only packages belonging to that -project will be visible. Alternatively, a group-level endpoint may be used to allow visibility to all packages +project are visible. Alternatively, a group-level endpoint may be used to allow visibility to all packages within a given group. Lastly, an instance-level endpoint can be used to allow visibility to all packages within an entire GitLab instance. -Using group and project level endpoints will allow for more flexibility in package naming, however, more remotes -will have to be managed. Using instance level endpoints requires [stricter naming conventions](#naming-conventions). +Using group and project level endpoints allows for more flexibility in package naming, however, more remotes +have to be managed. Using instance level endpoints requires [stricter naming conventions](#naming-conventions). The current state of existing package registries availability is: @@ -86,12 +86,12 @@ Composer package naming scope is Instance Level. ### Naming conventions -To avoid name conflict for instance-level endpoints you will need to define a package naming convention +To avoid name conflict for instance-level endpoints you must define a package naming convention that gives a way to identify the project that the package belongs to. This generally involves using the project ID or full project path in the package name. See [Conan's naming convention](../user/packages/conan_repository/index.md#package-recipe-naming-convention-for-instance-remotes) as an example. -For group and project-level endpoints, naming can be less constrained and it will be up to the group and project +For group and project-level endpoints, naming can be less constrained and it is up to the group and project members to be certain that there is no conflict between two package names. However, the system should prevent a user from reusing an existing name within a given scope. @@ -121,7 +121,7 @@ The way new package systems are integrated in GitLab is using an [MVC](https://a - Pulling a package - Required actions -Required actions are all the additional requests that GitLab will need to handle so the corresponding package manager CLI can work properly. It could be a search feature or an endpoint providing meta information about a package. For example: +Required actions are all the additional requests that GitLab needs to handle so the corresponding package manager CLI can work properly. It could be a search feature or an endpoint providing meta information about a package. For example: - For NuGet, the search request was implemented during the first MVC iteration, to support Visual Studio. - For NPM, there is a metadata endpoint used by `npm` to get the tarball URL. @@ -146,22 +146,22 @@ process. During this phase, the idea is to collect as much information as possible about the API used by the package system. Here some aspects that can be useful to include: - **Authentication**: What authentication mechanisms are available (OAuth, Basic - Authorization, other). Keep in mind that GitLab users will often want to use their + Authorization, other). Keep in mind that GitLab users often want to use their [Personal Access Tokens](../user/profile/personal_access_tokens.md). Although not needed for the MVC first iteration, the [CI job tokens](../user/project/new_ci_build_permissions_model.md#job-token) have to be supported at some point in the future. - **Requests**: Which requests are needed to have a working MVC. Ideally, produce a list of all the requests needed for the MVC (including required actions). Further investigation could provide an example for each request with the request and the response bodies. -- **Upload**: Carefully analyze how the upload process works. This will probably be the most +- **Upload**: Carefully analyze how the upload process works. This is likely the most complex request to implement. A detailed analysis is desired here as uploads can be encoded in different ways (body or multipart) and can even be in a totally different format (for example, a JSON structure where the package file is a Base64 value of a particular field). These different encodings lead to slightly different implementations on GitLab and GitLab Workhorse. For more detailed information, review [file uploads](#file-uploads). -- **Endpoints**: Suggest a list of endpoint URLs that will be implemented in GitLab. +- **Endpoints**: Suggest a list of endpoint URLs to implement in GitLab. - **Split work**: Suggest a list of changes to do to incrementally build the MVC. - This will give a good idea of how much work there is to be done. Here is an example + This gives a good idea of how much work there is to be done. Here is an example list that would need to be adapted on a case by case basis: 1. Empty file structure (API file, base service for this package) 1. Authentication system for "logging in" to the package manager @@ -177,7 +177,7 @@ In particular, the upload request can have some [requirements in the GitLab Work ### Implementation -The implementation of the different Merge Requests will vary between different package system integrations. Contributors should take into account some important aspects of the implementation phase. +The implementation of the different Merge Requests varies between different package system integrations. Contributors should take into account some important aspects of the implementation phase. #### Authentication @@ -217,23 +217,23 @@ If there is package specific behavior for a given package manager, add those met delegate from the package model. Note that the existing package UI only displays information within the `packages_packages` and `packages_package_files` -tables. If the data stored in the metadata tables need to be displayed, a ~frontend change will be required. +tables. If the data stored in the metadata tables need to be displayed, a ~frontend change is required. #### File uploads File uploads should be handled by GitLab Workhorse using object accelerated uploads. What this means is that -the workhorse proxy that checks all incoming requests to GitLab will intercept the upload request, +the workhorse proxy that checks all incoming requests to GitLab intercept the upload request, upload the file, and forward a request to the main GitLab codebase only containing the metadata and file location rather than the file itself. An overview of this process can be found in the [development documentation](uploads.md#direct-upload). -In terms of code, this means a route will need to be added to the +In terms of code, this means a route must be added to the [GitLab Workhorse project](https://gitlab.com/gitlab-org/gitlab-workhorse) for each upload endpoint being added (instance, group, project). [This merge request](https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/412/diffs) demonstrates adding an instance-level endpoint for Conan to workhorse. You can also see the Maven project level endpoint implemented in the same file. -Once the route has been added, you will need to add an additional `/authorize` version of the upload endpoint to your API file. +Once the route has been added, you must add an additional `/authorize` version of the upload endpoint to your API file. [Here is an example](https://gitlab.com/gitlab-org/gitlab/blob/398fef1ca26ae2b2c3dc89750f6b20455a1e5507/ee/lib/api/maven_packages.rb#L164) of the additional endpoint added for Maven. The `/authorize` endpoint verifies and authorizes the request from workhorse, then the normal upload endpoint is implemented below, consuming the metadata that workhorse provides in order to @@ -244,7 +244,7 @@ in your local development environment. ### Future Work -While working on the MVC, contributors will probably find features that are not mandatory for the MVC but can provide a better user experience. It's generally a good idea to keep an eye on those and open issues. +While working on the MVC, contributors might find features that are not mandatory for the MVC but can provide a better user experience. It's generally a good idea to keep an eye on those and open issues. Here are some examples diff --git a/doc/development/product_analytics/event_dictionary.md b/doc/development/product_analytics/event_dictionary.md index 88cb75fdb83..9c363f08cb4 100644 --- a/doc/development/product_analytics/event_dictionary.md +++ b/doc/development/product_analytics/event_dictionary.md @@ -3,3 +3,6 @@ redirect_to: 'https://about.gitlab.com/handbook/product/product-analytics-guide/ --- This document was moved to [another location](https://about.gitlab.com/handbook/product/product-analytics-guide/). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/product_analytics/index.md b/doc/development/product_analytics/index.md index 88cb75fdb83..9c363f08cb4 100644 --- a/doc/development/product_analytics/index.md +++ b/doc/development/product_analytics/index.md @@ -3,3 +3,6 @@ redirect_to: 'https://about.gitlab.com/handbook/product/product-analytics-guide/ --- This document was moved to [another location](https://about.gitlab.com/handbook/product/product-analytics-guide/). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/product_analytics/snowplow.md b/doc/development/product_analytics/snowplow.md index c5f48994d5c..a943bf273c7 100644 --- a/doc/development/product_analytics/snowplow.md +++ b/doc/development/product_analytics/snowplow.md @@ -98,7 +98,7 @@ sequenceDiagram ## Structured event taxonomy -When adding new click events, we should add them in a way that's internally consistent. If we don't, it'll be very painful to perform analysis across features since each feature will be capturing events differently. +When adding new click events, we should add them in a way that's internally consistent. If we don't, it is very painful to perform analysis across features since each feature captures events differently. The current method provides several attributes that are sent on each click event. Please try to follow these guidelines when specifying events to capture: @@ -110,6 +110,10 @@ The current method provides several attributes that are sent on each click event | property | text | false | Any additional property of the element, or object being acted on. | | value | decimal | false | Describes a numeric value or something directly related to the event. This could be the value of an input (e.g. `10` when clicking `internal` visibility). | +### Web-specific parameters + +Snowplow JS adds many [web-specific parameters](https://docs.snowplowanalytics.com/docs/collecting-data/collecting-from-own-applications/snowplow-tracker-protocol/#21_Web-specific_parameters) to all web events by default. + ## 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 use tracking, but each generally requires at minimum, a `category` and an `action`. Additional data can be provided that adheres to our [Structured event taxonomy](#structured-event-taxonomy). diff --git a/doc/development/product_analytics/usage_ping.md b/doc/development/product_analytics/usage_ping.md index fa785d934cb..66d431c2bb0 100644 --- a/doc/development/product_analytics/usage_ping.md +++ b/doc/development/product_analytics/usage_ping.md @@ -31,15 +31,15 @@ More useful links: - The usage data is primarily composed of row counts for different tables in the instance’s database. By comparing these counts month over month (or week over week), we can get a rough sense for how an instance is using the different features within the product. In addition to counts, other facts that help us classify and understand GitLab installations are collected. - Usage ping is important to GitLab as we use it to calculate our Stage Monthly Active Users (SMAU) which helps us measure the success of our stages and features. -- While usage ping is enabled, GitLab will gather data from the other instances and will be able to show usage statistics of your instance to your users. +- While usage ping is enabled, GitLab gathers data from the other instances and can show usage statistics of your instance to your users. ### Why should we enable Usage Ping? - The main purpose of Usage Ping is to build a better GitLab. Data about how GitLab is used is collected to better understand feature/stage adoption and usage, which helps us understand how GitLab is adding value and helps our team better understand the reasons why people use GitLab and with this knowledge we're able to make better product decisions. - As a benefit of having the usage ping active, GitLab lets you analyze the users’ activities over time of your GitLab installation. - As a benefit of having the usage ping active, GitLab provides you with The DevOps Report,which gives you an overview of your entire instance’s adoption of Concurrent DevOps from planning to monitoring. -- You will get better, more proactive support. (assuming that our TAMs and support organization used the data to deliver more value) -- You will get insight and advice into how to get the most value out of your investment in GitLab. Wouldn't you want to know that a number of features or values are not being adopted in your organization? +- You get better, more proactive support. (assuming that our TAMs and support organization used the data to deliver more value) +- You get insight and advice into how to get the most value out of your investment in GitLab. Wouldn't you want to know that a number of features or values are not being adopted in your organization? - You get a report that illustrates how you compare against other similar organizations (anonymized), with specific advice and recommendations on how to improve your DevOps processes. - Usage Ping is enabled by default. To disable it, see [Disable Usage Ping](#disable-usage-ping). @@ -189,7 +189,7 @@ Arguments: - `relation` the ActiveRecord_Relation to perform the count - `column` the column to perform the distinct count, by default is the primary key - `batch`: default `true` in order to use batch counting -- `batch_size`: if none set it will use default value 10000 from `Gitlab::Database::BatchCounter` +- `batch_size`: if none set it uses default value 10000 from `Gitlab::Database::BatchCounter` - `start`: custom start of the batch counting in order to avoid complex min calculations - `end`: custom end of the batch counting in order to avoid complex min calculations @@ -213,7 +213,7 @@ Arguments: - `relation` the ActiveRecord_Relation to perform the operation - `column` the column to sum on -- `batch_size`: if none set it will use default value 1000 from `Gitlab::Database::BatchCounter` +- `batch_size`: if none set it uses default value 1000 from `Gitlab::Database::BatchCounter` - `start`: custom start of the batch counting in order to avoid complex min calculations - `end`: custom end of the batch counting in order to avoid complex min calculations @@ -304,7 +304,7 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF access to a group of events. - `redis_slot`: optional Redis slot; default value: event name. Used if needed to calculate totals for a group of metrics. Ensure keys are in the same slot. For example: - `i_compliance_credential_inventory` with `redis_slot: 'compliance'` will build Redis key + `i_compliance_credential_inventory` with `redis_slot: 'compliance'` builds Redis key `i_{compliance}_credential_inventory-2020-34`. If `redis_slot` is not defined the Redis key will be `{i_compliance_credential_inventory}-2020-34`. - `expiry`: expiry time in days. Default: 29 days for daily aggregation and 6 weeks for weekly @@ -574,7 +574,7 @@ NOTE: **Note:** Prometheus as a data source for Usage Ping is currently only available for single-node Omnibus installations that are running the [bundled Prometheus](../../administration/monitoring/prometheus/index.md) instance. -In order to query Prometheus for metrics, a helper method is available that will `yield` a fully configured +To query Prometheus for metrics, a helper method is available to `yield` a fully configured `PrometheusClient`, given it is available as per the note above: ```ruby @@ -613,7 +613,7 @@ Gitlab::UsageData.distinct_count(::Note.with_suggestions.where(time_period), :au ### 3. Generate the SQL query -Your Rails console will return the generated SQL queries. +Your Rails console returns the generated SQL queries. Example: @@ -631,7 +631,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. -- Any single query must stay below 1 second execution time with cold caches. +- Any single query must stay below [1 second execution time](../query_performance.md#timing-guidelines-for-queries) 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: @@ -670,7 +670,7 @@ Ensure you comply with the [Changelog entries guide](../changelog.md). ### 9. Ask for a Product Analytics Review -On GitLab.com, we have DangerBot setup to monitor Product Analytics related files and DangerBot will recommend a Product Analytics review. Mention `@gitlab-org/growth/product_analytics/engineers` in your MR for a review. +On GitLab.com, we have DangerBot setup to monitor Product Analytics related files and DangerBot recommends a Product Analytics review. Mention `@gitlab-org/growth/product_analytics/engineers` in your MR for a review. ### 10. Verify your metric @@ -696,10 +696,10 @@ This is the recommended approach to test Prometheus based Usage Ping. The easiest way to verify your changes is to build a new Omnibus image from your code branch via CI, then download the image and run a local container instance: -1. From your merge request, click on the `qa` stage, then trigger the `package-and-qa` job. This job will trigger an Omnibus +1. From your merge request, click on the `qa` stage, then trigger the `package-and-qa` job. This job triggers an Omnibus build in a [downstream pipeline of the `omnibus-gitlab-mirror` project](https://gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/-/pipelines). 1. In the downstream pipeline, wait for the `gitlab-docker` job to finish. -1. Open the job logs and locate the full container name including the version. It will take the following form: `registry.gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/gitlab-ee:<VERSION>`. +1. Open the job logs and locate the full container name including the version. It takes the following form: `registry.gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/gitlab-ee:<VERSION>`. 1. On your local machine, make sure you are logged in to the GitLab Docker registry. You can find the instructions for this in [Authenticate to the GitLab Container Registry](../../user/packages/container_registry/index.md#authenticate-with-the-container-registry). 1. Once logged in, download the new image via `docker pull registry.gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/gitlab-ee:<VERSION>` @@ -720,7 +720,7 @@ but with the following limitations: - While it runs a `node_exporter`, `docker-compose` services emulate hosts, meaning that it would normally report itself to not be associated with any of the other services that are running. That is not how node metrics are reported in a production setup, where `node_exporter` always runs as a process alongside other GitLab components on any given node. From Usage Ping's perspective none of the node data would therefore -appear to be associated to any of the services running, since they all appear to be running on different hosts. To alleviate this problem, the `node_exporter` in GCK was arbitrarily "assigned" to the `web` service, meaning only for this service `node_*` metrics will appear in Usage Ping. +appear to be associated to any of the services running, since they all appear to be running on different hosts. To alleviate this problem, the `node_exporter` in GCK was arbitrarily "assigned" to the `web` service, meaning only for this service `node_*` metrics appears in Usage Ping. ## Aggregated metrics @@ -733,14 +733,14 @@ This feature is intended solely for internal GitLab use. In order to add data for aggregated metrics into Usage Ping payload you should add corresponding definition into [`aggregated_metrics.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data_counters/aggregated_metrics.yml) file. Each aggregate definition includes following parts: -- name: unique name under which aggregate metric will be added to Usage Ping payload -- operator: operator that defines how aggregated metric data will be counted. Available operators are: +- name: unique name under which aggregate metric is added to Usage Ping payload +- operator: operator that defines how aggregated metric data is counted. Available operators are: - `OR`: removes duplicates and counts all entries that triggered any of listed events - `AND`: removes duplicates and counts all elements that were observed triggering all of following events - events: list of events names (from [`known_events.yml`](#known-events-in-usage-data-payload)) to aggregate into metric. All events in this list must have the same `redis_slot` and `aggregation` attributes. -- feature_flag: name of [development feature flag](../feature_flags/development.md#development-type) that will be checked before -metrics aggregation is performed. Corresponding feature flag should have `default_enabled` attribute set to `false`. -`feature_flag` attribute is **OPTIONAL** and can be omitted, when `feature_flag` is missing no feature flag will be checked. +- feature_flag: name of [development feature flag](../feature_flags/development.md#development-type) that is checked before +metrics aggregation is performed. Corresponding feature flag should have `default_enabled` attribute set to `false`. +`feature_flag` attribute is **OPTIONAL** and can be omitted, when `feature_flag` is missing no feature flag is checked. Example aggregated metric entries: @@ -754,7 +754,7 @@ Example aggregated metric entries: feature_flag: example_aggregated_metric ``` -Aggregated metrics will be added under `aggregated_metrics` key in both `counts_weekly` and `counts_monthly` top level keys in Usage Ping payload. +Aggregated metrics are added under `aggregated_metrics` key in both `counts_weekly` and `counts_monthly` top level keys in Usage Ping payload. ```ruby { diff --git a/doc/development/prometheus.md b/doc/development/prometheus.md index fc1f2303d0a..62f30871f54 100644 --- a/doc/development/prometheus.md +++ b/doc/development/prometheus.md @@ -3,3 +3,6 @@ redirect_to: '../user/project/integrations/prometheus.md' --- This document was moved to [another location](../user/project/integrations/prometheus.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/prometheus_metrics.md b/doc/development/prometheus_metrics.md index 8fcc025b35b..f51c39e6b45 100644 --- a/doc/development/prometheus_metrics.md +++ b/doc/development/prometheus_metrics.md @@ -31,7 +31,7 @@ The requirement for adding a new metric is to make each query to have an unique ### Update existing metrics -After you add or change an existing common metric, you must [re-run the import script](../administration/raketasks/maintenance.md#import-common-metrics) that will query and update all existing metrics. +After you add or change an existing common metric, you must [re-run the import script](../administration/raketasks/maintenance.md#import-common-metrics) that queries and updates all existing metrics. Or, you can create a database migration: @@ -51,7 +51,7 @@ class ImportCommonMetrics < ActiveRecord::Migration[4.2] end ``` -If a query metric (which is identified by `id:`) is removed it will not be removed from database by default. +If a query metric (which is identified by `id:`) is removed, it isn't removed from database by default. You might want to add additional database migration that makes a decision what to do with removed one. For example: you might be interested in migrating all dependent data to a different metric. @@ -75,5 +75,5 @@ This section describes how to add new metrics for self-monitoring 1. Select the appropriate name for your metric. Refer to the guidelines for [Prometheus metric names](https://prometheus.io/docs/practices/naming/#metric-names). 1. Update the list of [GitLab Prometheus metrics](../administration/monitoring/prometheus/gitlab_metrics.md). -1. Trigger the relevant page/code that will record the new metric. +1. Trigger the relevant page or code that records the new metric. 1. Check that the new metric appears at `/-/metrics`. diff --git a/doc/development/query_performance.md b/doc/development/query_performance.md new file mode 100644 index 00000000000..b8be2bff07f --- /dev/null +++ b/doc/development/query_performance.md @@ -0,0 +1,74 @@ +--- +stage: Enablement +group: Database +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + +# Query performance guidelines + +This document describes various guidelines to follow when optimizing SQL queries. + +When you are optimizing your SQL queries, there are two dimensions to pay attention to: + +1. The query execution time. This is paramount as it reflects how the user experiences GitLab. +1. The query plan. Optimizing the query plan is important in allowing queries to independently scale over time. Realizing that an index will keep a query performing well as the table grows before the query degrades is an example of why we analyze these plans. + +## Timing guidelines for queries + +| Query Type | Maximum Query Time | Notes | +|----|----|---| +| General queries | `100ms` | This is not a hard limit, but if a query is getting above it, it is important to spend time understanding why it can or cannot be optimized. | +| Queries in a migration | `100ms` | This is different than the total [migration time](database_review.md#timing-guidelines-for-migrations). | +| Concurrent operations in a migration | `5min` | Concurrent operations do not block the database, but they block the GitLab update. This includes operations such as `add_concurrent_index` and `add_concurrent_foreign_key`. | +| Background migrations | `1s` | | +| Usage Ping | `1s` | See the [usage ping docs](product_analytics/usage_ping.md#developing-and-testing-usage-ping) for more details. | + +- When analyzing your query's performance, pay attention to if the time you are seeing is on a [cold or warm cache](#cold-and-warm-cache). These guidelines apply for both cache types. +- When working with batched queries, change the range and batch size to see how it effects the query timing and caching. +- If an existing query is already underperforming, make an effort to improve it. If it is too complex or would stall development, create a follow-up so it can be addressed in a timely manner. You can always ask the database reviewer or maintainer for help and guidance. + +## Cold and warm cache + +When evaluating query performance it is important to understand the difference between +cold and warm cached queries. + +The first time a query is made, it is made on a "cold cache". Meaning it needs +to read from disk. If you run the query again, the data can be read from the +cache, or what PostgreSQL calls shared buffers. This is the "warm cache" query. + +When analyzing an [`EXPLAIN` plan](understanding_explain_plans.md), you can see +the difference not only in the timing, but by looking at the output for `Buffers` +by running your explain with `EXPLAIN(analyze, buffers)`. The [#database-lab](understanding_explain_plans.md#database-lab) +tool will automatically include these options. + +If you are making a warm cache query, you will only see the `shared hits`. + +For example in #database-lab: + +```plaintext +Shared buffers: + - hits: 36467 (~284.90 MiB) from the buffer pool + - reads: 0 from the OS file cache, including disk I/O +``` + +Or in the explain plan from `psql`: + +```sql +Buffers: shared hit=7323 +``` + +If the cache is cold, you will also see `reads`. + +In #database-lab: + +```plaintext +Shared buffers: + - hits: 17204 (~134.40 MiB) from the buffer pool + - reads: 15229 (~119.00 MiB) from the OS file cache, including disk I/O +``` + +In `psql`: + +```sql +Buffers: shared hit=7202 read=121 +``` diff --git a/doc/development/rolling_out_changes_using_feature_flags.md b/doc/development/rolling_out_changes_using_feature_flags.md index cff88388ba0..7456e8df8d9 100644 --- a/doc/development/rolling_out_changes_using_feature_flags.md +++ b/doc/development/rolling_out_changes_using_feature_flags.md @@ -3,3 +3,6 @@ redirect_to: 'feature_flags/index.md' --- This document was moved to [another location](feature_flags/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/sidekiq_debugging.md b/doc/development/sidekiq_debugging.md index e0f74b39c9a..a9b6e246861 100644 --- a/doc/development/sidekiq_debugging.md +++ b/doc/development/sidekiq_debugging.md @@ -3,3 +3,6 @@ redirect_to: '../administration/troubleshooting/sidekiq.md' --- This document was moved to [another location](../administration/troubleshooting/sidekiq.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/telemetry/event_dictionary.md b/doc/development/telemetry/event_dictionary.md index 53b7ddea095..bc230a46441 100644 --- a/doc/development/telemetry/event_dictionary.md +++ b/doc/development/telemetry/event_dictionary.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/event_dictionary.md' --- This document was moved to [another location](../product_analytics/event_dictionary.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/telemetry/index.md b/doc/development/telemetry/index.md index 79ea80a52ea..24e83ffc524 100644 --- a/doc/development/telemetry/index.md +++ b/doc/development/telemetry/index.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/index.md' --- This document was moved to [another location](../product_analytics/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/telemetry/snowplow.md b/doc/development/telemetry/snowplow.md index fd75d29286b..7cd385be681 100644 --- a/doc/development/telemetry/snowplow.md +++ b/doc/development/telemetry/snowplow.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/snowplow.md' --- This document was moved to [another location](../product_analytics/snowplow.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/telemetry/usage_ping.md b/doc/development/telemetry/usage_ping.md index 1026e7a5a66..c890353fe3b 100644 --- a/doc/development/telemetry/usage_ping.md +++ b/doc/development/telemetry/usage_ping.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/usage_ping.md' --- This document was moved to [another location](../product_analytics/usage_ping.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/testing.md b/doc/development/testing.md index 79ef8e75432..a0130bfb4ac 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -3,3 +3,6 @@ redirect_to: 'testing_guide/index.md' --- This document was moved to [another location](testing_guide/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index dabb18c1f75..49ada6d9a13 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -603,6 +603,32 @@ it "really connects to Prometheus", :permit_dns do And if you need more specific control, the DNS blocking is implemented in `spec/support/helpers/dns_helpers.rb` and these methods can be called elsewhere. +#### Stubbing File methods + +In the situations where you need to +[stub](https://relishapp.com/rspec/rspec-mocks/v/3-9/docs/basics/allowing-messages) +methods such as `File.read`, make sure to: + +1. Stub `File.read` for only the filepath you are interested in. +1. Call the original implementation for other filepaths. + +Otherwise `File.read` calls from other parts of the codebase get +stubbed incorrectly. You should use the `stub_file_read`, and +`expect_file_read` helper methods which does the stubbing for +`File.read` correctly. + +```ruby +# bad, all Files will read and return nothing +allow(File).to receive(:read) + +# good +stub_file_read(my_filepath) + +# also OK +allow(File).to receive(:read).and_call_original +allow(File).to receive(:read).with(my_filepath) +``` + #### Filesystem Filesystem data can be roughly split into "repositories", and "everything else". 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 ef0bd9902e1..9dea8221210 100644 --- a/doc/development/testing_guide/end_to_end/beginners_guide.md +++ b/doc/development/testing_guide/end_to_end/beginners_guide.md @@ -6,15 +6,15 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Beginner's guide to writing end-to-end tests -In this tutorial, you will learn about the creation of end-to-end (_e2e_) tests +This tutorial walks you through the creation of end-to-end (_e2e_) tests for [GitLab Community Edition](https://about.gitlab.com/install/?version=ce) and [GitLab Enterprise Edition](https://about.gitlab.com/install/). -By the end of this tutorial, you will be able to: +By the end of this tutorial, you can: - Determine whether an end-to-end test is needed. - Understand the directory structure within `qa/`. -- Write a basic end-to-end test that will validate login features. +- Write a basic end-to-end test that validates login features. - Develop any missing [page object](page_objects.md) libraries. ## Before you write a test @@ -68,17 +68,17 @@ 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/product-categories/#devops-stages), -determine which feature the test will belong to, and then place it in a subdirectory +determine which feature the test belongs to, and then place it in a subdirectory under the stage.  -If the test is Enterprise Edition only, the test will be created in the `features/ee` +If the test is Enterprise Edition only, the test is created in the `features/ee` directory, but follow the same DevOps lifecycle format. ## Create a skeleton test -In the first part of this tutorial we will be testing login, which is owned by the +In the first part of this tutorial we are testing login, which is owned by the Manage stage. Inside `qa/specs/features/browser_ui/1_manage/login`, create a file `basic_login_spec.rb`. @@ -286,12 +286,12 @@ end Note the following important points: -- At the start of our example, we will be at the `page/issue/show.rb` [page](page_objects.md). +- At the start of our example, we are at the `page/issue/show.rb` [page](page_objects.md). - Our test fabricates only what it needs, when it needs it. - The issue is fabricated through the API to save time. - GitLab prefers `let()` over instance variables. See [best practices](../best_practices.md#subject-and-let-variables). -- `be_closed` is not implemented in `page/project/issue/show.rb` yet, but will be +- `be_closed` is not implemented in `page/project/issue/show.rb` yet, but is implemented in the next step. The issue is fabricated as a [Resource](resources.md), which is a GitLab entity diff --git a/doc/development/testing_guide/end_to_end/dynamic_element_validation.md b/doc/development/testing_guide/end_to_end/dynamic_element_validation.md index 871b3f80c18..f6aea82eae9 100644 --- a/doc/development/testing_guide/end_to_end/dynamic_element_validation.md +++ b/doc/development/testing_guide/end_to_end/dynamic_element_validation.md @@ -23,7 +23,7 @@ We interpret user actions on the page to have some sort of effect. These actions ### Navigation -When a page is navigated to, there are elements that will always appear on the page unconditionally. +When a page is navigated to, there are elements that always appear on the page unconditionally. Dynamic element validation is instituted when using @@ -100,7 +100,7 @@ Runtime::Browser.visit(:gitlab, Page::MyPage) execute_stuff ``` -will invoke GitLab QA to scan `MyPage` for `my_element` and `another_element` to be on the page before continuing to +invokes GitLab QA to scan `MyPage` for `my_element` and `another_element` to be on the page before continuing to `execute_stuff` ### Clicking @@ -113,7 +113,7 @@ def open_layer end ``` -will invoke GitLab QA to ensure that `message_content` appears on +invokes GitLab QA to ensure that `message_content` appears on the Layer upon clicking `my_element`. -This will imply that the Layer is indeed rendered before we continue our test. +This implies that the Layer is indeed rendered before we continue our test. diff --git a/doc/development/testing_guide/end_to_end/environment_selection.md b/doc/development/testing_guide/end_to_end/environment_selection.md index f5e3e99b79e..a2f3fda6db6 100644 --- a/doc/development/testing_guide/end_to_end/environment_selection.md +++ b/doc/development/testing_guide/end_to_end/environment_selection.md @@ -65,4 +65,4 @@ If you want to run an `only: { :pipeline }` tagged test on your local GDK make s 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. +For instance, `quarantine: { only: { subdomain: :staging } }` only quarantines 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 2ff1c9f6dc3..eabec05780d 100644 --- a/doc/development/testing_guide/end_to_end/feature_flags.md +++ b/doc/development/testing_guide/end_to_end/feature_flags.md @@ -10,7 +10,7 @@ To run a specific test with a feature flag enabled you can use the `QA::Runtime: enable and disable feature flags ([via the API](../../../api/features.md)). Note that administrator authorization is required to change feature flags. `QA::Runtime::Feature` -will automatically authenticate as an administrator as long as you provide an appropriate access +automatically authenticates as an administrator as long as you provide an appropriate access token via `GITLAB_QA_ADMIN_ACCESS_TOKEN` (recommended), or provide `GITLAB_ADMIN_USERNAME` and `GITLAB_ADMIN_PASSWORD`. @@ -60,7 +60,7 @@ feature_group = "a_feature_group" Runtime::Feature.enable(:feature_flag_name, feature_group: feature_group) ``` -If no scope is provided, the feature flag will be set instance-wide: +If no scope is provided, the feature flag is set instance-wide: ```ruby # This will affect all users! diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index 1d144359ed8..f71abd6d252 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -126,8 +126,8 @@ For example, when we [dequarantine](https://about.gitlab.com/handbook/engineerin a flaky test we first want to make sure that it's no longer flaky. We can do that using the `ce:custom-parallel` and `ee:custom-parallel` jobs. Both are manual jobs that you can configure using custom variables. -When you click the name (not the play icon) of one of the parallel jobs, -you'll be prompted to enter variables. You can use any of [the variables +When clicking the name (not the play icon) of one of the parallel jobs, +you are prompted to enter variables. You can use any of [the variables that can be used with `gitlab-qa`](https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/what_tests_can_be_run.md#supported-gitlab-environment-variables) as well as these: @@ -137,7 +137,7 @@ as well as these: | `QA_TESTS` | The test(s) to run (no default, which means run all the tests in the scenario). Use file paths as you would when running tests via RSpec, e.g., `qa/specs/features/ee/browser_ui` would include all the `EE` UI tests. | | `QA_RSPEC_TAGS` | The RSpec tags to add (no default) | -For now [manual jobs with custom variables will not use the same variable +For now [manual jobs with custom variables don't use the same variable when retried](https://gitlab.com/gitlab-org/gitlab/-/issues/31367), so if you want to run the same test(s) multiple times, specify the same variables in each `custom-parallel` job (up to as many of the 10 available jobs that you want to run). diff --git a/doc/development/testing_guide/end_to_end/page_objects.md b/doc/development/testing_guide/end_to_end/page_objects.md index 7eacaf4b08a..799a0b61025 100644 --- a/doc/development/testing_guide/end_to_end/page_objects.md +++ b/doc/development/testing_guide/end_to_end/page_objects.md @@ -30,13 +30,13 @@ need to rely on volatile helpers or invoke Capybara methods directly. Imagine invoking `fill_in :user_login` in every `*_spec.rb` file / test example. When someone later changes `t.text_field :login` in the view associated with -this page to `t.text_field :username` it will generate a different field +this page to `t.text_field :username` it generates a different field identifier, what would effectively break all tests. Because we are using `Page::Main::Login.perform(&:sign_in_using_credentials)` everywhere, when we want to sign in to GitLab, the page object is the single -source of truth, and we will need to update `fill_in :user_login` -to `fill_in :user_username` only in a one place. +source of truth, and we must update `fill_in :user_login` +to `fill_in :user_username` only in one place. ## What problems did we have in the past? @@ -44,7 +44,7 @@ We do not run QA tests for every commit, because of performance reasons, and the time it would take to build packages and test everything. That is why when someone changes `t.text_field :login` to -`t.text_field :username` in the _new session_ view we won't know about this +`t.text_field :username` in the _new session_ view we don't know about this change until our GitLab QA nightly pipeline fails, or until someone triggers `package-and-qa` action in their merge request. @@ -56,14 +56,14 @@ problem by introducing coupling between GitLab CE / EE views and GitLab QA. ## How did we solve fragile tests problem? -Currently, when you add a new `Page::Base` derived class, you will also need to +Currently, when you add a new `Page::Base` derived class, you must also define all selectors that your page objects depend on. Whenever you push your code to CE / EE repository, `qa:selectors` sanity test -job is going to be run as a part of a CI pipeline. +job runs as a part of a CI pipeline. -This test is going to validate all page objects that we have implemented in -`qa/page` directory. When it fails, you will be notified about missing +This test validates all page objects that we have implemented in +`qa/page` directory. When it fails, it notifies you about missing or invalid views/selectors definition. ## How to properly implement a page object? @@ -95,10 +95,10 @@ end ### Defining Elements -The `view` DSL method will correspond to the Rails view, partial, or Vue component that renders the elements. +The `view` DSL method corresponds to the Rails view, partial, or Vue component that renders the elements. The `element` DSL method in turn declares an element for which a corresponding -`data-qa-selector=element_name_snaked` data attribute will need to be added to the view file. +`data-qa-selector=element_name_snaked` data attribute must be added to the view file. You can also define a value (String or Regexp) to match to the actual view code but **this is deprecated** in favor of the above method for two reasons: @@ -257,7 +257,7 @@ These modules must: 1. Include/prepend other modules and define their `view`/`elements` in a `base.class_eval` block to ensure they're defined in the class that prepends the module. -These steps ensure the sanity selectors check will detect problems properly. +These steps ensure the sanity selectors check detect problems properly. For example, `qa/qa/ee/page/merge_request/show.rb` adds EE-specific methods to `qa/qa/page/merge_request/show.rb` (with `QA::Page::MergeRequest::Show.prepend_if_ee('QA::EE::Page::MergeRequest::Show')`) and following is how it's implemented diff --git a/doc/development/testing_guide/end_to_end/resources.md b/doc/development/testing_guide/end_to_end/resources.md index d73bae331d5..e23a74dcbd2 100644 --- a/doc/development/testing_guide/end_to_end/resources.md +++ b/doc/development/testing_guide/end_to_end/resources.md @@ -94,7 +94,7 @@ needs a group to be created in. To define a resource attribute, you can use the `attribute` method with a block using the other resource class to fabricate the resource. -That will allow access to the other resource from your resource object's +That allows access to the other resource from your resource object's methods. You would usually use it in `#fabricate!`, `#api_get_path`, `#api_post_path`, `#api_post_body`. @@ -144,7 +144,7 @@ end ``` **Note that all the attributes are lazily constructed. This means if you want -a specific attribute to be fabricated first, you'll need to call the +a specific attribute to be fabricated first, you must call the attribute method first even if you're not using it.** #### Product data attributes @@ -185,7 +185,7 @@ end ``` **Note again that all the attributes are lazily constructed. This means if -you call `shirt.brand` after moving to the other page, it'll not properly +you call `shirt.brand` after moving to the other page, it doesn't properly retrieve the data because we're no longer on the expected page.** Consider this: @@ -201,7 +201,7 @@ shirt.project.visit! shirt.brand # => FAIL! ``` -The above example will fail because now we're on the project page, trying to +The above example fails because now we're on the project page, trying to construct the brand data from the shirt page, however we moved to the project page already. There are two ways to solve this, one is that we could try to retrieve the brand before visiting the project again: @@ -219,8 +219,8 @@ shirt.project.visit! shirt.brand # => OK! ``` -The attribute will be stored in the instance therefore all the following calls -will be fine, using the data previously constructed. If we think that this +The attribute is stored in the instance, therefore all the following calls +are fine, using the data previously constructed. If we think that this might be too brittle, we could eagerly construct the data right before ending fabrication: @@ -249,12 +249,12 @@ module QA end ``` -The `populate` method will iterate through its arguments and call each +The `populate` method iterates through its arguments and call each attribute respectively. Here `populate(:brand)` has the same effect as just `brand`. Using the populate method makes the intention clearer. -With this, it will make sure we construct the data right after we create the -shirt. The drawback is that this will always construct the data when the +With this, it ensures we construct the data right after we create the +shirt. The drawback is that this always constructs the data when the resource is fabricated even if we don't need to use the data. Alternatively, we could just make sure we're on the right page before @@ -290,7 +290,7 @@ module QA end ``` -This will make sure it's on the shirt page before constructing brand, and +This ensures it's on the shirt page before constructing brand, and move back to the previous page to avoid breaking the state. #### Define an attribute based on an API response @@ -343,16 +343,16 @@ end - resource instance variables have the highest precedence - attributes from the API response take precedence over attributes from the block (usually from Browser UI) -- attributes without a value will raise a `QA::Resource::Base::NoValueError` error +- attributes without a value raises a `QA::Resource::Base::NoValueError` error ## Creating resources in your tests To create a resource in your tests, you can call the `.fabricate!` method on the resource class. -Note that if the resource class supports API fabrication, this will use this +Note that if the resource class supports API fabrication, this uses this fabrication by default. -Here is an example that will use the API fabrication method under the hood +Here is an example that uses the API fabrication method under the hood since it's supported by the `Shirt` resource class: ```ruby @@ -389,8 +389,7 @@ my_shirt = Resource::Shirt.fabricate_via_api! do |shirt| end ``` -In this case, the result will be similar to calling -`Resource::Shirt.fabricate!`. +In this case, the result is similar to calling `Resource::Shirt.fabricate!`. ## Where to ask for help? 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 8e99cf18ea0..013967a9826 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 @@ -14,16 +14,16 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec | Tag | Description | |-----|-------------| | `: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. | -| `:gitaly_cluster` | 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. | -| `: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. -| `: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`._ | +| `:gitaly_cluster` | The test runs 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. | +| `:jira` | The test requires a Jira Server. [GitLab-QA](https://gitlab.com/gitlab-org/gitlab-qa) provisions the Jira Server in a Docker container when the `Test::Integration::Jira` test scenario is run. +| `:kubernetes` | The test includes a GitLab instance that is configured to be run behind an SSH tunnel, allowing a TLS-accessible GitLab. This test also includes provisioning of at least one Kubernetes cluster to test against. _This tag is often be paired with `:orchestrated`._ | | `:only` | The test is only to be run against specific environments or pipelines. See [Environment selection](environment_selection.md) for more information. | | `: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. 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). | +| `:quarantine` | The test has been [quarantined](https://about.gitlab.com/handbook/engineering/quality/guidelines/debugging-qa-test-failures/#quarantining-tests), runs in a separate job that only includes quarantined tests, and is allowed to fail. The test is skipped in its regular job so that if it fails it doesn't 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. | -| `:skip_live_env` | The test will be excluded when run against live deployed environments such as Staging, Canary, and Production. | +| `:runner` | The test depends on and sets up a GitLab Runner instance, typically to run a pipeline. | +| `:skip_live_env` | The test is excluded when run against live deployed environments such as Staging, Canary, and Production. | | `:testcase` | The link to the test case issue in the [Quality Testcases project](https://gitlab.com/gitlab-org/quality/testcases/). | | `:mattermost` | The test requires a GitLab Mattermost service on the GitLab instance. | | `:ldap_no_server` | The test requires a GitLab instance to be configured to use LDAP. To be used with the `:orchestrated` tag. It does not spin up an LDAP server at orchestration time. Instead, it creates the LDAP server at runtime. | @@ -33,7 +33,7 @@ This is a partial list of the [RSpec metadata](https://relishapp.com/rspec/rspec | `:smtp` | The test requires a GitLab instance to be configured to use an SMTP server. Tests SMTP notification email delivery from GitLab by using MailHog. | | `:group_saml` | The test requires a GitLab instance that has SAML SSO enabled at the group level. Interacts with an external SAML identity provider. Paired with the `:orchestrated` tag. | | `:instance_saml` | The test requires a GitLab instance that has SAML SSO enabled at the instance level. Interacts with an external SAML identity provider. Paired with the `:orchestrated` tag. | -| `:skip_signup_disabled` | The test uses UI to sign up a new user and will be skipped in any environment that does not allow new user registration via the UI. | +| `:skip_signup_disabled` | The test uses UI to sign up a new user and is skipped in any environment that does not allow new user registration via the UI. | | `:smoke` | The test belongs to the test suite which verifies basic functionality of a GitLab instance.| | `:github` | The test requires a GitHub personal access token. | | `:repository_storage` | The test requires a GitLab instance to be configured to use multiple [repository storage paths](../../../administration/repository_storage_paths.md). Paired with the `:orchestrated` tag. | diff --git a/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md b/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md index df4b833526c..955fd27874d 100644 --- a/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md +++ b/doc/development/testing_guide/end_to_end/running_tests_that_require_special_setup.md @@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w The [`jenkins_build_status_spec`](https://gitlab.com/gitlab-org/gitlab/blob/163c8a8c814db26d11e104d1cb2dcf02eb567dbe/qa/qa/specs/features/ee/browser_ui/3_create/jenkins/jenkins_build_status_spec.rb) spins up a Jenkins instance in a Docker container based on an image stored in the [GitLab-QA container registry](https://gitlab.com/gitlab-org/gitlab-qa/container_registry). The Docker image it uses is preconfigured with some base data and plugins. -The test then configures the GitLab plugin in Jenkins with a URL of the GitLab instance that will be used +The test then configures the GitLab plugin in Jenkins with a URL of the GitLab instance that are used to run the tests. Unfortunately, the GitLab Jenkins plugin does not accept ports so `http://localhost:3000` would not be accepted. Therefore, this requires us to run GitLab on port 80 or inside a Docker container. @@ -30,7 +30,7 @@ To run the tests from the `/qa` directory: CHROME_HEADLESS=false bin/qa Test::Instance::All http://localhost -- qa/specs/features/ee/browser_ui/3_create/jenkins/jenkins_build_status_spec.rb ``` -The test will automatically spin up a Docker container for Jenkins and tear down once the test completes. +The test automatically spins up a Docker container for Jenkins and tear down once the test completes. However, if you need to run Jenkins manually outside of the tests, use this command: @@ -43,7 +43,7 @@ docker run \ registry.gitlab.com/gitlab-org/gitlab-qa/jenkins-gitlab:version1 ``` -Jenkins will be available on `http://localhost:8080`. +Jenkins is available on `http://localhost:8080`. Admin username is `admin` and password is `password`. @@ -59,19 +59,19 @@ the Docker Engine. The tests tagged `:gitaly_ha` are orchestrated tests that can only be run against a set of Docker containers as configured and started by [the `Test::Integration::GitalyCluster` GitLab QA scenario](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/master/docs/what_tests_can_be_run.md#testintegrationgitalycluster-ceeefull-image-address). -As described in the documentation about the scenario noted above, the following command will run the tests: +As described in the documentation about the scenario noted above, the following command runs the tests: ```shell gitlab-qa Test::Integration::GitalyCluster EE ``` -However, that will remove the containers after it finishes running the tests. If you would like to do further testing, for example, if you would like to run a single test via a debugger, you can use [the `--no-tests` option](https://gitlab.com/gitlab-org/gitlab-qa#command-line-options) to make `gitlab-qa` skip running the tests, and to leave the containers running so that you can continue to use them. +However, that removes the containers after it finishes running the tests. If you would like to do further testing, for example, if you would like to run a single test via a debugger, you can use [the `--no-tests` option](https://gitlab.com/gitlab-org/gitlab-qa#command-line-options) to make `gitlab-qa` skip running the tests, and to leave the containers running so that you can continue to use them. ```shell gitlab-qa Test::Integration::GitalyCluster EE --no-tests ``` -When all the containers are running you will see the output of the `docker ps` command, showing on which ports the GitLab container can be accessed. For example: +When all the containers are running, the output of the `docker ps` command shows which ports the GitLab container can be accessed on. For example: ```plaintext CONTAINER ID ... PORTS NAMES @@ -82,7 +82,7 @@ That shows that the GitLab instance running in the `gitlab-gitaly-ha` container Another option is to use NGINX. -In both cases you will need to configure your machine to translate `gitlab-gitlab-ha.test` into an appropriate IP address: +In both cases you must configure your machine to translate `gitlab-gitlab-ha.test` into an appropriate IP address: ```shell echo '127.0.0.1 gitlab-gitaly-ha.test' | sudo tee -a /etc/hosts @@ -205,7 +205,7 @@ You can now search through the logs for *Job log*, which matches delimited secti A Job log is a subsection within these logs, related to app deployment. We use two jobs: `build` and `production`. You can find the root causes of deployment failures in these logs, which can compromise the entire test. -If a `build` job fails, the `production` job won't run, and the test fails. +If a `build` job fails, the `production` job doesn't run, and the test fails. The long test setup does not take screenshots of failures, which is a known [issue](https://gitlab.com/gitlab-org/quality/team-tasks/-/issues/270). However, if the spec fails (after a successful deployment) then you should be able to find screenshots which display the feature failure. @@ -286,7 +286,7 @@ CHROME_HEADLESS=false bundle exec bin/qa QA::EE::Scenario::Test::Geo --primary-a You can use [GitLab-QA Orchestrator](https://gitlab.com/gitlab-org/gitlab-qa) to orchestrate two GitLab containers and configure them as a Geo setup. -Geo requires an EE license. To visit the Geo sites in your browser, you will need a reverse proxy server (for example, [NGINX](https://www.nginx.com/)). +Geo requires an EE license. To visit the Geo sites in your browser, you need a reverse proxy server (for example, [NGINX](https://www.nginx.com/)). 1. Export your EE license @@ -296,7 +296,7 @@ Geo requires an EE license. To visit the Geo sites in your browser, you will nee 1. (Optional) Pull the GitLab image - This step is optional because pulling the Docker image is part of the [`Test::Integration::Geo` orchestrated scenario](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/d8c5c40607c2be0eda58bbca1b9f534b00889a0b/lib/gitlab/qa/scenario/test/integration/geo.rb). However, it's easier to monitor the download progress if you pull the image first, and the scenario will skip this step after checking that the image is up to date. + This step is optional because pulling the Docker image is part of the [`Test::Integration::Geo` orchestrated scenario](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/d8c5c40607c2be0eda58bbca1b9f534b00889a0b/lib/gitlab/qa/scenario/test/integration/geo.rb). However, it's easier to monitor the download progress if you pull the image first, and the scenario skips this step after checking that the image is up to date. ```shell # For the most recent nightly image @@ -309,7 +309,7 @@ Geo requires an EE license. To visit the Geo sites in your browser, you will nee docker pull registry.gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/gitlab-ee:examplesha123456789 ``` -1. Run the [`Test::Integration::Geo` orchestrated scenario](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/d8c5c40607c2be0eda58bbca1b9f534b00889a0b/lib/gitlab/qa/scenario/test/integration/geo.rb) with the `--no-teardown` option to build the GitLab containers, configure the Geo setup, and run Geo end-to-end tests. Running the tests after the Geo setup is complete is optional; the containers will keep running after you stop the tests. +1. Run the [`Test::Integration::Geo` orchestrated scenario](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/d8c5c40607c2be0eda58bbca1b9f534b00889a0b/lib/gitlab/qa/scenario/test/integration/geo.rb) with the `--no-teardown` option to build the GitLab containers, configure the Geo setup, and run Geo end-to-end tests. Running the tests after the Geo setup is complete is optional; the containers keep running after you stop the tests. ```shell # Using the most recent nightly image diff --git a/doc/development/testing_guide/flaky_tests.md b/doc/development/testing_guide/flaky_tests.md index 47ed11d76a2..9ae6e9411ad 100644 --- a/doc/development/testing_guide/flaky_tests.md +++ b/doc/development/testing_guide/flaky_tests.md @@ -26,14 +26,14 @@ it 'should succeed', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/ end ``` -This means it will be skipped unless run with `--tag quarantine`: +This means it is skipped unless run with `--tag quarantine`: ```shell bin/rspec --tag quarantine ``` **Before putting a test in quarantine, you should make sure that a -~"master:broken" issue exists for it so it won't stay in quarantine forever.** +~"master:broken" issue exists for it so it doesn't stay in quarantine forever.** Once a test is in quarantine, there are 3 choices: diff --git a/doc/development/testing_guide/testing_rake_tasks.md b/doc/development/testing_guide/testing_rake_tasks.md index 384739d1adb..c1414db2387 100644 --- a/doc/development/testing_guide/testing_rake_tasks.md +++ b/doc/development/testing_guide/testing_rake_tasks.md @@ -11,7 +11,7 @@ in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use `require 'rake_helper'`. The helper includes `spec_helper` for you, and configures a few other things to make testing Rake tasks easier. -At a minimum, requiring the Rake helper will redirect `stdout`, include the +At a minimum, requiring the Rake helper redirects `stdout`, include the runtime task helpers, and include the `RakeHelpers` Spec support module. The `RakeHelpers` module exposes a `run_rake_task(<task>)` method to make diff --git a/doc/development/ux_guide/users.md b/doc/development/ux_guide/users.md index 3aecbbffd73..6d586ce807b 100644 --- a/doc/development/ux_guide/users.md +++ b/doc/development/ux_guide/users.md @@ -3,3 +3,6 @@ redirect_to: 'https://about.gitlab.com/handbook/marketing/product-marketing/role --- This document was moved to [another location](https://about.gitlab.com/handbook/marketing/product-marketing/roles-personas/). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> |